fix(sdk): a stored delete a script still looks up is hidden when overrides replay, not removed (#6143)

* fix(sdk): a stored delete a script still looks up is hidden when overrides replay, not removed

* fix(sdk): shorten the script lookup comment

* fix(sdk): a replayed delete a script looks up leaves the model and serializes back hidden

* fix(sdk): shorten a comment line

* fix(sdk): a kept delete is marked hidden so the audio mixer drops it

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

@@ -0,0 +1,71 @@
+import { describe, expect, it } from "vitest";
+import { parseMutable } from "./model.js";
+import { applyOverrideSet, applyPatchesToDocument } from "./apply-patches.js";
+import { serializeDocument } from "./serialize.js";
+import { openComposition } from "../session.js";
+
+// The failing shape: the script dereferences the lookup unguarded, so a removed node throws.
+const HTML = `<!DOCTYPE html><html><body>
+<div data-composition-id="c" data-width="1920" data-height="1080">
+<svg><path data-hf-id="hf-arch" id="arch-path" d="M0 0"></path></svg>
+<audio data-hf-id="hf-aud" id="narration" src="a.mp3" data-start="0" data-duration="9"></audio>
+<div data-hf-id="hf-plain" id="plain">keep</div>
+</div>
+<script>document.getElementById('arch-path').getTotalLength(); document.getElementById('narration');</script>
+</body></html>`;
+
+describe("a stored delete a script looks up", () => {
+  it("leaves the live document, and serializes back hidden", () => {
+    const parsed = parseMutable(HTML);
+    applyOverrideSet(parsed, { "hf-arch": null });
+    expect(parsed.document.getElementById("arch-path")).toBeNull();
+    const out = serializeDocument(parsed);
+    expect(out).toContain('id="arch-path"');
+    expect(out).toContain("display: none");
+    expect(parsed.document.getElementById("arch-path")).toBeNull();
+  });
+
+  it("is no longer a clip once serialized, so its media is not mixed", () => {
+    const parsed = parseMutable(HTML);
+    applyOverrideSet(parsed, { "hf-aud": null });
+    const out = serializeDocument(parsed);
+    expect(out).toContain('id="narration"');
+    expect(out).not.toMatch(/id="narration"[^>]*data-start|data-start[^>]*id="narration"/);
+    expect(out).not.toContain('data-duration="9"');
+    // The audio mixer and the runtime skip [data-hidden]; display:none alone would still play the track.
+    expect(out).toMatch(/<audio[^>]*data-hidden/);
+  });
+
+  it("is still absent from the session model", async () => {
+    const comp = await openComposition(HTML, { overrides: { "hf-arch": null }, history: false });
+    expect(comp.getElements().some((el) => el.id === "hf-arch")).toBe(false);
+    expect(comp.getElement("hf-arch")).toBeNull();
+    expect(comp.serialize()).toContain('id="arch-path"');
+  });
+
+  it("is not written back twice once an undo has put it back", () => {
+    const parsed = parseMutable(HTML);
+    applyOverrideSet(parsed, { "hf-arch": null });
+    const [kept] = parsed.keptDeletes ?? [];
+    applyPatchesToDocument(parsed, [
+      {
+        op: "add",
+        path: "/elements/hf-arch",
+        value: { html: kept?.html, parentId: kept?.parentId, siblingIndex: kept?.siblingIndex },
+      },
+    ]);
+    expect(serializeDocument(parsed).match(/id="arch-path"/g)).toHaveLength(1);
+  });
+
+  it("is removed as before when no script looks it up", () => {
+    const parsed = parseMutable(HTML);
+    applyOverrideSet(parsed, { "hf-plain": null });
+    expect(serializeDocument(parsed)).not.toContain('id="plain"');
+  });
+
+  it("is removed by a plain patch, which undo and redo rely on", () => {
+    const parsed = parseMutable(HTML);
+    applyPatchesToDocument(parsed, [{ op: "remove", path: "/elements/hf-arch" }]);
+    expect(serializeDocument(parsed)).not.toContain('id="arch-path"');
+  });
+});

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

@@ -20,6 +20,7 @@ import {
   setStyleSheet,
 } from "./model.js";
 import { keyToPath, stylePath } from "./patches.js";
+import { scriptLooksUp } from "./scriptLookups.js";
 import {
   writeVariableDefault,
   clearVariableDefault,
@@ -158,12 +159,29 @@ export function applyOverrideSet(parsed: ParsedDocument, overrides: OverrideSet)
       patches.push({ op: "remove", path: stylePath(rootId, `--${key.slice("var.".length)}`) });
     }
   }
-  applyPatchesToDocument(parsed, patches);
+  applyPatchesToDocument(parsed, patches, { keepScriptTargets: true });
+}
+
+/** Keep a deleted node a script looks up: serialize writes it back hidden, so the script finds it. */
+function keepDelete(parsed: ParsedDocument, el: Element): void {
+  const parent = el.parentElement;
+  (parsed.keptDeletes ??= []).push({
+    hfId: el.getAttribute("data-hf-id") ?? "",
+    html: el.outerHTML,
+    parentId: parent?.getAttribute("data-hf-id") ?? null,
+    siblingIndex: parent ? Array.from(parent.children).indexOf(el) : 0,
+  });
+}
+
+interface ApplyOptions {
+  /** Keep a copy of each removed element an inline script looks up (stored deletes replayed onto a base). */
+  keepScriptTargets?: boolean;
 }
 
 export function applyPatchesToDocument(
   parsed: ParsedDocument,
   patches: readonly JsonPatchOp[],
+  options: ApplyOptions = {},
 ): void {
   for (const patch of patches) {
     const p = parsePath(patch.path);
@@ -171,12 +189,17 @@ export function applyPatchesToDocument(
     if (p.id && patch.path.startsWith("/elements/")) {
       p.id = p.id.replace(/~1/g, "/").replace(/~0/g, "~");
     }
-    applyOne(parsed, patch, p);
+    applyOne(parsed, patch, p, options);
   }
 }
 
 // fallow-ignore-next-line complexity
-function applyOne(parsed: ParsedDocument, patch: JsonPatchOp, p: ParsedPath): void {
+function applyOne(
+  parsed: ParsedDocument,
+  patch: JsonPatchOp,
+  p: ParsedPath,
+  options: ApplyOptions,
+): void {
   switch (p.type) {
     case "style": {
       const el = p.id ? findById(parsed.document, p.id) : null;
@@ -245,6 +268,8 @@ function applyOne(parsed: ParsedDocument, patch: JsonPatchOp, p: ParsedPath): vo
       if (!p.id) return;
       if (patch.op === "remove") {
         const el = findById(parsed.document, p.id);
+        if (el && options.keepScriptTargets && scriptLooksUp(parsed.document, el))
+          keepDelete(parsed, el);
         el?.remove();
       } else if (patch.op === "add" && patch.value) {
         const v = patch.value as { html: string; parentId: string | null; siblingIndex: number };

packages/sdk/src/engine/model.ts

@@ -20,6 +20,15 @@ export interface ParsedDocument {
   wrapped: boolean;
   /** ensureHfIds-stamped original HTML — used as fallback / diff base. */
   stamped: string;
+  /** Replayed deletes a script still looks up: left out of the live document, written back hidden by serialize. */
+  keptDeletes?: KeptDelete[];
+}
+
+export interface KeptDelete {
+  hfId: string;
+  html: string;
+  parentId: string | null;
+  siblingIndex: number;
 }
 
 export function parseMutable(html: string): ParsedDocument {

packages/sdk/src/engine/scriptLookups.test.ts

@@ -0,0 +1,55 @@
+import { describe, expect, it } from "vitest";
+import { parseHTML } from "linkedom";
+import { scriptLooksUp } from "./scriptLookups.js";
+
+function doc(body: string, script: string): Document {
+  return parseHTML(`<!DOCTYPE html><html><body>${body}<script>${script}</script></body></html>`)
+    .document as unknown as Document;
+}
+
+const get = (d: Document, id: string): Element => d.getElementById(id) as Element;
+
+describe("scriptLooksUp", () => {
+  it("finds a getElementById lookup", () => {
+    const d = doc(
+      `<path id="arch-path"></path><i id="plain"></i>`,
+      `document.getElementById('arch-path').getTotalLength()`,
+    );
+    expect(scriptLooksUp(d, get(d, "arch-path"))).toBe(true);
+    expect(scriptLooksUp(d, get(d, "plain"))).toBe(false);
+  });
+
+  it("finds a querySelector('#id') lookup, also inside a selector list", () => {
+    const d = doc(`<b id="a"></b><b id="b"></b>`, `document.querySelectorAll("#a, #zzz")`);
+    expect(scriptLooksUp(d, get(d, "a"))).toBe(true);
+    expect(scriptLooksUp(d, get(d, "b"))).toBe(false);
+  });
+
+  it("finds a node a template literal builds the id of", () => {
+    const d = doc(
+      `<b id="card-3-left"></b>`,
+      "for (let i=1;i<4;i++) document.getElementById(`card-${i}-left`).getBoundingClientRect()",
+    );
+    expect(scriptLooksUp(d, get(d, "card-3-left"))).toBe(true);
+  });
+
+  it("counts a lookup of a descendant", () => {
+    const d = doc(
+      `<g id="dots-group"><circle id="dot"></circle></g>`,
+      `document.querySelector('#dot').appendChild(x)`,
+    );
+    expect(scriptLooksUp(d, get(d, "dots-group"))).toBe(true);
+  });
+
+  it("does not match a longer or shorter id", () => {
+    const d = doc(`<b id="arch"></b>`, `document.getElementById('arch-path')`);
+    expect(scriptLooksUp(d, get(d, "arch"))).toBe(false);
+  });
+
+  it("ignores a script with a src and a document with no script lookup", () => {
+    const d = parseHTML(
+      `<!DOCTYPE html><html><body><b id="x"></b><script src="a.js">getElementById('x')</script></body></html>`,
+    ).document as unknown as Document;
+    expect(scriptLooksUp(d, get(d, "x"))).toBe(false);
+  });
+});

packages/sdk/src/engine/scriptLookups.ts

@@ -0,0 +1,38 @@
+// Whether an inline script looks the element up by id. 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 PLACEHOLDER = /\$\{[^}]*\}/;
+
+function idPattern(text: string): RegExp {
+  const parts = text.split(new RegExp(PLACEHOLDER.source, "g"));
+  const escaped = parts.map((part) => part.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"));
+  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));
+  }
+  return patterns;
+}
+
+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));
+  });
+}

packages/sdk/src/engine/serialize.ts

@@ -5,7 +5,45 @@
  * Phase 3b: GSAP script section will use the meriyah/offset-splice path once available.
  */
 
-import type { ParsedDocument } from "./model.js";
+import { findById, setElementStyles, type KeptDelete, type ParsedDocument } from "./model.js";
+
+// A kept delete is not a clip: without these the renderer would still schedule its media and stretch the film.
+const CLIP_ATTRIBUTES = ["data-start", "data-duration", "data-end", "data-track-index"];
+
+/** The deleted node, hidden and no longer a clip, so a script that looks it up still finds it. */
+function hiddenCopy(doc: Document, kept: KeptDelete): Element | null {
+  const holder = doc.createElement("div");
+  holder.innerHTML = kept.html;
+  const node = holder.firstElementChild;
+  if (!node) return null;
+  for (const el of [node, ...Array.from(node.querySelectorAll("*"))]) {
+    for (const name of CLIP_ATTRIBUTES) el.removeAttribute(name);
+  }
+  setElementStyles(node, { display: "none" });
+  node.setAttribute("data-hidden", "");
+  return node;
+}
+
+/** Put the hidden copy back where the node was; null when an undo already made it live or its parent is gone. */
+function placeKeptDelete(doc: Document, kept: KeptDelete): Element | null {
+  if (kept.hfId && findById(doc, kept.hfId)) return null;
+  const node = hiddenCopy(doc, kept);
+  const parent = kept.parentId ? findById(doc, kept.parentId) : (doc.body as Element | null);
+  if (!node || !parent) return null;
+  parent.insertBefore(node, parent.children[kept.siblingIndex] ?? null);
+  return node;
+}
+
+function withKeptDeletes<T>(parsed: ParsedDocument, serialize: () => T): T {
+  const placed = (parsed.keptDeletes ?? []).flatMap(
+    (kept) => placeKeptDelete(parsed.document, kept) ?? [],
+  );
+  try {
+    return serialize();
+  } finally {
+    for (const node of placed) node.remove();
+  }
+}
 
 /**
  * Serialize the live document back to HTML.
@@ -14,9 +52,11 @@ import type { ParsedDocument } from "./model.js";
  * If the original input had a full HTML shell (wrapped=false), returns the full document.
  */
 export function serializeDocument(parsed: ParsedDocument): string {
-  const doc = parsed.document;
-  if (parsed.wrapped) {
-    return (doc.body as HTMLBodyElement).innerHTML ?? "";
-  }
-  return `<!DOCTYPE html>\n${doc.documentElement.outerHTML}`;
+  return withKeptDeletes(parsed, () => {
+    const doc = parsed.document;
+    if (parsed.wrapped) {
+      return (doc.body as HTMLBodyElement).innerHTML ?? "";
+    }
+    return `<!DOCTYPE html>\n${doc.documentElement.outerHTML}`;
+  });
 }