From 2c7c7478aae0c5f769d0e5e8b33458c11bdaea98 Mon Sep 17 00:00:00 2001 From: David <244863912+dyk1454683243-sudo@users.noreply.github.com> Date: Sun, 20 Sep 2026 02:29:15 +0000 Subject: [PATCH] Preserve noprotect comment when changing p5.js library version Version and addon HTML rewrites serialize via documentElement.outerHTML, which drops comments outside and flips the Loop Protection toggle. Restore the existing flag after serialization. Fixes #4303 Co-authored-by: David --- client/modules/IDE/hooks/useP5Version.jsx | 3 +- .../IDE/hooks/useP5Version.unit.test.jsx | 87 +++++++++++++++++++ client/modules/IDE/utils/loopProtection.js | 8 ++ .../IDE/utils/loopProtection.unit.test.js | 64 ++++++++++++++ 4 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 client/modules/IDE/hooks/useP5Version.unit.test.jsx create mode 100644 client/modules/IDE/utils/loopProtection.unit.test.js diff --git a/client/modules/IDE/hooks/useP5Version.jsx b/client/modules/IDE/hooks/useP5Version.jsx index a21a7c3b7f..8f52b289bc 100644 --- a/client/modules/IDE/hooks/useP5Version.jsx +++ b/client/modules/IDE/hooks/useP5Version.jsx @@ -11,6 +11,7 @@ import { p5DataAddonURL, p5URLTemplate } from '../../../../common/p5URLs'; +import { preserveNoProtect } from '../utils/loopProtection'; export const majorVersion = (version) => version.split('.')[0]; @@ -46,7 +47,7 @@ export function P5VersionProvider(props) { src = `${doctype}\n${src}`; } - return src; + return preserveNoProtect(indexSrc, src); }; const usedP5Versions = [...dom.documentElement.querySelectorAll('script')] diff --git a/client/modules/IDE/hooks/useP5Version.unit.test.jsx b/client/modules/IDE/hooks/useP5Version.unit.test.jsx new file mode 100644 index 0000000000..2ad73628b5 --- /dev/null +++ b/client/modules/IDE/hooks/useP5Version.unit.test.jsx @@ -0,0 +1,87 @@ +import React from 'react'; +import { reduxRender } from '../../../test-utils'; +import { initialState as filesInitialState } from '../reducers/files'; +import { p5PreloadAddonURL } from '../../../../common/p5URLs'; +import { hasNoProtect } from '../utils/loopProtection'; +import { P5VersionProvider, useP5Version } from './useP5Version'; + +const P5_SCRIPT = 'https://cdn.jsdelivr.net/npm/p5@2.3.2/lib/p5.js'; + +function indexHtml({ noprotect = false, sound = false } = {}) { + const soundTag = sound + ? '\n ' + : ''; + const html = ` + + + ${soundTag} + + +`; + return noprotect ? `\n${html}` : html; +} + +let capturedVersionInfo = null; + +function VersionProbe() { + const { versionInfo } = useP5Version(); + capturedVersionInfo = versionInfo; + return null; +} + +function renderVersionInfo(indexContent) { + capturedVersionInfo = null; + reduxRender( + + + , + { + initialState: { + files: filesInitialState().map((file) => ({ + ...file, + ...(file.fileType === 'file' && + file.name === 'index.html' && + file.filePath === '' && { content: indexContent }) + })) + } + } + ); + return capturedVersionInfo; +} + +describe('useP5Version HTML rewrites', () => { + afterEach(() => { + capturedVersionInfo = null; + }); + + it('keeps when replaceVersion rewrites index.html', () => { + const versionInfo = renderVersionInfo(indexHtml({ noprotect: true })); + const next = versionInfo.replaceVersion('2.3.3'); + + expect(hasNoProtect(next)).toBe(true); + expect(next).toContain('p5@2.3.3/lib/p5.js'); + expect(next.match(//g)).toHaveLength(1); + }); + + it('does not insert when the original had none', () => { + const versionInfo = renderVersionInfo(indexHtml()); + const next = versionInfo.replaceVersion('2.3.3'); + + expect(hasNoProtect(next)).toBe(false); + expect(next).toContain('p5@2.3.3/lib/p5.js'); + }); + + it('keeps when addon setters rewrite index.html', () => { + const versionInfo = renderVersionInfo( + indexHtml({ noprotect: true, sound: true }) + ); + + const afterSoundOff = versionInfo.setP5Sound(false); + expect(hasNoProtect(afterSoundOff)).toBe(true); + expect(afterSoundOff).not.toContain('p5.sound'); + + const afterPreload = versionInfo.setP5PreloadAddon(true); + expect(hasNoProtect(afterPreload)).toBe(true); + expect(afterPreload).toContain(p5PreloadAddonURL); + }); +}); diff --git a/client/modules/IDE/utils/loopProtection.js b/client/modules/IDE/utils/loopProtection.js index 49e591496a..72af02872c 100644 --- a/client/modules/IDE/utils/loopProtection.js +++ b/client/modules/IDE/utils/loopProtection.js @@ -16,3 +16,11 @@ export function removeNoProtect(src = '') { export function toggleLoopProtection(src = '', enabled) { return enabled ? removeNoProtect(src) : addNoProtect(src); } + +// Version/addon HTML rewrites serialize via documentElement.outerHTML, which +// drops comments outside — including a leading . +export function preserveNoProtect(originalSrc = '', serializedSrc = '') { + return hasNoProtect(originalSrc) + ? addNoProtect(serializedSrc) + : serializedSrc; +} diff --git a/client/modules/IDE/utils/loopProtection.unit.test.js b/client/modules/IDE/utils/loopProtection.unit.test.js new file mode 100644 index 0000000000..d9483a775d --- /dev/null +++ b/client/modules/IDE/utils/loopProtection.unit.test.js @@ -0,0 +1,64 @@ +import { + addNoProtect, + hasNoProtect, + preserveNoProtect, + removeNoProtect, + toggleLoopProtection +} from './loopProtection'; + +describe('loopProtection helpers', () => { + const html = ` + + + +`; + const withNoProtect = `\n${html}`; + + describe('hasNoProtect', () => { + it('is true when a leading noprotect comment exists', () => { + expect(hasNoProtect(withNoProtect)).toBe(true); + }); + + it('is false when the comment is absent', () => { + expect(hasNoProtect(html)).toBe(false); + }); + }); + + describe('addNoProtect / removeNoProtect', () => { + it('prepends the comment once', () => { + expect(addNoProtect(html)).toBe(withNoProtect); + expect(addNoProtect(withNoProtect)).toBe(withNoProtect); + }); + + it('strips the comment', () => { + expect(removeNoProtect(withNoProtect)).toBe(html); + expect(removeNoProtect(html)).toBe(html); + }); + }); + + describe('toggleLoopProtection', () => { + it('removes noprotect when protection is enabled', () => { + expect(toggleLoopProtection(withNoProtect, true)).toBe(html); + }); + + it('adds noprotect when protection is disabled', () => { + expect(toggleLoopProtection(html, false)).toBe(withNoProtect); + }); + }); + + describe('preserveNoProtect', () => { + it('restores noprotect after a rewrite that dropped it', () => { + expect(preserveNoProtect(withNoProtect, html)).toBe(withNoProtect); + }); + + it('does not insert noprotect when the original had none', () => { + expect(preserveNoProtect(html, html)).toBe(html); + }); + + it('does not duplicate noprotect when the rewrite kept it', () => { + expect(preserveNoProtect(withNoProtect, withNoProtect)).toBe( + withNoProtect + ); + }); + }); +});