From fb6351582145005a84af34aec47a2d35cb21f3d7 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Tue, 18 Aug 2026 17:42:55 -0400 Subject: [PATCH 1/2] fix(studio): let hfId-only clips move and trim on the timeline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dragging a clip that has no author-written id showed "This clip can't be moved or resized from the timeline yet", even though Studio could already patch that exact element from the canvas. hasPatchableTimelineTarget only accepted domId or selector. Studio stamps data-hf-id into the source, TimelineElement already carries hfId, and findElementForSelection resolves data-hf-id ahead of id and selector, so the write path supported these clips the whole time — only the capability gate did not. resolveDomEditCapabilities already treats hfId as a stable target, so the two paths disagreed about the same element. Accept hfId in the gate, and pass it through the two call sites that map TimelineElement fields by hand (canMoveTimelineElement, canTrimEdge); the call sites that forward the whole element already had it. Clips with implicit timing or on a locked row stay blocked. --- .../components/timelineAuthoredMoveTarget.ts | 1 + .../components/timelineEditCapabilities.ts | 14 +++++- .../player/components/timelineEditing.test.ts | 46 +++++++++++++++++++ .../player/components/timelineGroupEditing.ts | 1 + 4 files changed, 60 insertions(+), 2 deletions(-) diff --git a/packages/studio/src/player/components/timelineAuthoredMoveTarget.ts b/packages/studio/src/player/components/timelineAuthoredMoveTarget.ts index 6dc93c7968..c0adffcdfb 100644 --- a/packages/studio/src/player/components/timelineAuthoredMoveTarget.ts +++ b/packages/studio/src/player/components/timelineAuthoredMoveTarget.ts @@ -11,6 +11,7 @@ export function canMoveTimelineElement(element: TimelineElement): boolean { duration: element.duration, domId: element.domId, selector: element.selector, + hfId: element.hfId, compositionSrc: element.compositionSrc, playbackStart: element.playbackStart, playbackStartAttr: element.playbackStartAttr, diff --git a/packages/studio/src/player/components/timelineEditCapabilities.ts b/packages/studio/src/player/components/timelineEditCapabilities.ts index 9d211f4a2f..061faf58ae 100644 --- a/packages/studio/src/player/components/timelineEditCapabilities.ts +++ b/packages/studio/src/player/components/timelineEditCapabilities.ts @@ -23,8 +23,17 @@ function isDeterministicTimelineWindow(input: { return ["video", "audio", "img"].includes(input.tag.toLowerCase()); } -export function hasPatchableTimelineTarget(input: { domId?: string; selector?: string }): boolean { - return Boolean(input.domId || input.selector); +export function hasPatchableTimelineTarget(input: { + domId?: string; + selector?: string; + hfId?: string; +}): boolean { + // hfId counts as a stable target: Studio stamps data-hf-id into the source and + // findElementForSelection resolves it before id/selector, so a clip carrying only + // an hfId is just as patchable as one with an author-written id. Omitting it here + // made those clips report canMove:false and surface "This clip can't be moved or + // resized from the timeline yet", even though the write path fully supported them. + return Boolean(input.domId || input.selector || input.hfId); } export function getTimelineEditCapabilities(input: { @@ -33,6 +42,7 @@ export function getTimelineEditCapabilities(input: { duration: number; domId?: string; selector?: string; + hfId?: string; compositionSrc?: string; playbackStart?: number; playbackStartAttr?: "media-start" | "playback-start"; diff --git a/packages/studio/src/player/components/timelineEditing.test.ts b/packages/studio/src/player/components/timelineEditing.test.ts index 50b749c153..d3d2943db5 100644 --- a/packages/studio/src/player/components/timelineEditing.test.ts +++ b/packages/studio/src/player/components/timelineEditing.test.ts @@ -377,12 +377,58 @@ describe("hasPatchableTimelineTarget", () => { expect(hasPatchableTimelineTarget({ selector: ".hero-card" })).toBe(true); }); + it("returns true when the clip only has an hfId", () => { + // Studio stamps data-hf-id into the source and resolves it ahead of id/selector, + // so an hfId-only clip is patchable. Regression: it used to report false, which + // blocked dragging any authored clip the user had not given an id. + expect(hasPatchableTimelineTarget({ hfId: "hf-6wbt" })).toBe(true); + }); + it("returns false when the clip has no stable patch target", () => { expect(hasPatchableTimelineTarget({})).toBe(false); }); }); describe("getTimelineEditCapabilities", () => { + it("lets an hfId-only authored clip move and trim", () => { + // A plain
with no author id: the + // exact shape that produced "This clip can't be moved or resized from the + // timeline yet" while the DOM-edit path considered the same element editable. + expect( + getTimelineEditCapabilities({ + tag: "div", + kind: "element", + duration: 2.6, + hfId: "hf-opwy", + timingSource: "authored", + }), + ).toEqual({ canMove: true, canTrimStart: true, canTrimEnd: true }); + }); + + it("still refuses an hfId-only clip whose timing is implicit", () => { + expect( + getTimelineEditCapabilities({ + tag: "div", + kind: "element", + duration: 2.6, + hfId: "hf-opwy", + timingSource: "implicit", + }).canMove, + ).toBe(false); + }); + + it("still refuses an hfId-only clip on a locked row", () => { + expect( + getTimelineEditCapabilities({ + tag: "div", + kind: "element", + duration: 2.6, + hfId: "hf-opwy", + timelineLocked: true, + }).canMove, + ).toBe(false); + }); + it("does not disable editable audio just because it spans multiple scenes", () => { expect( getTimelineEditCapabilities({ diff --git a/packages/studio/src/player/components/timelineGroupEditing.ts b/packages/studio/src/player/components/timelineGroupEditing.ts index 567a98cd3e..76e4dac886 100644 --- a/packages/studio/src/player/components/timelineGroupEditing.ts +++ b/packages/studio/src/player/components/timelineGroupEditing.ts @@ -193,6 +193,7 @@ function canTrimEdge(element: TimelineElement, edge: TimelineGroupResizeEdge): b duration: element.duration, domId: element.domId, selector: element.selector, + hfId: element.hfId, compositionSrc: element.compositionSrc, playbackStart: element.playbackStart, playbackStartAttr: element.playbackStartAttr, From 85b9b2e248b37de71e57e255442da8080d7430cf Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Tue, 18 Aug 2026 18:29:31 -0400 Subject: [PATCH 2/2] fix(studio): stop the positional fallback stealing another clip's DOM node MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is the actual cause of "This clip can't be moved or resized from the timeline yet" on well-formed clips. findTimelineDomNodeForClip falls back to candidates[fallbackIndex] when a clip matches nothing by identity or attributes. Candidates are only nodes carrying [data-start], so a clip whose element has none — an expanded child row, for instance — is never among them, and the fallback hands it an unrelated node. That corrupts two clips per collision: the caller gets a host that is not its element, and the clip that owned that node is starved to null, losing hfId, domId and selector, and with them canMove. Observed on a six-scene composition: two img clips took the scene divs at 6.0s and 11.2s positionally, and those two scenes were the only ones the timeline refused to move. Require the positional candidate to share the clip's tag. Same-tag attribute drift still resolves, which is what the fallback is for. Verified end to end: the previously refused clip now drags and its new data-start is written back to the source file. --- .../studio/src/player/lib/timelineDOM.test.ts | 68 +++++++++++++++++++ .../src/player/lib/timelineElementHelpers.ts | 13 +++- 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/packages/studio/src/player/lib/timelineDOM.test.ts b/packages/studio/src/player/lib/timelineDOM.test.ts index 78a0bc7ec6..82275f5735 100644 --- a/packages/studio/src/player/lib/timelineDOM.test.ts +++ b/packages/studio/src/player/lib/timelineDOM.test.ts @@ -6,6 +6,8 @@ import { createImplicitTimelineLayersFromDOM, mergeTimelineElementsPreservingDowngrades, } from "./timelineDOM"; +import { findTimelineDomNodeForClip } from "./timelineElementHelpers"; +import type { ClipManifestClip } from "./playbackTypes"; import type { TimelineElement } from "../store/playerStore"; function el(id: string, extra: Partial = {}): TimelineElement { @@ -329,3 +331,69 @@ describe("audio FX attributes on parsed elements", () => { expect(bgm?.automation).toBeUndefined(); }); }); + +describe("findTimelineDomNodeForClip — positional fallback never crosses tags", () => { + // Regression: a clip whose own element carries no [data-start] is not a + // candidate here. Before the tag guard it took candidates[fallbackIndex] — an + // unrelated scene
— which corrupted two clips at once: the img clip got a + // host that was not its element, and the div clip that owned that node was + // starved to null, losing hfId/domId/selector and therefore canMove. That is + // what surfaced as "This clip can't be moved or resized from the timeline yet". + const html = ` +
+
+
+
`; + + const imgClip: ClipManifestClip = { + id: null, + label: "Site Hero", + start: 0, + duration: 14, + track: 2, + kind: "image", + tagName: "img", + compositionId: null, + parentCompositionId: null, + compositionSrc: null, + assetUrl: null, + }; + + it("returns null instead of stealing a div for an img clip", () => { + const doc = makeDoc(html); + expect(findTimelineDomNodeForClip(doc, imgClip, 0)).toBeNull(); + }); + + it("leaves the div clips resolvable after an unmatched img clip", () => { + const doc = makeDoc(html); + const used = new Set(); + const stolen = findTimelineDomNodeForClip(doc, imgClip, 0, used); + if (stolen) used.add(stolen); + + const divClip: ClipManifestClip = { + ...imgClip, + label: "S", + start: 6, + duration: 2.6, + track: 1, + kind: "element", + tagName: "div", + }; + const host = findTimelineDomNodeForClip(doc, divClip, 1, used); + expect(host?.getAttribute("data-hf-id")).toBe("hf-b"); + }); + + it("still allows a same-tag positional fallback when attributes drift", () => { + const doc = makeDoc(html); + const drifted: ClipManifestClip = { + ...imgClip, + label: "S", + start: 99, + duration: 99, + track: 9, + kind: "element", + tagName: "div", + }; + expect(findTimelineDomNodeForClip(doc, drifted, 0)?.getAttribute("data-hf-id")).toBe("hf-a"); + }); +}); diff --git a/packages/studio/src/player/lib/timelineElementHelpers.ts b/packages/studio/src/player/lib/timelineElementHelpers.ts index fc403f9b2f..7a2dcc2c33 100644 --- a/packages/studio/src/player/lib/timelineElementHelpers.ts +++ b/packages/studio/src/player/lib/timelineElementHelpers.ts @@ -419,7 +419,18 @@ export function findTimelineDomNodeForClip( const exact = candidates.find((node) => nodeMatchesManifestClip(node, clip)); if (exact) return exact; - return candidates[fallbackIndex] ?? null; + // Positional fallback, but never across tags. A clip whose element carries no + // [data-start] of its own (an expanded child row, say) is not a candidate here, + // so without this guard it takes candidates[fallbackIndex] — an unrelated node — + // and corrupts TWO clips at once: the thief gets a host that is not its element, + // and the rightful owner is starved to null, losing hfId/domId/selector and with + // them canMove. That is what produced "This clip can't be moved or resized from + // the timeline yet" on clips that were perfectly well-formed. + const positional = candidates[fallbackIndex]; + if (!positional) return null; + const clipTag = clip.tagName?.toLowerCase(); + if (clipTag && positional.tagName.toLowerCase() !== clipTag) return null; + return positional; } // ---------------------------------------------------------------------------