diff --git a/packages/core/src/generators/hyperframes.ts b/packages/core/src/generators/hyperframes.ts index 68307921b..9028013c1 100644 --- a/packages/core/src/generators/hyperframes.ts +++ b/packages/core/src/generators/hyperframes.ts @@ -447,6 +447,7 @@ function generateZoomGsapAnimations( function generateElementHtml(element: TimelineElement, keyframes?: Keyframe[]): string { const baseAttrs = [ `id="${element.id}"`, + `data-hf-id="${element.id}"`, `data-start="${element.startTime}"`, `data-end="${element.startTime + element.duration}"`, `data-layer="${element.zIndex}"`, diff --git a/packages/core/src/parsers/htmlParser.test.ts b/packages/core/src/parsers/htmlParser.test.ts index 949943bd0..2d786a7d3 100644 --- a/packages/core/src/parsers/htmlParser.test.ts +++ b/packages/core/src/parsers/htmlParser.test.ts @@ -26,13 +26,13 @@ describe("parseHtml", () => { const result = parseHtml(html); expect(result.elements).toHaveLength(2); - expect(result.elements[0].id).toBe("text1"); + expect(result.elements[0].id).toMatch(/^hf-[a-z0-9]{4}$/); expect(result.elements[0].startTime).toBe(0); expect(result.elements[0].duration).toBe(5); expect(result.elements[0].name).toBe("Title"); expect(result.elements[0].type).toBe("text"); - expect(result.elements[1].id).toBe("text2"); + expect(result.elements[1].id).toMatch(/^hf-[a-z0-9]{4}$/); expect(result.elements[1].startTime).toBe(2); expect(result.elements[1].duration).toBe(5); }); @@ -53,7 +53,7 @@ describe("parseHtml", () => { expect(result.elements).toHaveLength(1); expect(result.elements[0].type).toBe("composition"); - expect(result.elements[0].id).toBe("comp1"); + expect(result.elements[0].id).toMatch(/^hf-[a-z0-9]{4}$/); if (result.elements[0].type === "composition") { expect(result.elements[0].compositionId).toBe("abc123"); expect(result.elements[0].src).toBe("/compositions/abc123"); @@ -76,20 +76,20 @@ describe("parseHtml", () => { expect(result.elements).toHaveLength(3); - const video = result.elements.find((e) => e.id === "vid1"); + const video = result.elements.find((e) => e.type === "video"); expect(video).toBeDefined(); - expect(video?.type).toBe("video"); + expect(video?.id).toMatch(/^hf-[a-z0-9]{4}$/); if (video?.type === "video") { expect(video.src).toBe("video.mp4"); } - const audio = result.elements.find((e) => e.id === "aud1"); + const audio = result.elements.find((e) => e.type === "audio"); expect(audio).toBeDefined(); - expect(audio?.type).toBe("audio"); + expect(audio?.id).toMatch(/^hf-[a-z0-9]{4}$/); - const img = result.elements.find((e) => e.id === "img1"); + const img = result.elements.find((e) => e.type === "image"); expect(img).toBeDefined(); - expect(img?.type).toBe("image"); + expect(img?.id).toMatch(/^hf-[a-z0-9]{4}$/); }); it("handles missing attributes gracefully", () => { @@ -123,7 +123,7 @@ describe("parseHtml", () => { const result = parseHtml(html); expect(result.elements).toHaveLength(1); - expect(result.elements[0].id).toMatch(/^element-\d+$/); + expect(result.elements[0].id).toMatch(/^hf-[a-z0-9]{4}$/); }); it("extracts GSAP script from script tags", () => { @@ -398,9 +398,12 @@ describe("parseHtml", () => { `; const result = parseHtml(html); - expect(result.keyframes["text1"]).toBeDefined(); - expect(result.keyframes["text1"]).toHaveLength(2); - expect(result.keyframes["text1"][0].id).toBe("kf1"); + const elId = result.elements[0]?.id ?? ""; + expect(elId).toMatch(/^hf-[a-z0-9]{4}$/); + const kfs = result.keyframes[elId]; + expect(kfs).toBeDefined(); + expect(kfs).toHaveLength(2); + expect(kfs[0].id).toBe("kf1"); }); it("parses stage zoom keyframes", () => { diff --git a/packages/core/src/parsers/htmlParser.ts b/packages/core/src/parsers/htmlParser.ts index 92864e04e..47859badf 100644 --- a/packages/core/src/parsers/htmlParser.ts +++ b/packages/core/src/parsers/htmlParser.ts @@ -11,6 +11,7 @@ import type { CompositionVariable, } from "../core.types"; import { validateCompositionGsap } from "./gsapSerialize"; +import { ensureHfIds } from "./hfIds.js"; import type { ValidationResult } from "../core.types"; const MEDIA_TYPES = new Set(["video", "image", "audio"]); @@ -156,8 +157,9 @@ function resolveResolutionFromDimensions(width: number, height: number): CanvasR } export function parseHtml(html: string): ParsedHtml { + const withIds = ensureHfIds(html); const parser = new DOMParser(); - const doc = parser.parseFromString(html, "text/html"); + const doc = parser.parseFromString(withIds, "text/html"); const elements: TimelineElement[] = []; const keyframes: Record = {}; @@ -190,7 +192,16 @@ export function parseHtml(html: string): ParsedHtml { duration = 5; } - const id = el.id || `element-${++idCounter}`; + // R1: stable hf- id minted by ensureHfIds above; clips just read it. + // Legacy/migration note: ensureHfIds pins a pre-existing `data-hf-id`, and + // the generator emits `data-hf-id="${element.id}"`. So a clip authored + // before R1 with `id="my-title"` round-trips as `data-hf-id="my-title"` — + // a non-`hf-`-shaped but still stable, exact-match handle. This is safe + // indefinitely: targeting uses exact `[data-hf-id="…"]` match (it does not + // require the hf- prefix). ensureHfIds skips elements that already carry + // data-hf-id, so legacy values are NOT re-minted automatically — they + // persist until the user re-saves the composition through Studio. Not a bug. + const id = el.getAttribute("data-hf-id") || el.id || `element-${++idCounter}`; const name = getElementName(el); const zIndex = getZIndex(el); diff --git a/packages/core/src/parsers/stableIds.test.ts b/packages/core/src/parsers/stableIds.test.ts index a3887f04f..4656f98c8 100644 --- a/packages/core/src/parsers/stableIds.test.ts +++ b/packages/core/src/parsers/stableIds.test.ts @@ -18,7 +18,7 @@ import { serialize } from "./test-utils.js"; describe("T2 — stable element ids (spec for R1)", () => { // --- Spec (red until R1) --- - it.fails("[spec] elements without an id get a hf- prefixed id at parse", () => { + it("[spec] elements without an id get a hf- prefixed id at parse", () => { const html = `
Text
@@ -29,7 +29,7 @@ describe("T2 — stable element ids (spec for R1)", () => { } }); - it.fails("[spec] generated hf- ids match /^hf-[a-z0-9]{4}$/", () => { + it("[spec] generated hf- ids match /^hf-[a-z0-9]{4}$/", () => { const html = `
X
@@ -41,7 +41,7 @@ describe("T2 — stable element ids (spec for R1)", () => { } }); - it.fails("[spec] adding an element before existing ones does not change existing ids", () => { + it("[spec] adding an element before existing ones does not change existing ids", () => { const base = `
A
B
@@ -62,12 +62,12 @@ describe("T2 — stable element ids (spec for R1)", () => { // --- Baseline (already pass, must not regress) --- - it("elements with an existing id keep it unchanged", () => { + it("existing data-hf-id is pinned and becomes the clip id (never re-minted)", () => { const html = `
-
Hi
+
Hi
`; const { elements } = parseHtml(html); - expect(elements.some((e) => e.id === "my-title")).toBe(true); + expect(elements.some((e) => e.id === "hf-anch")).toBe(true); }); it("ids are deterministic: same input produces same ids on re-parse", () => { diff --git a/packages/core/src/studio-api/helpers/sourceMutation.test.ts b/packages/core/src/studio-api/helpers/sourceMutation.test.ts index c23b45911..8915d411d 100644 --- a/packages/core/src/studio-api/helpers/sourceMutation.test.ts +++ b/packages/core/src/studio-api/helpers/sourceMutation.test.ts @@ -368,13 +368,90 @@ describe("probeElementInSource", () => { // Covers the same surface as T3 (Studio sourcePatcher) — Core sourceMutation supports // all patch types (inline-style, attribute, text-content) via patchElementInHtml. describe("T7 — data-hf-id targeting (spec for R1)", () => { - it.todo("updates inline style by data-hf-id when no HTML id attribute is present"); + it("updates inline style by data-hf-id when no HTML id attribute is present", () => { + const source = `

Hello

`; + const { html, matched } = patchElementInHtml(source, { hfId: "hf-x7k2" }, [ + { type: "inline-style", property: "color", value: "blue" }, + ]); + expect(matched).toBe(true); + expect(html).toMatch(/color:\s*blue/); + expect(html).toContain('data-hf-id="hf-x7k2"'); + }); - it.todo("updates text content by data-hf-id"); + it("updates text content by data-hf-id", () => { + const source = `

Old text

`; + const { html, matched } = patchElementInHtml(source, { hfId: "hf-a1b2" }, [ + { type: "text-content", property: "", value: "New text" }, + ]); + expect(matched).toBe(true); + expect(html).toContain("New text"); + }); - it.todo("updates attribute by data-hf-id"); + it("updates attribute by data-hf-id", () => { + const source = `
`; + const { html, matched } = patchElementInHtml(source, { hfId: "hf-c3d4" }, [ + { type: "attribute", property: "start", value: "2.5" }, + ]); + expect(matched).toBe(true); + expect(html).toContain('data-start="2.5"'); + }); - it.todo("data-hf-id attribute survives the patch (can be targeted again)"); + it("data-hf-id attribute survives the patch (can be targeted again)", () => { + const source = `

Hello

`; + const { html } = patchElementInHtml(source, { hfId: "hf-x7k2" }, [ + { type: "inline-style", property: "color", value: "blue" }, + ]); + expect(html).toContain('data-hf-id="hf-x7k2"'); + }); - it.todo("hfId lookup falls through to selector when hfId is not found in the document"); + it("hfId lookup falls through to selector when hfId is not found in the document", () => { + const source = `

Hello

`; + const { html, matched } = patchElementInHtml( + source, + { hfId: "hf-missing", selector: ".headline" }, + [{ type: "inline-style", property: "color", value: "blue" }], + ); + expect(matched).toBe(true); + expect(html).toMatch(/color:\s*blue/); + }); + + it("does not break out of the selector on a crafted hfId (CSS injection guard)", () => { + // A value with a quote/bracket must be escaped, not injected — it should + // simply match nothing and leave the source untouched, never throw. + const source = `

A

B

`; + const evil = `x"] , [class="victim`; + const run = () => + patchElementInHtml(source, { hfId: evil }, [ + { type: "text-content", property: "textContent", value: "HACKED" }, + ]); + expect(run).not.toThrow(); + const { html, matched } = run(); + expect(matched).toBe(false); + expect(html).toBe(source); + expect(html).not.toContain("HACKED"); + }); + + // The Studio edit path targets by id/selector (it never sends hfId). Once a + // persisted data-hf-id exists in source, those edits must NOT strip it — else + // the stable handle is destroyed by the next edit. This is the preservation + // guarantee the write-back design depends on. + it("preserves an existing data-hf-id when the element is patched by id", () => { + const source = `

Hello

`; + const { html, matched } = patchElementInHtml(source, { id: "hero" }, [ + { type: "inline-style", property: "color", value: "blue" }, + ]); + expect(matched).toBe(true); + expect(html).toMatch(/color:\s*blue/); + expect(html).toContain('data-hf-id="hf-x7k2"'); + }); + + it("preserves an existing data-hf-id when the element is patched by selector", () => { + const source = `

Old

`; + const { html, matched } = patchElementInHtml(source, { selector: ".body" }, [ + { type: "text-content", property: "textContent", value: "New" }, + ]); + expect(matched).toBe(true); + expect(html).toContain("New"); + expect(html).toContain('data-hf-id="hf-a1b2"'); + }); }); diff --git a/packages/core/src/studio-api/helpers/sourceMutation.ts b/packages/core/src/studio-api/helpers/sourceMutation.ts index 8cdc5ae05..d7c2c2ef7 100644 --- a/packages/core/src/studio-api/helpers/sourceMutation.ts +++ b/packages/core/src/studio-api/helpers/sourceMutation.ts @@ -2,6 +2,7 @@ import { parseHTML } from "linkedom"; export interface SourceMutationTarget { id?: string | null; + hfId?: string; selector?: string; selectorIndex?: number; } @@ -31,7 +32,41 @@ function querySelectorAllWithTemplates(root: Document | Element, selector: strin return []; } +// Prevent CSS attribute-selector injection via a crafted hfId: escape +// backslashes first, then double-quotes. Keeps a malformed/hostile value from +// breaking out of the `[data-hf-id="…"]` selector once callers beyond the +// internal mint contract (R2+ user flows) pass values here. +function escapeCssAttrValue(value: string): string { + return value.replace(/\\/g, "\\\\").replace(/"/g, '\\"'); +} + +function findByHfId(document: Document, hfId: string): Element | null { + try { + const matches = querySelectorAllWithTemplates( + document, + `[data-hf-id="${escapeCssAttrValue(hfId)}"]`, + ); + if (matches.length > 1) { + // The mint contract guarantees uniqueness; a duplicate means upstream + // id drift. Don't silently patch an arbitrary one — surface it. + // eslint-disable-next-line no-console + console.warn( + `sourceMutation: data-hf-id "${hfId}" matched ${matches.length} elements; using the first. ids must be unique per document.`, + ); + } + return matches[0] ?? null; + } catch { + // Malformed selector despite escaping — let the caller fall back. + return null; + } +} + function findTargetElement(document: Document, target: SourceMutationTarget): Element | null { + if (target.hfId) { + const el = findByHfId(document, target.hfId); + if (el) return el; + } + if (target.id) { const byId = document.getElementById(target.id); if (byId) return byId; @@ -207,7 +242,7 @@ export function patchElementInHtml( } export function probeElementInSource(source: string, target: SourceMutationTarget): boolean { - if (!target.id && !target.selector) return false; + if (!target.id && !target.hfId && !target.selector) return false; const { document } = parseSourceDocument(source); const el = findTargetElement(document, target); return el != null && isHTMLElement(el); diff --git a/packages/studio/src/utils/sourcePatcher.test.ts b/packages/studio/src/utils/sourcePatcher.test.ts index 7697d24e6..ee4ab514c 100644 --- a/packages/studio/src/utils/sourcePatcher.test.ts +++ b/packages/studio/src/utils/sourcePatcher.test.ts @@ -517,17 +517,88 @@ describe("motion attribute round-trip via sourcePatcher", () => { }); }); -// T3 — id-based targeting (spec for R1). -// R1 adds `hfId?: string` to PatchTarget and a `[data-hf-id="…"]` lookup branch -// in findTagByTarget. Convert from it.todo to real assertions in the R1 PR. +// T3 — id-based targeting (R1). describe("T3 — hfId targeting (spec for R1)", () => { - it.todo("updates inline style by data-hf-id"); + it("updates inline style by data-hf-id", () => { + const html = `

Hello

`; + const result = applyPatchByTarget( + html, + { hfId: "hf-x7k2" }, + { + type: "inline-style", + property: "color", + value: "blue", + }, + ); + expect(result).toContain("color: blue"); + expect(result).toContain('data-hf-id="hf-x7k2"'); + }); - it.todo("updates text content by data-hf-id"); + it("updates text content by data-hf-id", () => { + const html = `

Old text

`; + const result = applyPatchByTarget( + html, + { hfId: "hf-a1b2" }, + { + type: "text-content", + property: "", + value: "New text", + }, + ); + expect(result).toContain(">New text<"); + }); - it.todo("updates attribute by data-hf-id"); + it("updates attribute by data-hf-id", () => { + const html = `
`; + const result = applyPatchByTarget( + html, + { hfId: "hf-c3d4" }, + { + type: "attribute", + property: "start", + value: "2.5", + }, + ); + expect(result).toContain('data-start="2.5"'); + }); + + it("data-hf-id attribute is preserved after a style patch", () => { + const html = `

Hello

`; + const patched = applyPatchByTarget( + html, + { hfId: "hf-x7k2" }, + { + type: "inline-style", + property: "color", + value: "blue", + }, + ); + expect(readAttributeByTarget(patched, { hfId: "hf-x7k2" }, "data-hf-id")).toBe("hf-x7k2"); + }); - it.todo("data-hf-id attribute is preserved after a style patch"); + it("hfId lookup falls through to selector when hfId not found", () => { + const html = `

Hello

`; + const result = applyPatchByTarget( + html, + { hfId: "hf-missing", selector: ".headline" }, + { type: "inline-style", property: "color", value: "blue" }, + ); + expect(result).toContain("color: blue"); + }); - it.todo("hfId lookup falls through to selector when hfId not found"); + it("hfId match is authoritative — selector is not used as a narrowing filter", () => { + // hfId matches h1; selector points at h2. hfId wins — patch lands on h1, h2 untouched. + const html = `

A

B

`; + const result = applyPatchByTarget( + html, + { hfId: "hf-x7k2", selector: ".b" }, + { type: "inline-style", property: "color", value: "blue" }, + ); + expect(result).toContain('data-hf-id="hf-x7k2"'); + const h1End = result.indexOf(""); + const bluePos = result.indexOf("color: blue"); + expect(bluePos).toBeGreaterThan(-1); + expect(bluePos).toBeLessThan(h1End); + expect(result).toContain('

B

'); + }); }); diff --git a/packages/studio/src/utils/sourcePatcher.ts b/packages/studio/src/utils/sourcePatcher.ts index 2d89eea2e..9d19114d4 100644 --- a/packages/studio/src/utils/sourcePatcher.ts +++ b/packages/studio/src/utils/sourcePatcher.ts @@ -94,6 +94,7 @@ export interface PatchOperation { export interface PatchTarget { id?: string | null; + hfId?: string; selector?: string; selectorIndex?: number; } @@ -232,61 +233,67 @@ function replaceTagAtMatch(html: string, match: TagMatch, newTag: string): strin return `${html.slice(0, match.start)}${newTag}${html.slice(match.end)}`; } -export function findTagByTarget(html: string, target: PatchTarget): TagMatch | null { - if (target.id) { - const idPattern = new RegExp(`(<[^>]*\\bid=(["'])${escapeRegex(target.id)}\\2[^>]*)>`, "i"); - const match = idPattern.exec(html); - if (match?.index != null) { +function execDataAttrPattern(html: string, attr: string, value: string): TagMatch | null { + const pattern = new RegExp(`(<[^>]*\\b${attr}=(["'])${escapeRegex(value)}\\2[^>]*)>`, "i"); + const match = pattern.exec(html); + if (match?.index == null) return null; + // Defensive: a second exact match means a duplicate id/attr in the source + // (id drift). Don't silently patch the first while leaving the other stale — + // surface it. By the mint contract this should never fire. + const all = html.match(new RegExp(`<[^>]*\\b${attr}=(["'])${escapeRegex(value)}\\1[^>]*>`, "gi")); + if (all && all.length > 1) { + // eslint-disable-next-line no-console + console.warn( + `sourcePatcher: ${attr}="${value}" matched ${all.length} elements; patching the first. ids/attrs must be unique per document.`, + ); + } + return { tag: match[1], start: match.index, end: match.index + match[1].length }; +} + +function findTagByClass(html: string, target: PatchTarget): TagMatch | null { + const classMatch = target.selector?.match(/^\.([a-zA-Z0-9_-]+)$/); + if (!classMatch) return null; + const cls = classMatch[1]; + const pattern = new RegExp( + `(<[^>]*\\bclass=(["'])[^"']*\\b${escapeRegex(cls)}\\b[^"']*\\2[^>]*)>`, + "gi", + ); + const selectorIndex = target.selectorIndex ?? 0; + let match: RegExpExecArray | null; + let currentIndex = 0; + while ((match = pattern.exec(html)) !== null) { + if (currentIndex === selectorIndex && match.index != null) { return { tag: match[1], start: match.index, end: match.index + match[1].length, }; } + currentIndex += 1; + } + return null; +} + +export function findTagByTarget(html: string, target: PatchTarget): TagMatch | null { + if (target.hfId) { + const result = execDataAttrPattern(html, "data-hf-id", target.hfId); + if (result) return result; + } + + if (target.id) { + const result = execDataAttrPattern(html, "id", target.id); + if (result) return result; } if (!target.selector) return null; const compositionIdMatch = target.selector.match(/^\[data-composition-id="([^"]+)"\]$/); if (compositionIdMatch) { - const compId = compositionIdMatch[1]; - const pattern = new RegExp( - `(<[^>]*\\bdata-composition-id=(["'])${escapeRegex(compId)}\\2[^>]*)>`, - "i", - ); - const match = pattern.exec(html); - if (match?.index != null) { - return { - tag: match[1], - start: match.index, - end: match.index + match[1].length, - }; - } + const result = execDataAttrPattern(html, "data-composition-id", compositionIdMatch[1]); + if (result) return result; } - const classMatch = target.selector.match(/^\.([a-zA-Z0-9_-]+)$/); - if (classMatch) { - const cls = classMatch[1]; - const pattern = new RegExp( - `(<[^>]*\\bclass=(["'])[^"']*\\b${escapeRegex(cls)}\\b[^"']*\\2[^>]*)>`, - "gi", - ); - const selectorIndex = target.selectorIndex ?? 0; - let match: RegExpExecArray | null; - let currentIndex = 0; - while ((match = pattern.exec(html)) !== null) { - if (currentIndex === selectorIndex && match.index != null) { - return { - tag: match[1], - start: match.index, - end: match.index + match[1].length, - }; - } - currentIndex += 1; - } - } - - return null; + return findTagByClass(html, target); } export function readAttributeByTarget(