diff --git a/packages/core/src/runtime/startResolver.test.ts b/packages/core/src/runtime/startResolver.test.ts index 486b1359233..0880ffe99e2 100644 --- a/packages/core/src/runtime/startResolver.test.ts +++ b/packages/core/src/runtime/startResolver.test.ts @@ -275,6 +275,35 @@ describe("createRuntimeStartTimeResolver", () => { expect(resolver.resolveStartForElement(video)).toBe(26); }); + it("resolves a reference inside the referencing clip's own composition", () => { + const mount = (hostId: string, start: string) => { + const host = document.createElement("div"); + host.id = hostId; + host.setAttribute("data-composition-id", "card"); + host.setAttribute("data-start", start); + host.setAttribute("data-duration", "2"); + document.body.appendChild(host); + + const first = document.createElement("div"); + first.id = "first"; + first.setAttribute("data-start", "0"); + first.setAttribute("data-duration", "1"); + host.appendChild(first); + + const second = document.createElement("div"); + second.setAttribute("data-start", "first"); + second.setAttribute("data-duration", "1"); + host.appendChild(second); + return second; + }; + const secondInA = mount("card-a", "0"); + const secondInB = mount("card-b", "2"); + + const resolver = createRuntimeStartTimeResolver({}); + expect(resolver.resolveStartForElement(secondInA)).toBe(1); + expect(resolver.resolveStartForElement(secondInB)).toBe(3); + }); + it("returns fallback when reference target not found", () => { const el = document.createElement("div"); el.setAttribute("data-start", "nonexistent"); diff --git a/packages/core/src/runtime/startResolver.ts b/packages/core/src/runtime/startResolver.ts index 28c4b3d19b4..e6ecf1f906b 100644 --- a/packages/core/src/runtime/startResolver.ts +++ b/packages/core/src/runtime/startResolver.ts @@ -46,7 +46,18 @@ export function createRuntimeStartTimeResolver(params: { const durationCache = new WeakMap(); const visiting = new Set(); - const findReferenceTarget = (refId: string): Element | null => { + const compositionScopeOf = (element: Element): Element | null => + (element.hasAttribute("data-composition-id") ? element.parentElement : element)?.closest( + "[data-composition-id]", + ) ?? null; + + const findReferenceTarget = (refId: string, from: Element): Element | null => { + const scope = compositionScopeOf(from); + if (scope) { + for (const candidate of scope.querySelectorAll("[id]")) { + if (candidate.id === refId && compositionScopeOf(candidate) === scope) return candidate; + } + } const byId = doc.getElementById(refId); if (byId) return byId; return ( @@ -135,7 +146,7 @@ export function createRuntimeStartTimeResolver(params: { // Negative when a host's in-point is past its start; the host's slot cuts that part. return resolveHostOffsetForElement(element, fallback) + Math.max(0, expression.value); } - const target = findReferenceTarget(expression.refId); + const target = findReferenceTarget(expression.refId, element); if (!target) return fallback; const targetStart = resolveStartForElementInternal(target, 0); const targetDuration = resolveDurationForElement(target) ?? 0; diff --git a/packages/engine/src/services/videoFrameExtractor.test.ts b/packages/engine/src/services/videoFrameExtractor.test.ts index 442e77035ab..4ac7a391c2b 100644 --- a/packages/engine/src/services/videoFrameExtractor.test.ts +++ b/packages/engine/src/services/videoFrameExtractor.test.ts @@ -872,6 +872,21 @@ describe("parseVideoElements", () => { expect(videos.find((v) => v.id === "main")?.start).toBe(9); }); + it("resolves a reference inside its own composition when another composition reuses the id", () => { + const videos = parseVideoElements( + '
' + + '
' + + '
' + + "
" + + '
' + + '
' + + '' + + "
" + + "
", + ); + expect(videos.find((v) => v.id === "clip")?.start).toBe(3); + }); + it("falls back to 0 (never NaN) for an unknown reference target", () => { const videos = parseVideoElements( '', diff --git a/packages/parsers/src/referenceResolver.test.ts b/packages/parsers/src/referenceResolver.test.ts new file mode 100644 index 00000000000..d53cb5812f4 --- /dev/null +++ b/packages/parsers/src/referenceResolver.test.ts @@ -0,0 +1,17 @@ +import { describe, expect, it } from "vitest"; +import { resolveReferencedStart, type RefResolverEl } from "./referenceResolver.js"; + +describe("resolveReferencedStart", () => { + it("still resolves through an adapter that only implements getAttribute", () => { + const element = (attrs: Record): RefResolverEl => ({ + getAttribute: (name) => attrs[name] ?? null, + }); + const intro = element({ id: "intro", "data-start": "1", "data-duration": "3" }); + const clip = element({ id: "clip", "data-start": "intro + 1" }); + const doc = { + getElementById: (id: string) => (id === "intro" ? intro : null), + querySelector: () => null, + }; + expect(resolveReferencedStart(doc, clip, new Map(), new Set())).toBe(5); + }); +}); diff --git a/packages/parsers/src/referenceResolver.ts b/packages/parsers/src/referenceResolver.ts index 6679ed0bc53..ec449f170ae 100644 --- a/packages/parsers/src/referenceResolver.ts +++ b/packages/parsers/src/referenceResolver.ts @@ -6,14 +6,38 @@ import { parseNumeric, parseStartExpression } from "./compositionContract.js"; /** Minimal structural DOM shape the reference resolver needs. */ export interface RefResolverEl { getAttribute(name: string): string | null; + readonly parentElement?: RefResolverEl | null; + closest?(selector: string): RefResolverEl | null; + querySelectorAll?(selector: string): Iterable; } export interface RefResolverDoc { getElementById(id: string): RefResolverEl | null; querySelector(selector: string): RefResolverEl | null; } -// By `id`, then `data-composition-id`. The reference grammar allows only `[A-Za-z0-9_.:-]`, so no CSS.escape. -function findReferenceTargetEl(doc: RefResolverDoc, refId: string): RefResolverEl | null { +function compositionScopeOf(el: RefResolverEl): RefResolverEl | null { + return ( + (el.getAttribute("data-composition-id") !== null ? el.parentElement : el)?.closest?.( + "[data-composition-id]", + ) ?? null + ); +} + +// By `id` in the clip's own composition, then document-wide by `id`, then `data-composition-id`. +// The reference grammar allows only `[A-Za-z0-9_.:-]`, so no CSS.escape. +function findReferenceTargetEl( + doc: RefResolverDoc, + refId: string, + from: RefResolverEl, +): RefResolverEl | null { + const scope = compositionScopeOf(from); + if (scope) { + for (const candidate of scope.querySelectorAll?.("[id]") ?? []) { + if (candidate.getAttribute("id") === refId && compositionScopeOf(candidate) === scope) { + return candidate; + } + } + } return doc.getElementById(refId) ?? doc.querySelector(`[data-composition-id="${refId}"]`); } @@ -40,7 +64,7 @@ export function resolveReferencedStart( startCache.set(el, value); return value; } - const target = findReferenceTargetEl(doc, expression.refId); + const target = findReferenceTargetEl(doc, expression.refId, el); if (!target) { startCache.set(el, 0); return 0; diff --git a/packages/sdk/src/session.timings.test.ts b/packages/sdk/src/session.timings.test.ts index c6ce94fa3ce..5677e8da96b 100644 --- a/packages/sdk/src/session.timings.test.ts +++ b/packages/sdk/src/session.timings.test.ts @@ -224,8 +224,8 @@ describe("getElementTimings — relative data-start references", () => { it("resolves a colliding bare id to the TOP-LEVEL match, not a same-scope sibling", async () => { // Bare ids have no scope syntax — resolveScoped's bare-id rule prefers the - // canonical top-level match when one exists, same as the runtime's own (also - // global, not scope-aware) resolver. Both the outer document AND the sub-comp + // canonical top-level match when one exists. The runtime's `id` lookup prefers + // the clip's own composition instead. Both the outer document AND the sub-comp // author an element with the SAME bare id "hf-intro" — a genuine collision. const html = ` diff --git a/packages/sdk/src/session.ts b/packages/sdk/src/session.ts index 86d096126d1..780e0e2a93e 100644 --- a/packages/sdk/src/session.ts +++ b/packages/sdk/src/session.ts @@ -381,9 +381,9 @@ class CompositionImpl implements Composition { this._gsapLabelCache = scripts.length ? { scripts: [...scripts], labels: allLabels } : null; } - // refId remains a bare global id, matching runtime resolution. The shared - // contract owns parsing, canonical duration preference, clamping, and - // legacy data-end compatibility; this adapter only supplies document lookup. + // refId resolves as a bare hf-id (top level first), unlike the runtime's + // composition-scoped `id` lookup. The shared contract owns parsing, canonical + // duration preference, clamping, and legacy data-end; this adapter only supplies lookup. const timingCache = new Map(); const visiting = new Set(); const resolveTiming = (el: Element): ClipTiming => {