diff --git a/packages/core/src/compiler/compositionScoping.ts b/packages/core/src/compiler/compositionScoping.ts index c1fc996200..62f19f9e5d 100644 --- a/packages/core/src/compiler/compositionScoping.ts +++ b/packages/core/src/compiler/compositionScoping.ts @@ -1,4 +1,5 @@ import postcss, { type AtRule, type Node, type Rule } from "postcss"; +import { replaceSelectorIdTokens } from "./selectorIdTokens"; const AUTHORED_ROOT_ID_ATTR = "data-hf-authored-id"; const INNER_ROOT_ATTR = "data-hf-inner-root"; @@ -23,68 +24,13 @@ function getAuthoredRootIdSelectorForms(authoredRootId: string): string[] { return Array.from(new Set([trimmed, escapeCssIdentifier(trimmed)])).filter(Boolean); } -function isSelectorNameChar(char: string | undefined): boolean { - return !!char && /[\w-]/.test(char); -} - function replaceAuthoredRootIdSelectors( selector: string, authoredRootId: string, replacement: string, ): string { - const forms = getAuthoredRootIdSelectorForms(authoredRootId).sort((a, b) => b.length - a.length); - if (forms.length === 0) return selector; - - let result = ""; - let bracketDepth = 0; - let quote: '"' | "'" | null = null; - - for (let index = 0; index < selector.length; index += 1) { - const char = selector[index]; - const previousChar = index > 0 ? selector[index - 1] : ""; - - if (quote) { - result += char; - if (char === quote && previousChar !== "\\") { - quote = null; - } - continue; - } - - if (char === '"' || char === "'") { - quote = char; - result += char; - continue; - } - - if (char === "[") { - bracketDepth += 1; - result += char; - continue; - } - - if (char === "]") { - bracketDepth = Math.max(0, bracketDepth - 1); - result += char; - continue; - } - - if (char === "#" && bracketDepth === 0) { - const matchedForm = forms.find((form) => selector.startsWith(form, index + 1)); - if (matchedForm) { - const nextChar = selector[index + 1 + matchedForm.length]; - if (!isSelectorNameChar(nextChar)) { - result += replacement; - index += matchedForm.length; - continue; - } - } - } - - result += char; - } - - return result; + const forms = getAuthoredRootIdSelectorForms(authoredRootId); + return replaceSelectorIdTokens(selector, forms, () => replacement); } function normalizeAuthoredRootIdSelector(selector: string, authoredRootId?: string | null): string { diff --git a/packages/core/src/compiler/inlineSubCompositions.test.ts b/packages/core/src/compiler/inlineSubCompositions.test.ts index a05e993fab..d94bbe1c5c 100644 --- a/packages/core/src/compiler/inlineSubCompositions.test.ts +++ b/packages/core/src/compiler/inlineSubCompositions.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { parseHTML } from "linkedom"; import { inlineSubCompositions } from "./inlineSubCompositions"; import { readDeclaredDefaults, parseHostVariableValues } from "../runtime/getVariables"; +import { assignBundledRuntimeCompositionIds } from "./htmlBundler"; // Fixtures reference GSAP CDN but are never loaded in a real browser — resolveHtml is mocked. @@ -676,3 +677,135 @@ describe("inlineSubCompositions – sub-composition asset paths", () => { ); }); }); + +describe("inlineSubCompositions – #3490 nested SVG id collisions", () => { + // Same shape as the issue repro: a composition-scoped clipPath, a + // target, and a CSS filter, all hardcoded ids a catalog block or a scene + // author has no reason to think are unsafe to repeat. + function svgScene(compId: string, color: string): string { + return `
+ + + + + + +
+ +
`; + } + + 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("does not rename svg ids that have no native reference (url/href)", () => { + const { document, idMap } = namespace( + '', + "scene-a", + ); + expect(idMap.size).toBe(0); + expect(document.querySelector("path")!.getAttribute("id")).toBe("cut-1"); + }); + + it("renames a natively-referenced svg 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("detects url(#id) references inside style elements", () => { + const { document, idMap } = namespace( + '' + + "", + "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 = + '' + + '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..f3dc8a980a --- /dev/null +++ b/packages/core/src/compiler/svgIdNamespacing.ts @@ -0,0 +1,276 @@ +/** + * 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 `