From 31274556ee30e6d42fd869dfbcdcdb8fde8601b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Wed, 26 Aug 2026 00:25:17 +0000 Subject: [PATCH 1/2] fix(core): namespace SVG ids during composition inline to prevent cross-scene collisions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two nested compositions that each declare their own SVG ids (``, ``, ``) are legal per file and pass `hyperframes check`, but collide once both are inlined into one render/preview document. `url(#id)` funcrefs (`clip-path`, `filter`, `mask`, `fill`, `stroke`, `marker-start/mid/end`) and fragment `href`/`xlink:href` refs (``) are resolved by the browser's native SVG/CSS engine, which always binds to the first matching id in document order — so the later scene either clips to nothing or paints the earlier scene's content. `getElementById` was already scoped per composition in #646, and media pipeline ids got a parallel `data-hf-render-id` attribute in #3340. Neither covers this: native `url(#id)`/`href="#id"` resolution can't be intercepted by a JS proxy, so the `id` attribute itself has to become document-unique. Add `svgIdNamespacing.ts`: during `inlineSubCompositions`, every id declared on an ``-subtree element is prefixed with the composition's document-unique runtime id, and every same-document reference to it — DOM attributes (`href`, `xlink:href`, any `url(#id)` value including inside `style`) and the composition's own extracted `
+ + + + + + +
+ +
`; + } + + function inlineTwoScenes( + sources: Record, + hostEntries: Array<{ compId: string; src: string }>, + ) { + const hostsMarkup = hostEntries + .map( + ({ compId, src }, i) => + `
`, + ) + .join("\n"); + const { document } = parseHTML(` + +
${hostsMarkup}
+`); + const hosts = Array.from(document.querySelectorAll("[data-composition-src]")); + const hostIdentityMap = assignBundledRuntimeCompositionIds(hosts); + const result = inlineSubCompositions(document, hosts, { + resolveHtml: (src) => sources[src] ?? null, + parseHtml: (html) => parseHTML(html).document, + hostIdentityMap, + }); + return { document, result }; + } + + it("gives two sibling scenes reusing #clip/#shape/#fx distinct, non-colliding ids", () => { + const { document, result } = inlineTwoScenes( + { "scene-a.html": svgScene("scene-a", "red"), "scene-b.html": svgScene("scene-b", "blue") }, + [ + { compId: "scene-a", src: "scene-a.html" }, + { compId: "scene-b", src: "scene-b.html" }, + ], + ); + + // No literal "clip"/"shape"/"fx" id survives verbatim, and no id repeats + // across the two scenes — the exact document-order collision the browser + // resolves incorrectly before this fix. + const allIds = Array.from(document.querySelectorAll("[id]")).map((el) => el.getAttribute("id")); + expect(allIds.filter((id) => id === "clip")).toHaveLength(0); + expect(new Set(allIds).size).toBe(allIds.length); + + const [gA, gB] = Array.from(document.querySelectorAll("g.clipped")); + const [useA, useB] = Array.from(document.querySelectorAll("use.shape")); + const [fxA, fxB] = Array.from(document.querySelectorAll(".fx-target")); + + const clipA = gA!.getAttribute("clip-path"); + const clipB = gB!.getAttribute("clip-path"); + expect(clipA).toMatch(/^url\(#.*clip\)$/); + expect(clipA).not.toBe(clipB); + + const hrefA = useA!.getAttribute("href"); + const hrefB = useB!.getAttribute("href"); + expect(hrefA).toMatch(/^#.*shape$/); + expect(hrefA).not.toBe(hrefB); + + // /clip-path must resolve to the id actually declared in THIS + // scene's , not merely to a unique-looking string. + const clipAId = clipA!.slice("url(#".length, -1); + const shapeAId = hrefA!.slice(1); + expect(document.getElementById(clipAId)?.closest("svg")).toBeTruthy(); + expect(document.getElementById(shapeAId)?.closest("svg")).toBeTruthy(); + + const styleFxA = fxA!.getAttribute("style"); + const styleFxB = fxB!.getAttribute("style"); + expect(styleFxA).not.toBe(styleFxB); + + // The CSS text (`

hello

", "scene-a"); + expect(idMap.size).toBe(0); + }); + + it("is a no-op when namespace is empty (anonymous host)", () => { + const { document, idMap } = namespace('', ""); + expect(idMap.size).toBe(0); + expect(document.querySelector("clipPath")!.getAttribute("id")).toBe("clip"); + }); + + it("renames an svg element id and records the authored id", () => { + const { document, idMap } = namespace( + '', + "scene-a", + ); + const clipPath = document.querySelector("clipPath")!; + expect(idMap.get("clip")).toBe("scene-a--clip"); + expect(clipPath.getAttribute("id")).toBe("scene-a--clip"); + expect(clipPath.getAttribute(SVG_AUTHORED_ID_ATTR)).toBe("clip"); + }); + + it("rewrites a clip-path url() presentation attribute", () => { + const { document } = namespace( + '
', + "scene-a", + ); + expect(document.querySelector("div")!.getAttribute("clip-path")).toBe("url(#scene-a--clip)"); + }); + + it("rewrites filter/mask/fill/stroke/marker url() references", () => { + const html = ` + + + + + + +
+ + `; + const { document } = namespace(html, "scene-b"); + const div = document.querySelector("div")!; + expect(div.getAttribute("style")).toBe("filter:url(#scene-b--fx); mask: url('#scene-b--msk')"); + const rect = document.querySelector("rect")!; + expect(rect.getAttribute("fill")).toBe("url(#scene-b--grad)"); + expect(rect.getAttribute("stroke")).toBe("url(#scene-b--grad)"); + expect(rect.getAttribute("marker-start")).toBe("url(#scene-b--arrow)"); + expect(rect.getAttribute("marker-end")).toBe("url(#scene-b--arrow)"); + }); + + it("rewrites and xlink:href fragment refs", () => { + const html = + ''; + const { document } = namespace(html, "scene-c"); + const uses = [...document.querySelectorAll("use")]; + expect(uses[0]!.getAttribute("href")).toBe("#scene-c--shape"); + expect(uses[1]!.getAttribute("xlink:href")).toBe("#scene-c--shape"); + }); + + it("leaves unrelated hrefs and non-fragment urls untouched", () => { + const html = + '' + + 'link' + + '
'; + const { document } = namespace(html, "scene-d"); + expect(document.querySelector("a")!.getAttribute("href")).toBe("https://example.com/#clip"); + expect(document.querySelector("div")!.getAttribute("style")).toBe("background:url(image.png)"); + }); + + it("disambiguates two composition instances reusing the same catalog block ids", () => { + // Same shape as the #3490 repro: two sibling scenes each author their + // own #clip/#shape/#fx, and both must keep resolving to their OWN scene. + const sceneA = namespace( + '' + + '' + + '' + + '
', + "scene-a", + ); + const sceneB = namespace( + '' + + '' + + '' + + '
', + "scene-b", + ); + + expect(sceneA.document.querySelector("g")!.getAttribute("clip-path")).toBe( + "url(#scene-a--clip)", + ); + expect(sceneB.document.querySelector("g")!.getAttribute("clip-path")).toBe( + "url(#scene-b--clip)", + ); + // The two namespaced ids are document-unique, so merging both fragments + // into one document (what inlining actually does) no longer collides. + expect(sceneA.idMap.get("clip")).not.toBe(sceneB.idMap.get("clip")); + }); +}); + +describe("rewriteSvgIdReferencesInCss", () => { + it("is a no-op with an empty map", () => { + const css = "#clip { fill: red; }"; + expect(rewriteSvgIdReferencesInCss(css, new Map())).toBe(css); + }); + + it("rewrites a bare id selector to the namespaced id", () => { + const idMap = new Map([["clip", "scene-a--clip"]]); + const css = "#clip rect { fill: red; }"; + expect(rewriteSvgIdReferencesInCss(css, idMap)).toContain("#scene-a--clip rect"); + }); + + it("rewrites a url(#id) declaration value", () => { + const idMap = new Map([["fx", "scene-a--fx"]]); + const css = ".glow { filter: url(#fx); }"; + expect(rewriteSvgIdReferencesInCss(css, idMap)).toContain("filter: url(#scene-a--fx)"); + }); + + it("does not touch an id selector for an id outside the map", () => { + const idMap = new Map([["clip", "scene-a--clip"]]); + const css = "#other { fill: red; }"; + expect(rewriteSvgIdReferencesInCss(css, idMap)).toBe(css); + }); + + it("does not confuse a short id with a longer one that starts with it", () => { + const idMap = new Map([ + ["clip", "scene-a--clip"], + ["clip2", "scene-a--clip2"], + ]); + const css = "#clip2 { fill: red; }"; + const result = rewriteSvgIdReferencesInCss(css, idMap); + expect(result).toContain("#scene-a--clip2"); + expect(result).not.toContain("scene-a--clipscene-a"); + }); +}); diff --git a/packages/core/src/compiler/svgIdNamespacing.ts b/packages/core/src/compiler/svgIdNamespacing.ts new file mode 100644 index 0000000000..529d4da8d8 --- /dev/null +++ b/packages/core/src/compiler/svgIdNamespacing.ts @@ -0,0 +1,229 @@ +/** + * Namespace SVG element ids during sub-composition inline. + * + * An element `id` is only unique within one composition FILE. The assembled + * render/preview document is the inlined union of every file, so two nested + * scenes that each declare their own ``, `` or `` — legal per file, and invisible to + * `hyperframes check` — collide once inlined. Catalog blocks make this easy + * to hit by accident: a block's markup hardcodes ids like + * `url(#tracing-beam-glow)`, so using the SAME block twice collides without + * the author duplicating anything by hand. + * + * `getElementById` was already scoped per composition in #646 (see + * `compositionScoping.ts`'s `__hfGetElementById` shim) and media pipeline ids + * were disambiguated with a parallel `data-hf-render-id` attribute in #3340 + * (`mediaRenderIds.ts`). Neither covers `url(#id)` funcrefs (`clip-path`, + * `filter`, `mask`, `fill`, `stroke`, `marker-start/mid/end`, `cursor`, + * `mask-image`, …) or bare `#id` fragment refs (``, ``, ``/`` `href`): those are resolved by the + * BROWSER'S NATIVE SVG/CSS engine, which always binds to the first element in + * DOCUMENT ORDER carrying that literal `id` attribute. No JS proxy can + * intercept native resolution, so unlike the two fixes above, this one + * actually renames the `id` attribute. + * + * Renaming a real `id` would break an inline script's own + * `document.getElementById(originalId)` or a same-composition CSS `#id` + * selector, so every renamed element keeps its original id on + * `data-hf-authored-id` — the exact attribute `__hfGetElementById`'s fallback + * already checks (introduced in #646 for the composition ROOT's own id; + * reused here for any descendant), and `rewriteSvgIdReferencesInCss` below + * rewrites the composition's own `", + "scene-a", + ); + expect(idMap.get("rough-filter")).toBe("scene-a--rough-filter"); + expect(document.querySelector("filter")!.getAttribute("id")).toBe("scene-a--rough-filter"); + }); + it("leaves unrelated hrefs and non-fragment urls untouched", () => { const html = '' + diff --git a/packages/core/src/compiler/svgIdNamespacing.ts b/packages/core/src/compiler/svgIdNamespacing.ts index 529d4da8d8..f3dc8a980a 100644 --- a/packages/core/src/compiler/svgIdNamespacing.ts +++ b/packages/core/src/compiler/svgIdNamespacing.ts @@ -131,26 +131,73 @@ interface SvgIdScopeLike { * host has no unique identity to prefix with, the same guard * `scopeCssToComposition` and `wrapScopedCompositionScript` already apply. */ +function collectUrlHashRefsFromText( + text: string, + filter: ReadonlySet, + out: Set, +): void { + let m: RegExpExecArray | null; + URL_HASH_REF_RE.lastIndex = 0; + while ((m = URL_HASH_REF_RE.exec(text))) { + const refId = m[3]!; + if (filter.has(refId)) out.add(refId); + } +} + +/** Ids referenced by native browser resolution — `url(#id)` funcrefs and + * bare `href="#id"` fragment refs — as opposed to JavaScript-only refs + * (e.g. GSAP's `tl.to("#cut-1")`). Global libraries access `document` + * directly and bypass the composition-scoped querySelector Proxy, so only + * natively-referenced ids are safe to rename. */ +function collectHrefFragmentRef(attr: Attr, filter: ReadonlySet, out: Set): void { + if (isHrefAttrName(attr.name) && attr.value.startsWith("#")) { + const id = attr.value.slice(1); + if (filter.has(id)) out.add(id); + } +} + +function collectNativelyReferencedIds( + root: SvgIdScopeLike, + candidates: readonly Element[], + svgIds: ReadonlySet, +): Set { + const referenced = new Set(); + for (const el of candidates) { + for (const attr of el.attributes ? Array.from(el.attributes) : []) { + if (!attr.value) continue; + collectHrefFragmentRef(attr, svgIds, referenced); + if (attr.value.includes("url(")) collectUrlHashRefsFromText(attr.value, svgIds, referenced); + } + } + for (const styleEl of root.querySelectorAll("style")) { + const text = (styleEl as unknown as { textContent: string | null }).textContent; + if (text && text.includes("url(")) collectUrlHashRefsFromText(text, svgIds, referenced); + } + return referenced; +} + export function namespaceSvgIds(root: SvgIdScopeLike, namespace: string): Map { const idMap = new Map(); if (!namespace) return idMap; + const svgIds = new Set(); for (const el of root.querySelectorAll("svg [id], svg[id]")) { - const originalId = el.getAttribute(ID_ATTR); - if (!originalId || idMap.has(originalId)) continue; - idMap.set(originalId, buildNamespacedId(namespace, originalId)); + const id = el.getAttribute(ID_ATTR); + if (id) svgIds.add(id); } - if (idMap.size === 0) return idMap; + if (svgIds.size === 0) return idMap; const candidates: Element[] = [...root.querySelectorAll("*")]; - // The scan root itself may carry a reference (e.g. a host `
` wrapping the `` that defines it) — - // `querySelectorAll("*")` only returns descendants, so include it - // explicitly when it exposes the attribute methods (an Element, not a - // Document, which has no id and nothing to reference from itself). if (typeof root.getAttribute === "function" && typeof root.setAttribute === "function") { candidates.unshift(root as unknown as Element); } + + const nativelyReferenced = collectNativelyReferencedIds(root, candidates, svgIds); + if (nativelyReferenced.size === 0) return idMap; + + for (const id of nativelyReferenced) { + idMap.set(id, buildNamespacedId(namespace, id)); + } for (const el of candidates) { const currentId = el.getAttribute(ID_ATTR); if (currentId && idMap.has(currentId)) {