From c3b4b06fd4d2865a563441a09be205905c7934fa Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 23 Oct 2026 19:45:02 -0400 Subject: [PATCH 1/2] fix(sdk): a delete is kept hidden when any script lookup could reach it, not only an id --- .../apply-patches.scriptTargets.test.ts | 38 +++++++++++++ packages/sdk/src/engine/scriptLookups.ts | 55 ++++++++++++------- 2 files changed, 73 insertions(+), 20 deletions(-) diff --git a/packages/sdk/src/engine/apply-patches.scriptTargets.test.ts b/packages/sdk/src/engine/apply-patches.scriptTargets.test.ts index f03453b96dc..601fd5b9013 100644 --- a/packages/sdk/src/engine/apply-patches.scriptTargets.test.ts +++ b/packages/sdk/src/engine/apply-patches.scriptTargets.test.ts @@ -101,5 +101,43 @@ describe("a kept delete carries no media source", () => { it("leaves the sources of nodes that were not deleted", () => { expect(out()).toContain("live.png"); + +const lookupDoc = (script: string) => ` +
+ +
+
keep
+
+ +`; + +describe("a stored delete any script lookup could reach", () => { + const kept = (script: string, id: string) => { + const parsed = parseMutable(lookupDoc(script)); + applyOverrideSet(parsed, { [id]: null }); + return serializeDocument(parsed).includes("data-hidden"); + }; + + it("is hidden when the script looks it up by class", () => { + expect(kept("document.querySelector('.halftone-svg').getBBox();", "hf-halftone")).toBe(true); + expect( + kept("document.getElementsByClassName('halftone-svg')[0].getBBox();", "hf-halftone"), + ).toBe(true); + }); + + it("is hidden when the lookup argument is a variable", () => { + expect( + kept( + "['#fiber-1'].forEach(function (s) { document.querySelector(s).getTotalLength(); });", + "hf-fiber", + ), + ).toBe(true); + expect( + kept("var id = 'fiber-1'; document.getElementById(id).getTotalLength();", "hf-fiber"), + ).toBe(true); + }); + + it("is still removed when no lookup can reach it", () => { + expect(kept("document.getElementById('other').getBBox();", "hf-plain")).toBe(false); }); }); diff --git a/packages/sdk/src/engine/scriptLookups.ts b/packages/sdk/src/engine/scriptLookups.ts index ae02e201a86..75b0b5a724a 100644 --- a/packages/sdk/src/engine/scriptLookups.ts +++ b/packages/sdk/src/engine/scriptLookups.ts @@ -1,9 +1,12 @@ -// Whether an inline script looks the element up by id. A replayed delete of such a node is hidden, +// Whether an inline script could look the element up. A replayed delete of such a node is hidden, // not removed: a missing node makes the script throw before the timeline registers. -// The string handed to getElementById / querySelector(All); a template's ${...} stands for any text. -const LOOKUP = /(getElementById|querySelector(?:All)?)\s*\(\s*(["'`])([\s\S]*?)\2/g; -const SELECTOR_ID = /#((?:[\w-]|\$\{[^}]*\})+)/g; +// A literal handed to a lookup; a template's ${...} stands for any text. +const LITERAL_LOOKUP = + /(getElementById|getElementsByClassName|getElementsByTagName|querySelector(?:All)?)\s*\(\s*(["'`])([\s\S]*?)\2/g; +// A lookup whose argument is a variable or an expression: it can name any node. +const DYNAMIC_LOOKUP = + /(?:getElementById|getElementsByClassName|getElementsByTagName|querySelector(?:All)?)\s*\(\s*(?!["'`)\s])/; const PLACEHOLDER = /\$\{[^}]*\}/; function idPattern(text: string): RegExp { @@ -12,27 +15,39 @@ function idPattern(text: string): RegExp { return new RegExp(`^${escaped.join("[\\w-]+")}$`); } -function lookedUpPatterns(scripts: string): RegExp[] { - const patterns: RegExp[] = []; - for (const [, method, , argument] of scripts.matchAll(LOOKUP)) { - const ids = - method === "getElementById" - ? [argument ?? ""] - : Array.from((argument ?? "").matchAll(SELECTOR_ID), (match) => match[1] ?? ""); - patterns.push(...ids.map(idPattern)); +function matchesSelector(el: Element, selector: string): boolean { + if (PLACEHOLDER.test(selector)) return true; + try { + return el.matches(selector) || el.querySelector(selector) !== null; + } catch { + return false; } - return patterns; +} + +function lookupReaches(el: Element, method: string, argument: string): boolean { + const nodes = [el, ...Array.from(el.querySelectorAll("*"))]; + if (method === "getElementById") { + const pattern = idPattern(argument); + return nodes.some((node) => pattern.test(node.getAttribute("id") ?? "\0")); + } + if (method === "getElementsByClassName") { + const wanted = argument.split(/\s+/).filter(Boolean); + return nodes.some((node) => wanted.every((name) => node.classList.contains(name))); + } + if (method === "getElementsByTagName") { + return argument === "*" || nodes.some((node) => node.localName === argument.toLowerCase()); + } + return matchesSelector(el, argument); } export function scriptLooksUp(document: Document, el: Element): boolean { const scripts = Array.from( document.querySelectorAll("script:not([src])"), (s) => s.textContent ?? "", - ); - const patterns = lookedUpPatterns(scripts.join("\n")); - if (patterns.length === 0) return false; - return [el, ...Array.from(el.querySelectorAll("[id]"))].some((node) => { - const id = node.getAttribute("id"); - return id !== null && patterns.some((pattern) => pattern.test(id)); - }); + ).join("\n"); + if (DYNAMIC_LOOKUP.test(scripts)) return true; + for (const [, method, , argument] of scripts.matchAll(LITERAL_LOOKUP)) { + if (lookupReaches(el, method ?? "", argument ?? "")) return true; + } + return false; } From ae5830a708108485d6c65564641f067464ffc3f3 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 23 Oct 2026 20:18:19 -0400 Subject: [PATCH 2/2] fix(sdk): a lookup argument that is not one closed literal keeps the delete hidden --- .../engine/apply-patches.scriptTargets.test.ts | 15 +++++++++++++++ packages/sdk/src/engine/scriptLookups.ts | 8 ++++---- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/packages/sdk/src/engine/apply-patches.scriptTargets.test.ts b/packages/sdk/src/engine/apply-patches.scriptTargets.test.ts index 601fd5b9013..15fa317060c 100644 --- a/packages/sdk/src/engine/apply-patches.scriptTargets.test.ts +++ b/packages/sdk/src/engine/apply-patches.scriptTargets.test.ts @@ -101,6 +101,8 @@ describe("a kept delete carries no media source", () => { it("leaves the sources of nodes that were not deleted", () => { expect(out()).toContain("live.png"); + }); +}); const lookupDoc = (script: string) => `
@@ -137,6 +139,19 @@ describe("a stored delete any script lookup could reach", () => { ).toBe(true); }); + it("is hidden when the argument is a literal continued by an expression", () => { + const loop = (call: string) => `for (var n = 1; n < 4; n++) ${call}.getTotalLength();`; + expect(kept(loop("document.getElementById('fiber-' + n)"), "hf-fiber")).toBe(true); + expect(kept(loop("document.querySelector('#fiber-' + n)"), "hf-fiber")).toBe(true); + expect(kept(loop("document.getElementById('fiber-'.concat(n))"), "hf-fiber")).toBe(true); + }); + + it("is hidden when a class or tag lookup carries a template placeholder", () => { + const call = (lookup: string) => `${lookup}[0].getBBox();`; + expect(kept(call("document.getElementsByClassName(`item-${n}`)"), "hf-halftone")).toBe(true); + expect(kept(call("document.getElementsByTagName(`${t}`)"), "hf-halftone")).toBe(true); + }); + it("is still removed when no lookup can reach it", () => { expect(kept("document.getElementById('other').getBBox();", "hf-plain")).toBe(false); }); diff --git a/packages/sdk/src/engine/scriptLookups.ts b/packages/sdk/src/engine/scriptLookups.ts index 75b0b5a724a..a99011e8bd6 100644 --- a/packages/sdk/src/engine/scriptLookups.ts +++ b/packages/sdk/src/engine/scriptLookups.ts @@ -1,9 +1,8 @@ // Whether an inline script could look the element up. A replayed delete of such a node is hidden, // not removed: a missing node makes the script throw before the timeline registers. -// A literal handed to a lookup; a template's ${...} stands for any text. const LITERAL_LOOKUP = - /(getElementById|getElementsByClassName|getElementsByTagName|querySelector(?:All)?)\s*\(\s*(["'`])([\s\S]*?)\2/g; + /(getElementById|getElementsByClassName|getElementsByTagName|querySelector(?:All)?)\s*\(\s*(["'`])([\s\S]*?)\2\s*(\S?)/g; // A lookup whose argument is a variable or an expression: it can name any node. const DYNAMIC_LOOKUP = /(?:getElementById|getElementsByClassName|getElementsByTagName|querySelector(?:All)?)\s*\(\s*(?!["'`)\s])/; @@ -25,6 +24,7 @@ function matchesSelector(el: Element, selector: string): boolean { } function lookupReaches(el: Element, method: string, argument: string): boolean { + if (PLACEHOLDER.test(argument)) return true; const nodes = [el, ...Array.from(el.querySelectorAll("*"))]; if (method === "getElementById") { const pattern = idPattern(argument); @@ -46,8 +46,8 @@ export function scriptLooksUp(document: Document, el: Element): boolean { (s) => s.textContent ?? "", ).join("\n"); if (DYNAMIC_LOOKUP.test(scripts)) return true; - for (const [, method, , argument] of scripts.matchAll(LITERAL_LOOKUP)) { - if (lookupReaches(el, method ?? "", argument ?? "")) return true; + for (const [, method, , argument, next] of scripts.matchAll(LITERAL_LOOKUP)) { + if (next !== ")" || lookupReaches(el, method ?? "", argument ?? "")) return true; } return false; }