fix(sdk): a delete is kept hidden when any script lookup could reach it, not only an id

packages/sdk/src/engine/apply-patches.scriptTargets.test.ts

@@ -103,3 +103,56 @@ describe("a kept delete carries no media source", () => {
     expect(out()).toContain("live.png");
   });
 });
+
+const lookupDoc = (script: string) => `<!DOCTYPE html><html><body>
+<div data-composition-id="c" data-width="1920" data-height="1080">
+<svg data-hf-id="hf-halftone" class="halftone-svg"><path d="M0 0"></path></svg>
+<div data-hf-id="hf-fiber" id="fiber-1"></div>
+<div data-hf-id="hf-plain" id="plain">keep</div>
+</div>
+<script>${script}</script>
+</body></html>`;
+
+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 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);
+  });
+});

packages/sdk/src/engine/scriptLookups.ts

@@ -1,9 +1,11 @@
-// 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;
+const LITERAL_LOOKUP =
+  /(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])/;
 const PLACEHOLDER = /\$\{[^}]*\}/;
 
 function idPattern(text: string): RegExp {
@@ -12,27 +14,40 @@ 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 {
+  if (PLACEHOLDER.test(argument)) return true;
+  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, next] of scripts.matchAll(LITERAL_LOOKUP)) {
+    if (next !== ")" || lookupReaches(el, method ?? "", argument ?? "")) return true;
+  }
+  return false;
 }