From 9b63192228c54335228d21d125f1aa05445a874e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Tue, 28 Apr 2026 15:30:24 -0400 Subject: [PATCH 1/3] fix(studio): warn on anonymous timeline clips --- packages/core/src/lint/rules/core.test.ts | 29 ++++++++ packages/core/src/lint/rules/core.ts | 58 +++++++++++++++ packages/core/src/runtime/timeline.test.ts | 33 +++++++++ packages/core/src/runtime/timeline.ts | 74 ++++++++++++++++--- packages/studio/src/App.tsx | 14 ++-- .../src/player/components/TimelineClip.tsx | 3 +- .../player/hooks/useTimelinePlayer.test.ts | 1 + .../src/player/hooks/useTimelinePlayer.ts | 62 ++++++++++++---- .../studio/src/player/store/playerStore.ts | 1 + 9 files changed, 247 insertions(+), 28 deletions(-) diff --git a/packages/core/src/lint/rules/core.test.ts b/packages/core/src/lint/rules/core.test.ts index 04b7fd34e..778e8de16 100644 --- a/packages/core/src/lint/rules/core.test.ts +++ b/packages/core/src/lint/rules/core.test.ts @@ -92,6 +92,35 @@ describe("core rules", () => { expect(finding).toBeUndefined(); }); + it("warns when a timeline-visible element has no stable id for Studio editing", () => { + const html = ` + +
+
+
+ +`; + const result = lintHyperframeHtml(html); + const finding = result.findings.find((f) => f.code === "studio_missing_editable_id"); + expect(finding).toBeDefined(); + expect(finding?.severity).toBe("warning"); + expect(finding?.message).toContain('
'); + expect(finding?.fixHint).toContain("stable, human-readable id"); + }); + + it("does not warn about the composition root or timeline elements with ids", () => { + const html = ` + +
+
+
+ +`; + const result = lintHyperframeHtml(html); + const finding = result.findings.find((f) => f.code === "studio_missing_editable_id"); + expect(finding).toBeUndefined(); + }); + describe("non_deterministic_code", () => { it("detects Math.random() in script content", () => { const html = ` diff --git a/packages/core/src/lint/rules/core.ts b/packages/core/src/lint/rules/core.ts index eddb0a5f3..f1fa46662 100644 --- a/packages/core/src/lint/rules/core.ts +++ b/packages/core/src/lint/rules/core.ts @@ -21,6 +21,40 @@ function selectorTargetsCompositionId(selector: string, compositionId: string): ).test(selector); } +function isStudioTimelineElement(tag: { raw: string; name: string }): boolean { + if (["script", "style", "link", "meta", "template", "noscript"].includes(tag.name)) { + return false; + } + return Boolean( + readAttr(tag.raw, "data-start") || + readAttr(tag.raw, "data-track-index") || + readAttr(tag.raw, "data-track") || + readAttr(tag.raw, "data-composition-src") || + readAttr(tag.raw, "data-composition-file"), + ); +} + +function describeStudioElement(tag: { raw: string; name: string }): string { + const parts = [`<${tag.name}`]; + const className = readAttr(tag.raw, "class"); + const compositionId = readAttr(tag.raw, "data-composition-id"); + const dataStart = readAttr(tag.raw, "data-start"); + const dataTrack = readAttr(tag.raw, "data-track-index") ?? readAttr(tag.raw, "data-track"); + + if (className) { + const primaryClass = className + .split(/\s+/) + .map((value) => value.trim()) + .find((value) => value && value !== "clip"); + if (primaryClass) parts.push(` class="${primaryClass}"`); + } + if (compositionId) parts.push(` data-composition-id="${compositionId}"`); + if (dataStart) parts.push(` data-start="${dataStart}"`); + if (dataTrack) parts.push(` data-track-index="${dataTrack}"`); + parts.push(">"); + return parts.join(""); +} + export const coreRules: Array<(ctx: LintContext) => HyperframeLintFinding[]> = [ // root_missing_composition_id + root_missing_dimensions ({ rootTag }) => { @@ -213,6 +247,30 @@ export const coreRules: Array<(ctx: LintContext) => HyperframeLintFinding[]> = [ return findings; }, + // studio_missing_editable_id + ({ tags, rootTag }) => { + const findings: HyperframeLintFinding[] = []; + for (const tag of tags) { + if (rootTag && tag.index === rootTag.index) continue; + if (!isStudioTimelineElement(tag)) continue; + if (readAttr(tag.raw, "id")) continue; + + const descriptor = describeStudioElement(tag); + findings.push({ + code: "studio_missing_editable_id", + severity: "warning", + message: `${descriptor} has no id, so Studio cannot use a stable edit target for its timeline and canvas controls.`, + selector: readAttr(tag.raw, "data-composition-id") + ? `[data-composition-id="${readAttr(tag.raw, "data-composition-id")}"]` + : undefined, + fixHint: + 'Add a stable, human-readable id such as id="hero-title" or id="scene-1-card" to every timeline-visible element you want agents or Studio to edit.', + snippet: truncateSnippet(tag.raw), + }); + } + return findings; + }, + // non_deterministic_code ({ scripts }) => { const findings: HyperframeLintFinding[] = []; diff --git a/packages/core/src/runtime/timeline.test.ts b/packages/core/src/runtime/timeline.test.ts index 1996907c0..bc3a1a093 100644 --- a/packages/core/src/runtime/timeline.test.ts +++ b/packages/core/src/runtime/timeline.test.ts @@ -271,6 +271,39 @@ describe("collectRuntimeTimelinePayload", () => { expect(result.clips[0].label).toBe("Hero Shot"); }); + it("uses a friendly label and null id for anonymous clips", () => { + const root = document.createElement("div"); + root.setAttribute("data-composition-id", "main"); + root.setAttribute("data-duration", "10"); + document.body.appendChild(root); + + const clip = document.createElement("div"); + clip.className = "clip hero-card"; + clip.setAttribute("data-start", "0"); + clip.setAttribute("data-duration", "5"); + root.appendChild(clip); + + const result = collectRuntimeTimelinePayload(defaultParams); + expect(result.clips[0].id).toBeNull(); + expect(result.clips[0].label).toBe("Hero Card"); + }); + + it("falls back to a readable ordinal label instead of a node index id", () => { + const root = document.createElement("div"); + root.setAttribute("data-composition-id", "main"); + root.setAttribute("data-duration", "10"); + document.body.appendChild(root); + + const clip = document.createElement("div"); + clip.setAttribute("data-start", "0"); + clip.setAttribute("data-duration", "5"); + root.appendChild(clip); + + const result = collectRuntimeTimelinePayload(defaultParams); + expect(result.clips[0].id).toBeNull(); + expect(result.clips[0].label).toBe("Element 1"); + }); + it("handles timeline registry for composition duration", () => { const root = document.createElement("div"); root.setAttribute("data-composition-id", "main"); diff --git a/packages/core/src/runtime/timeline.ts b/packages/core/src/runtime/timeline.ts index 91856ca51..67e7ab0c3 100644 --- a/packages/core/src/runtime/timeline.ts +++ b/packages/core/src/runtime/timeline.ts @@ -110,6 +110,69 @@ function resolveNodeAssetUrl(node: Element): string | null { return toAbsoluteAssetUrl(mediaDescendant.getAttribute("src")); } +function getFirstClassToken(node: Element): string | null { + const className = (node as HTMLElement).className; + if (typeof className !== "string") return null; + return ( + className + .split(/\s+/) + .map((value) => value.trim()) + .find((value) => value && value !== "clip" && !value.startsWith("__hf-")) ?? null + ); +} + +function filenameFromAssetUrl(url: string | null): string | null { + if (!url) return null; + try { + const parsed = new URL(url, document.baseURI); + return parsed.pathname.split("/").filter(Boolean).at(-1) ?? null; + } catch { + return url.split(/[\\/]/).filter(Boolean).at(-1) ?? null; + } +} + +function textPreview(node: Element): string | null { + const text = node.textContent?.replace(/\s+/g, " ").trim(); + if (!text) return null; + return text.length > 32 ? `${text.slice(0, 31)}...` : text; +} + +function humanizeTimelineToken(value: string): string { + const normalized = value + .replace(/\.[^.]+$/i, "") + .replace(/[-_]+/g, " ") + .replace(/\s+/g, " ") + .trim(); + if (!normalized) return value; + return normalized.replace(/\b\w/g, (char) => char.toUpperCase()); +} + +function buildTimelineClipLabel(node: Element, kind: RuntimeTimelineClip["kind"], ordinal: number) { + const explicit = + node.getAttribute("data-timeline-label") ?? + node.getAttribute("data-label") ?? + node.getAttribute("aria-label") ?? + null; + if (explicit?.trim()) return explicit.trim(); + + const compositionId = node.getAttribute("data-composition-id"); + if (compositionId) return humanizeTimelineToken(compositionId); + + const id = (node as HTMLElement).id; + if (id) return humanizeTimelineToken(id); + + const classToken = getFirstClassToken(node); + if (classToken) return humanizeTimelineToken(classToken); + + const assetName = filenameFromAssetUrl(resolveNodeAssetUrl(node)); + if (assetName) return humanizeTimelineToken(assetName); + + const text = textPreview(node); + if (text) return text; + + return `${humanizeTimelineToken(kind)} ${ordinal + 1}`; +} + export function collectRuntimeTimelinePayload(params: { canonicalFps: number; maxTimelineDurationSeconds: number; @@ -367,15 +430,8 @@ export function collectRuntimeTimelinePayload(params: { ? "image" : "element"; clips.push({ - id: (node as HTMLElement).id || nodeCompositionId || `__node__index_${i}`, - label: - node.getAttribute("data-timeline-label") ?? - node.getAttribute("data-label") ?? - node.getAttribute("aria-label") ?? - nodeCompositionId ?? - (node as HTMLElement).id ?? - (node as HTMLElement).className?.split(" ")[0] ?? - kind, + id: (node as HTMLElement).id || nodeCompositionId || null, + label: buildTimelineClipLabel(node, kind, clips.length), start, duration, track: diff --git a/packages/studio/src/App.tsx b/packages/studio/src/App.tsx index aeeca36dc..332cb82d2 100644 --- a/packages/studio/src/App.tsx +++ b/packages/studio/src/App.tsx @@ -51,6 +51,10 @@ interface AppToast { tone: "error" | "info"; } +function getTimelineElementLabel(element: TimelineElement): string { + return element.label || element.id || element.tag; +} + const DEFAULT_TIMELINE_ASSET_DURATION: Record = { image: 3, video: 5, @@ -361,7 +365,7 @@ export function StudioApp() { return ( ); @@ -427,7 +431,7 @@ export function StudioApp() { return ( @@ -438,7 +442,7 @@ export function StudioApp() { return ( 0.01; return ( @@ -93,7 +94,7 @@ export const TimelineClip = memo(function TimelineClip({ title={ isComposition ? `${el.compositionSrc} \u2022 Double-click to open` - : `${el.id || el.tag} \u2022 ${el.start.toFixed(1)}s \u2013 ${(el.start + el.duration).toFixed(1)}s` + : `${displayLabel} \u2022 ${el.start.toFixed(1)}s \u2013 ${(el.start + el.duration).toFixed(1)}s` } onPointerEnter={onHoverStart} onPointerLeave={onHoverEnd} diff --git a/packages/studio/src/player/hooks/useTimelinePlayer.test.ts b/packages/studio/src/player/hooks/useTimelinePlayer.test.ts index c0f5db6a1..fcd4af04f 100644 --- a/packages/studio/src/player/hooks/useTimelinePlayer.test.ts +++ b/packages/studio/src/player/hooks/useTimelinePlayer.test.ts @@ -39,6 +39,7 @@ describe("buildStandaloneRootTimelineElement", () => { }), ).toEqual({ id: "hero", + label: "hero", key: 'scenes/hero.html:[data-composition-id="hero"]:0', tag: "div", start: 0, diff --git a/packages/studio/src/player/hooks/useTimelinePlayer.ts b/packages/studio/src/player/hooks/useTimelinePlayer.ts index 9947b1d63..f28a04757 100644 --- a/packages/studio/src/player/hooks/useTimelinePlayer.ts +++ b/packages/studio/src/player/hooks/useTimelinePlayer.ts @@ -165,6 +165,19 @@ export function shouldIgnorePlaybackShortcutEvent( ); } +function getTimelineElementDisplayLabel(input: { + id?: string | null; + label?: string | null; + tag?: string | null; +}): string { + const label = input.label?.trim(); + if (label) return label; + const id = input.id?.trim(); + if (id) return id; + const tag = input.tag?.trim().toLowerCase(); + return tag ? `${tag} clip` : "Timeline clip"; +} + /** * Parse [data-start] elements from a Document into TimelineElement[]. * Shared helper — used by onIframeLoad fallback, handleMessage, and enrichMissingCompositions. @@ -200,9 +213,15 @@ function parseTimelineFromDOM(doc: Document, rootDuration: number): TimelineElem const selector = getTimelineElementSelector(el); const sourceFile = getTimelineElementSourceFile(el); const selectorIndex = getTimelineElementSelectorIndex(doc, el, selector); - const id = el.id || compId || el.className?.split(" ")[0] || tagLower; + const label = getTimelineElementDisplayLabel({ + id: el.id || compId || null, + label: el.getAttribute("data-timeline-label") ?? el.getAttribute("data-label"), + tag: tagLower, + }); + const id = el.id || compId || label; const entry: TimelineElement = { id, + label, key: buildTimelineElementKey({ id, fallbackIndex: els.length, @@ -333,6 +352,10 @@ export function buildStandaloneRootTimelineElement(params: { return { id: params.compositionId, + label: getTimelineElementDisplayLabel({ + id: params.compositionId, + tag: params.tagName, + }), key: buildTimelineElementKey({ id: params.compositionId, fallbackIndex: 0, @@ -822,26 +845,33 @@ export function useTimelinePlayer() { const filtered = data.clips.filter( (clip) => !clip.parentCompositionId || !clipCompositionIds.has(clip.parentCompositionId), ); + let iframeDoc: Document | null = null; + try { + iframeDoc = iframeRef.current?.contentDocument ?? null; + } catch { + iframeDoc = null; + } + const usedHostEls = new Set(); const els: TimelineElement[] = filtered.map((clip, index) => { - let hostEl: Element | null = null; - const id = clip.id || clip.label || clip.tagName || "element"; + let hostEl = iframeDoc + ? findTimelineDomNodeForClip(iframeDoc, clip, index, usedHostEls) + : null; + if (hostEl) usedHostEls.add(hostEl); + const label = getTimelineElementDisplayLabel({ + id: clip.id, + label: clip.label, + tag: clip.tagName || clip.kind, + }); + const id = clip.id || label; const entry: TimelineElement = { id, + label, tag: clip.tagName || clip.kind, start: clip.start, duration: clip.duration, track: clip.track, }; - try { - const iframeDoc = iframeRef.current?.contentDocument; - if (iframeDoc && entry.id) { - hostEl = findTimelineDomNode(iframeDoc, entry.id); - } - } catch { - /* cross-origin */ - } if (hostEl) { - const iframeDoc = iframeRef.current?.contentDocument; entry.domId = hostEl.id || undefined; entry.selector = getTimelineElementSelector(hostEl); entry.selectorIndex = @@ -1014,9 +1044,15 @@ export function useTimelinePlayer() { const selector = getTimelineElementSelector(el); const sourceFile = getTimelineElementSourceFile(el); const selectorIndex = getTimelineElementSelectorIndex(doc, el, selector); - const id = el.id || compId; + const label = getTimelineElementDisplayLabel({ + id: el.id || compId || null, + label: el.getAttribute("data-timeline-label") ?? el.getAttribute("data-label"), + tag: el.tagName, + }); + const id = el.id || compId || label; const entry: TimelineElement = { id, + label, key: buildTimelineElementKey({ id, fallbackIndex: missing.length, diff --git a/packages/studio/src/player/store/playerStore.ts b/packages/studio/src/player/store/playerStore.ts index 5d9c2d1a9..348665f23 100644 --- a/packages/studio/src/player/store/playerStore.ts +++ b/packages/studio/src/player/store/playerStore.ts @@ -2,6 +2,7 @@ import { create } from "zustand"; export interface TimelineElement { id: string; + label?: string; key?: string; tag: string; start: number; From 90e9027f1a637c216014a45523b0d704eaf8bcf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Tue, 28 Apr 2026 16:48:32 -0400 Subject: [PATCH 2/3] fix(studio): keep anonymous clip identities stable --- .../studio/src/player/components/Timeline.tsx | 7 +- .../player/components/timelineTheme.test.ts | 19 ++ .../src/player/components/timelineTheme.ts | 6 +- .../player/hooks/useTimelinePlayer.test.ts | 168 +++++++++ .../src/player/hooks/useTimelinePlayer.ts | 321 ++++++++++++------ 5 files changed, 416 insertions(+), 105 deletions(-) diff --git a/packages/studio/src/player/components/Timeline.tsx b/packages/studio/src/player/components/Timeline.tsx index 13f971ba4..4ed4319cc 100644 --- a/packages/studio/src/player/components/Timeline.tsx +++ b/packages/studio/src/player/components/Timeline.tsx @@ -1014,7 +1014,10 @@ export const Timeline = memo(function Timeline({ major.length >= 2 ? Math.max(0.25, major[1] - major[0]) : effectiveDuration; const getPreviewElement = useCallback( (element: TimelineElement): TimelineElement => { - if (resizingClip?.element.id === element.id) { + if ( + resizingClip && + (resizingClip.element.key ?? resizingClip.element.id) === (element.key ?? element.id) + ) { return { ...element, start: resizingClip.previewStart, @@ -1242,7 +1245,7 @@ export const Timeline = memo(function Timeline({ draggedClip?.started === true && draggedElement ? getRenderedTimelineElement({ element: draggedElement, - draggedElementId: draggedElement.id, + draggedElementId: draggedElement.key ?? draggedElement.id, previewStart: draggedClip.previewStart, previewTrack: draggedClip.previewTrack, }) diff --git a/packages/studio/src/player/components/timelineTheme.test.ts b/packages/studio/src/player/components/timelineTheme.test.ts index 92f92eb33..d1e3b90dc 100644 --- a/packages/studio/src/player/components/timelineTheme.test.ts +++ b/packages/studio/src/player/components/timelineTheme.test.ts @@ -53,4 +53,23 @@ describe("getRenderedTimelineElement", () => { }), ).toEqual({ ...element, start: 2.4, track: 3 }); }); + + it("uses key before id when matching the dragged clip", () => { + const element = { + id: "Card", + key: "index.html:.card:1", + tag: "div", + start: 1, + duration: 2, + track: 0, + }; + expect( + getRenderedTimelineElement({ + element, + draggedElementId: "index.html:.card:1", + previewStart: 2.4, + previewTrack: 3, + }), + ).toEqual({ ...element, start: 2.4, track: 3 }); + }); }); diff --git a/packages/studio/src/player/components/timelineTheme.ts b/packages/studio/src/player/components/timelineTheme.ts index 1b29dbf7f..9e9da32dd 100644 --- a/packages/studio/src/player/components/timelineTheme.ts +++ b/packages/studio/src/player/components/timelineTheme.ts @@ -130,7 +130,11 @@ export function getRenderedTimelineElement({ previewStart: number | null; previewTrack: number | null; }): TimelineElement { - if (element.id !== draggedElementId || previewStart === null || previewTrack === null) { + if ( + (element.key ?? element.id) !== draggedElementId || + previewStart === null || + previewTrack === null + ) { return element; } return { diff --git a/packages/studio/src/player/hooks/useTimelinePlayer.test.ts b/packages/studio/src/player/hooks/useTimelinePlayer.test.ts index fcd4af04f..d5366af0d 100644 --- a/packages/studio/src/player/hooks/useTimelinePlayer.test.ts +++ b/packages/studio/src/player/hooks/useTimelinePlayer.test.ts @@ -1,6 +1,12 @@ import { describe, expect, it } from "vitest"; +import { Window } from "happy-dom"; import { buildStandaloneRootTimelineElement, + createTimelineElementFromManifestClip, + findTimelineDomNodeForClip, + getTimelineElementSelector, + parseTimelineFromDOM, + type ClipManifestClip, mergeTimelineElementsPreservingDowngrades, resolveStandaloneRootCompositionSrc, shouldIgnorePlaybackShortcutEvent, @@ -27,6 +33,29 @@ function mockKeyboardEvent( }; } +function createDocument(markup: string): Document { + const window = new Window(); + window.document.body.innerHTML = markup; + return window.document; +} + +function createClip(overrides: Partial): ClipManifestClip { + return { + id: null, + label: "Element", + start: 0, + duration: 4, + track: 0, + kind: "element", + tagName: "div", + compositionId: null, + parentCompositionId: null, + compositionSrc: null, + assetUrl: null, + ...overrides, + }; +} + describe("buildStandaloneRootTimelineElement", () => { it("includes selector and source metadata for standalone composition fallback clips", () => { expect( @@ -88,6 +117,86 @@ describe("resolveStandaloneRootCompositionSrc", () => { }); }); +describe("findTimelineDomNodeForClip", () => { + it("matches anonymous manifest clips back to repeated DOM nodes in timeline order", () => { + const doc = createDocument(` +
+
+
+
+
+ `); + const used = new Set(); + + const first = findTimelineDomNodeForClip( + doc, + createClip({ id: "__node__index_2", track: 1 }), + 1, + used, + ) as HTMLElement; + used.add(first); + const second = findTimelineDomNodeForClip( + doc, + createClip({ id: "__node__index_3", track: 2 }), + 2, + used, + ) as HTMLElement; + + expect(first.className).toBe("clip duplicate-card first"); + expect(second.className).toBe("clip duplicate-card second"); + expect(getTimelineElementSelector(first)).toBe(".duplicate-card"); + expect(getTimelineElementSelector(second)).toBe(".duplicate-card"); + }); +}); + +describe("anonymous timeline identity", () => { + it("keeps fallback-parsed anonymous clips distinct when labels match", () => { + const doc = createDocument(` +
+
+
+
+ `); + + const elements = parseTimelineFromDOM(doc, 8); + + expect(elements).toHaveLength(2); + expect(elements.map((element) => element.label)).toEqual(["Card", "Card"]); + expect(new Set(elements.map((element) => element.id)).size).toBe(2); + expect(new Set(elements.map((element) => element.key)).size).toBe(2); + expect(elements.map((element) => element.selectorIndex)).toEqual([0, 1]); + }); + + it("keeps runtime-manifest anonymous clips distinct when labels match", () => { + const doc = createDocument(` +
+
+
+
+ `); + const clips = [ + createClip({ id: null, label: "Card", start: 0, duration: 3, track: 0 }), + createClip({ id: null, label: "Card", start: 3, duration: 3, track: 1 }), + ]; + const used = new Set(); + const elements = clips.map((clip, index) => { + const hostEl = findTimelineDomNodeForClip(doc, clip, index, used); + if (hostEl) used.add(hostEl); + return createTimelineElementFromManifestClip({ + clip, + fallbackIndex: index, + doc, + hostEl, + }); + }); + + expect(elements.map((element) => element.label)).toEqual(["Card", "Card"]); + expect(new Set(elements.map((element) => element.id)).size).toBe(2); + expect(new Set(elements.map((element) => element.key)).size).toBe(2); + expect(elements.map((element) => element.selectorIndex)).toEqual([0, 1]); + }); +}); + describe("mergeTimelineElementsPreservingDowngrades", () => { it("preserves missing current elements when a shorter manifest arrives", () => { expect( @@ -116,6 +225,65 @@ describe("mergeTimelineElementsPreservingDowngrades", () => { ), ).toEqual([{ id: "hero", tag: "div", start: 0, duration: 4, track: 0 }]); }); + + it("preserves distinct anonymous clips that share the same friendly id label", () => { + expect( + mergeTimelineElementsPreservingDowngrades( + [ + { + id: "Card", + key: "index.html:.card:0", + label: "Card", + tag: "div", + start: 0, + duration: 3, + track: 0, + }, + { + id: "Card", + key: "index.html:.card:1", + label: "Card", + tag: "div", + start: 3, + duration: 3, + track: 1, + }, + ], + [ + { + id: "Card", + key: "index.html:.card:0", + label: "Card", + tag: "div", + start: 0, + duration: 3, + track: 0, + }, + ], + 8, + 8, + ), + ).toEqual([ + { + id: "Card", + key: "index.html:.card:0", + label: "Card", + tag: "div", + start: 0, + duration: 3, + track: 0, + }, + { + id: "Card", + key: "index.html:.card:1", + label: "Card", + tag: "div", + start: 3, + duration: 3, + track: 1, + }, + ]); + }); }); describe("shouldIgnorePlaybackShortcutTarget", () => { diff --git a/packages/studio/src/player/hooks/useTimelinePlayer.ts b/packages/studio/src/player/hooks/useTimelinePlayer.ts index f28a04757..5e872664b 100644 --- a/packages/studio/src/player/hooks/useTimelinePlayer.ts +++ b/packages/studio/src/player/hooks/useTimelinePlayer.ts @@ -64,9 +64,12 @@ function wrapTimeline(tl: TimelineLike): PlaybackAdapter { } function resolveMediaElement(el: Element): HTMLMediaElement | HTMLImageElement | null { - if (el instanceof HTMLMediaElement || el instanceof HTMLImageElement) return el; + const win = el.ownerDocument.defaultView ?? window; + const MediaElementCtor = win.HTMLMediaElement ?? globalThis.HTMLMediaElement; + const ImageElementCtor = win.HTMLImageElement ?? globalThis.HTMLImageElement; + if (el instanceof MediaElementCtor || el instanceof ImageElementCtor) return el; const candidate = el.querySelector("video, audio, img"); - return candidate instanceof HTMLMediaElement || candidate instanceof HTMLImageElement + return candidate instanceof MediaElementCtor || candidate instanceof ImageElementCtor ? candidate : null; } @@ -182,7 +185,7 @@ function getTimelineElementDisplayLabel(input: { * Parse [data-start] elements from a Document into TimelineElement[]. * Shared helper — used by onIframeLoad fallback, handleMessage, and enrichMissingCompositions. */ -function parseTimelineFromDOM(doc: Document, rootDuration: number): TimelineElement[] { +export function parseTimelineFromDOM(doc: Document, rootDuration: number): TimelineElement[] { const rootComp = doc.querySelector("[data-composition-id]"); const nodes = doc.querySelectorAll("[data-start]"); const els: TimelineElement[] = []; @@ -218,18 +221,19 @@ function parseTimelineFromDOM(doc: Document, rootDuration: number): TimelineElem label: el.getAttribute("data-timeline-label") ?? el.getAttribute("data-label"), tag: tagLower, }); - const id = el.id || compId || label; + const identity = buildTimelineElementIdentity({ + preferredId: el.id || compId || null, + label, + fallbackIndex: els.length, + domId: el.id || undefined, + selector, + selectorIndex, + sourceFile, + }); const entry: TimelineElement = { - id, + id: identity.id, label, - key: buildTimelineElementKey({ - id, - fallbackIndex: els.length, - domId: el.id || undefined, - selector, - selectorIndex, - sourceFile, - }), + key: identity.key, tag: tagLower, start, duration: dur, @@ -272,12 +276,18 @@ function parseTimelineFromDOM(doc: Document, rootDuration: number): TimelineElem return els; } -function getTimelineElementSelector(el: Element): string | undefined { - if (el instanceof HTMLElement && el.id) return `#${el.id}`; +function isHtmlElement(el: Element): el is HTMLElement { + const HtmlElementCtor = el.ownerDocument.defaultView?.HTMLElement ?? globalThis.HTMLElement; + return typeof HtmlElementCtor !== "undefined" && el instanceof HtmlElementCtor; +} + +export function getTimelineElementSelector(el: Element): string | undefined { + if (isHtmlElement(el) && el.id) return `#${el.id}`; const compId = el.getAttribute("data-composition-id"); if (compId) return `[data-composition-id="${compId}"]`; - if (el instanceof HTMLElement) { - const firstClass = el.className.split(/\s+/).find(Boolean); + if (isHtmlElement(el)) { + const classes = el.className.split(/\s+/).filter(Boolean); + const firstClass = classes.find((className) => className !== "clip") ?? classes[0]; if (firstClass) return `.${firstClass}`; } return undefined; @@ -324,6 +334,178 @@ function buildTimelineElementKey(params: { return `${scope}:${params.id}:${params.fallbackIndex}`; } +function buildTimelineElementIdentity(params: { + preferredId?: string | null; + label: string; + fallbackIndex: number; + domId?: string; + selector?: string; + selectorIndex?: number; + sourceFile?: string; +}): { id: string; key: string } { + const id = + params.preferredId?.trim() || + buildTimelineElementKey({ + id: params.label, + fallbackIndex: params.fallbackIndex, + domId: params.domId, + selector: params.selector, + selectorIndex: params.selectorIndex, + sourceFile: params.sourceFile, + }); + const key = buildTimelineElementKey({ + id, + fallbackIndex: params.fallbackIndex, + domId: params.domId, + selector: params.selector, + selectorIndex: params.selectorIndex, + sourceFile: params.sourceFile, + }); + return { id, key }; +} + +function getTimelineElementIdentity(element: TimelineElement): string { + return element.key ?? element.id; +} + +function getTimelineDomNodes(doc: Document): Element[] { + const rootComp = doc.querySelector("[data-composition-id]"); + return Array.from(doc.querySelectorAll("[data-start]")).filter((node) => node !== rootComp); +} + +function numbersNearlyEqual(a: number, b: number): boolean { + return Math.abs(a - b) < 0.001; +} + +function nodeMatchesManifestClip(node: Element, clip: ClipManifestClip): boolean { + const tagName = clip.tagName?.toLowerCase(); + if (tagName && node.tagName.toLowerCase() !== tagName) return false; + + const start = Number.parseFloat(node.getAttribute("data-start") ?? ""); + if (Number.isFinite(start) && !numbersNearlyEqual(start, clip.start)) return false; + + const duration = Number.parseFloat(node.getAttribute("data-duration") ?? ""); + if (Number.isFinite(duration) && !numbersNearlyEqual(duration, clip.duration)) return false; + + const track = Number.parseInt(node.getAttribute("data-track-index") ?? "", 10); + if (Number.isFinite(track) && track !== clip.track) return false; + + return true; +} + +export function findTimelineDomNodeForClip( + doc: Document, + clip: ClipManifestClip, + fallbackIndex: number, + usedNodes = new Set(), +): Element | null { + const byIdentity = clip.id ? findTimelineDomNode(doc, clip.id) : null; + if (byIdentity && !usedNodes.has(byIdentity)) return byIdentity; + + const candidates = getTimelineDomNodes(doc).filter((node) => !usedNodes.has(node)); + const exact = candidates.find((node) => nodeMatchesManifestClip(node, clip)); + if (exact) return exact; + + return candidates[fallbackIndex] ?? null; +} + +export function createTimelineElementFromManifestClip(params: { + clip: ClipManifestClip; + fallbackIndex: number; + doc?: Document | null; + hostEl?: Element | null; +}): TimelineElement { + const { clip, fallbackIndex, doc } = params; + let hostEl = params.hostEl ?? null; + const label = getTimelineElementDisplayLabel({ + id: clip.id, + label: clip.label, + tag: clip.tagName || clip.kind, + }); + + let domId: string | undefined; + let selector: string | undefined; + let selectorIndex: number | undefined; + let sourceFile: string | undefined; + + if (hostEl) { + domId = hostEl.id || undefined; + selector = getTimelineElementSelector(hostEl); + selectorIndex = + doc && selector ? getTimelineElementSelectorIndex(doc, hostEl, selector) : undefined; + sourceFile = getTimelineElementSourceFile(hostEl); + } + + const identity = buildTimelineElementIdentity({ + preferredId: clip.id, + label, + fallbackIndex, + domId, + selector, + selectorIndex, + sourceFile, + }); + const entry: TimelineElement = { + id: identity.id, + label, + key: identity.key, + tag: clip.tagName || clip.kind, + start: clip.start, + duration: clip.duration, + track: clip.track, + domId, + selector, + selectorIndex, + sourceFile, + }; + + if (hostEl) { + applyMediaMetadataFromElement(entry, hostEl); + } + if (clip.assetUrl) entry.src = clip.assetUrl; + if (clip.kind === "composition" && clip.compositionId) { + let resolvedSrc = clip.compositionSrc; + if (!resolvedSrc) { + hostEl = doc?.querySelector(`[data-composition-id="${clip.compositionId}"]`) ?? hostEl; + resolvedSrc = + hostEl?.getAttribute("data-composition-src") ?? + hostEl?.getAttribute("data-composition-file") ?? + null; + } + if (resolvedSrc) { + entry.compositionSrc = resolvedSrc; + } else if (hostEl) { + const innerVideo = hostEl.querySelector("video[src]"); + if (innerVideo) { + entry.src = innerVideo.getAttribute("src") || undefined; + entry.tag = "video"; + } + } + if (hostEl) { + entry.domId = hostEl.id || undefined; + entry.selector = getTimelineElementSelector(hostEl); + entry.selectorIndex = + doc && entry.selector + ? getTimelineElementSelectorIndex(doc, hostEl, entry.selector) + : undefined; + entry.sourceFile = getTimelineElementSourceFile(hostEl); + const nextIdentity = buildTimelineElementIdentity({ + preferredId: clip.id, + label, + fallbackIndex, + domId: entry.domId, + selector: entry.selector, + selectorIndex: entry.selectorIndex, + sourceFile: entry.sourceFile, + }); + entry.id = nextIdentity.id; + entry.key = nextIdentity.key; + } + } + + return entry; +} + function findTimelineDomNode(doc: Document, id: string): Element | null { return ( doc.getElementById(id) ?? @@ -477,8 +659,10 @@ export function mergeTimelineElementsPreservingDowngrades( return nextElements; } - const nextIds = new Set(nextElements.map((element) => element.id)); - const preserved = currentElements.filter((element) => !nextIds.has(element.id)); + const nextIdentities = new Set(nextElements.map(getTimelineElementIdentity)); + const preserved = currentElements.filter( + (element) => !nextIdentities.has(getTimelineElementIdentity(element)), + ); if (preserved.length === 0) return nextElements; return [...nextElements, ...preserved]; } @@ -853,84 +1037,16 @@ export function useTimelinePlayer() { } const usedHostEls = new Set(); const els: TimelineElement[] = filtered.map((clip, index) => { - let hostEl = iframeDoc + const hostEl = iframeDoc ? findTimelineDomNodeForClip(iframeDoc, clip, index, usedHostEls) : null; if (hostEl) usedHostEls.add(hostEl); - const label = getTimelineElementDisplayLabel({ - id: clip.id, - label: clip.label, - tag: clip.tagName || clip.kind, - }); - const id = clip.id || label; - const entry: TimelineElement = { - id, - label, - tag: clip.tagName || clip.kind, - start: clip.start, - duration: clip.duration, - track: clip.track, - }; - if (hostEl) { - entry.domId = hostEl.id || undefined; - entry.selector = getTimelineElementSelector(hostEl); - entry.selectorIndex = - iframeDoc && entry.selector - ? getTimelineElementSelectorIndex(iframeDoc, hostEl, entry.selector) - : undefined; - entry.sourceFile = getTimelineElementSourceFile(hostEl); - applyMediaMetadataFromElement(entry, hostEl); - } - if (clip.assetUrl) entry.src = clip.assetUrl; - if (clip.kind === "composition" && clip.compositionId) { - // The bundler renames data-composition-src to data-composition-file - // after inlining, so the clip manifest may not have compositionSrc. - // Fall back to reading data-composition-file from the DOM. - let resolvedSrc = clip.compositionSrc; - let hostEl: Element | null = null; - if (!resolvedSrc) { - try { - const iframeDoc = iframeRef.current?.contentDocument; - hostEl = - iframeDoc?.querySelector(`[data-composition-id="${clip.compositionId}"]`) ?? hostEl; - resolvedSrc = - hostEl?.getAttribute("data-composition-src") ?? - hostEl?.getAttribute("data-composition-file") ?? - null; - } catch { - /* cross-origin */ - } - } - if (resolvedSrc) { - entry.compositionSrc = resolvedSrc; - } else if (hostEl) { - // Inline composition (no external file) — expose inner video for thumbnails - const innerVideo = hostEl.querySelector("video[src]"); - if (innerVideo) { - entry.src = innerVideo.getAttribute("src") || undefined; - entry.tag = "video"; - } - } - if (hostEl) { - const iframeDoc = iframeRef.current?.contentDocument; - entry.domId = hostEl.id || undefined; - entry.selector = getTimelineElementSelector(hostEl); - entry.selectorIndex = - iframeDoc && entry.selector - ? getTimelineElementSelectorIndex(iframeDoc, hostEl, entry.selector) - : undefined; - entry.sourceFile = getTimelineElementSourceFile(hostEl); - } - } - entry.key = buildTimelineElementKey({ - id, + return createTimelineElementFromManifestClip({ + clip, fallbackIndex: index, - domId: entry.domId, - selector: entry.selector, - selectorIndex: entry.selectorIndex, - sourceFile: entry.sourceFile, + doc: iframeDoc, + hostEl, }); - return entry; }); const rawDuration = data.durationInFrames / 30; // Clamp non-finite or absurdly large durations — the runtime can emit @@ -1049,18 +1165,19 @@ export function useTimelinePlayer() { label: el.getAttribute("data-timeline-label") ?? el.getAttribute("data-label"), tag: el.tagName, }); - const id = el.id || compId || label; + const identity = buildTimelineElementIdentity({ + preferredId: el.id || compId || null, + label, + fallbackIndex: missing.length, + domId: el.id || undefined, + selector, + selectorIndex, + sourceFile, + }); const entry: TimelineElement = { - id, + id: identity.id, label, - key: buildTimelineElementKey({ - id, - fallbackIndex: missing.length, - domId: el.id || undefined, - selector, - selectorIndex, - sourceFile, - }), + key: identity.key, tag: el.tagName.toLowerCase(), start, duration: dur, From 67efe6733a0ffdd67d654df0a332aca894a16ca5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Thu, 30 Apr 2026 00:46:38 -0400 Subject: [PATCH 3/3] fix(studio): address review follow-ups --- packages/core/src/lint/utils.ts | 12 ++++++-- .../player/hooks/useTimelinePlayer.test.ts | 29 +++++++++++++++++++ .../src/player/hooks/useTimelinePlayer.ts | 4 ++- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/packages/core/src/lint/utils.ts b/packages/core/src/lint/utils.ts index 7ff05ec5f..cf8a7b121 100644 --- a/packages/core/src/lint/utils.ts +++ b/packages/core/src/lint/utils.ts @@ -59,12 +59,18 @@ export function extractBlocks(source: string, pattern: RegExp): ExtractedBlock[] } export function findRootTag(source: string): OpenTag | null { - const bodyMatch = source.match(/]*>([\s\S]*?)<\/body>/i); - const bodyContent = bodyMatch ? (bodyMatch[1] ?? source) : source; + const bodyOpenMatch = /]*>/i.exec(source); + const bodyCloseMatch = /<\/body>/i.exec(source); + const bodyStart = bodyOpenMatch ? bodyOpenMatch.index + bodyOpenMatch[0].length : 0; + const bodyEnd = + bodyOpenMatch && bodyCloseMatch && bodyCloseMatch.index > bodyStart + ? bodyCloseMatch.index + : source.length; + const bodyContent = bodyOpenMatch ? source.slice(bodyStart, bodyEnd) : source; const bodyTags = extractOpenTags(bodyContent); for (const tag of bodyTags) { if (["script", "style", "meta", "link", "title"].includes(tag.name)) continue; - return tag; + return { ...tag, index: tag.index + bodyStart }; } return null; } diff --git a/packages/studio/src/player/hooks/useTimelinePlayer.test.ts b/packages/studio/src/player/hooks/useTimelinePlayer.test.ts index d5366af0d..2c89583f2 100644 --- a/packages/studio/src/player/hooks/useTimelinePlayer.test.ts +++ b/packages/studio/src/player/hooks/useTimelinePlayer.test.ts @@ -195,6 +195,35 @@ describe("anonymous timeline identity", () => { expect(new Set(elements.map((element) => element.key)).size).toBe(2); expect(elements.map((element) => element.selectorIndex)).toEqual([0, 1]); }); + + it("reads media metadata from owner-window media elements", () => { + const doc = createDocument(` +
+
+ +
+
+ `); + const hostEl = doc.querySelector(".video-card"); + const video = hostEl?.querySelector("video"); + if (!hostEl || !video) throw new Error("missing video test fixture"); + Object.defineProperty(video, "defaultPlaybackRate", { + value: 1.5, + configurable: true, + }); + + const element = createTimelineElementFromManifestClip({ + clip: createClip({ kind: "video", tagName: "div" }), + fallbackIndex: 0, + doc, + hostEl, + }); + + expect(element.tag).toBe("video"); + expect(element.src).toBe("/clip.mp4"); + expect(element.sourceDuration).toBe(12); + expect(element.playbackRate).toBe(1.5); + }); }); describe("mergeTimelineElementsPreservingDowngrades", () => { diff --git a/packages/studio/src/player/hooks/useTimelinePlayer.ts b/packages/studio/src/player/hooks/useTimelinePlayer.ts index 5e872664b..2cdcf0b2e 100644 --- a/packages/studio/src/player/hooks/useTimelinePlayer.ts +++ b/packages/studio/src/player/hooks/useTimelinePlayer.ts @@ -95,7 +95,9 @@ function applyMediaMetadataFromElement(entry: TimelineElement, el: Element): voi const src = mediaEl.getAttribute("src"); if (src) entry.src = src; - if (!(mediaEl instanceof HTMLMediaElement)) return; + const win = mediaEl.ownerDocument.defaultView ?? window; + const MediaElementCtor = win.HTMLMediaElement ?? globalThis.HTMLMediaElement; + if (typeof MediaElementCtor === "undefined" || !(mediaEl instanceof MediaElementCtor)) return; const sourceDurationAttr = el.getAttribute("data-source-duration") ?? mediaEl.getAttribute("data-source-duration");