From 4916d6580cfe30711c45d5084494ca7f55591dcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Tue, 19 May 2026 12:42:46 -0400 Subject: [PATCH] fix(studio): stop injecting inline z-index on all clips during timeline edits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Timeline move, delete, and asset-drop operations were looping over every clip in the file and writing style="z-index: N" derived from an inverted data-track-index mapping. This silently overrode the author's CSS z-index — contradicting the documented contract that data-track-index does not affect visual layering — and persisted the corruption in the source HTML. Remove the z-index injection loops from all three timeline commit paths. Move and delete now only patch timing/track attributes on the affected clip. Asset drop still sets z-index on the newly created element via the generated HTML, without touching existing clips. Delete the now-unused buildTrackZIndexMap helper. Also fix patchInlineStyleInTag to handle self-closing void elements: the old code produced malformed `` because it didn't account for the trailing `/` before appending the style attribute. Closes #958 --- .filesize-allowlist | 2 + .../studio/src/hooks/useTimelineEditing.ts | 80 +-------- .../player/components/timelineEditing.test.ts | 24 --- .../src/player/components/timelineEditing.ts | 6 - packages/studio/src/utils/blockInstaller.ts | 8 +- .../studio/src/utils/sourcePatcher.test.ts | 22 +++ packages/studio/src/utils/sourcePatcher.ts | 6 +- .../src/utils/timelineZIndexInjection.test.ts | 155 ++++++++++++++++++ 8 files changed, 191 insertions(+), 112 deletions(-) create mode 100644 packages/studio/src/utils/timelineZIndexInjection.test.ts diff --git a/.filesize-allowlist b/.filesize-allowlist index ba27c83d5..94f488521 100644 --- a/.filesize-allowlist +++ b/.filesize-allowlist @@ -5,5 +5,7 @@ packages/studio/src/components/editor/manualEdits.test.ts packages/studio/src/player/hooks/useTimelinePlayer.test.ts packages/studio/src/components/editor/manualEditsDom.ts packages/studio/src/utils/sourcePatcher.ts +packages/studio/src/utils/sourcePatcher.test.ts packages/studio/src/App.tsx packages/studio/src/player/components/Timeline.tsx +packages/studio/src/player/components/timelineEditing.test.ts diff --git a/packages/studio/src/hooks/useTimelineEditing.ts b/packages/studio/src/hooks/useTimelineEditing.ts index 0dd88bb02..b2267378d 100644 --- a/packages/studio/src/hooks/useTimelineEditing.ts +++ b/packages/studio/src/hooks/useTimelineEditing.ts @@ -2,10 +2,7 @@ import { useCallback, useRef } from "react"; import type { TimelineElement } from "../player"; import { usePlayerStore } from "../player"; import { applyPatchByTarget, readAttributeByTarget } from "../utils/sourcePatcher"; -import { - buildTrackZIndexMap, - formatTimelineAttributeNumber, -} from "../player/components/timelineEditing"; +import { formatTimelineAttributeNumber } from "../player/components/timelineEditing"; import { buildTimelineAssetId, buildTimelineAssetInsertHtml, @@ -101,16 +98,6 @@ export function useTimelineEditing({ throw new Error(`Timeline element ${element.id} is missing a patchable target`); } - const resolvedTargetPath = targetPath || "index.html"; - const relevantElements = timelineElements - .map((te) => - (te.key ?? te.id) === (element.key ?? element.id) - ? { ...te, start: updates.start, track: updates.track } - : te, - ) - .filter((te) => (te.sourceFile || activeCompPath || "index.html") === resolvedTargetPath); - const trackZIndices = buildTrackZIndexMap(relevantElements.map((te) => te.track)); - let patchedContent = applyPatchByTarget(originalContent, patchTarget, { type: "attribute", property: "start", @@ -121,17 +108,6 @@ export function useTimelineEditing({ property: "track-index", value: String(updates.track), }); - for (const te of relevantElements) { - const elementTarget = buildPatchTarget(te); - if (!elementTarget) continue; - const nextZIndex = trackZIndices.get(te.track); - if (nextZIndex == null) continue; - patchedContent = applyPatchByTarget(patchedContent, elementTarget, { - type: "inline-style", - property: "z-index", - value: String(nextZIndex), - }); - } if (patchedContent === originalContent) { throw new Error(`Unable to patch timeline element ${element.id} in ${targetPath}`); @@ -150,14 +126,7 @@ export function useTimelineEditing({ reloadPreview(); }, - [ - activeCompPath, - recordEdit, - timelineElements, - writeProjectFile, - domEditSaveTimestampRef, - reloadPreview, - ], + [activeCompPath, recordEdit, writeProjectFile, domEditSaveTimestampRef, reloadPreview], ); const handleTimelineElementResize = useCallback( @@ -247,14 +216,6 @@ export function useTimelineEditing({ throw new Error(`Timeline element ${element.id} is missing a patchable target`); } - const resolvedTargetPath = targetPath || "index.html"; - const remainingElements = timelineElements.filter( - (te) => - (te.key ?? te.id) !== (element.key ?? element.id) && - (te.sourceFile || activeCompPath || "index.html") === resolvedTargetPath, - ); - const trackZIndices = buildTrackZIndexMap(remainingElements.map((te) => te.track)); - const removeResponse = await fetch( `/api/projects/${pid}/file-mutations/remove-element/${encodeURIComponent(targetPath)}`, { @@ -271,19 +232,8 @@ export function useTimelineEditing({ changed?: boolean; content?: string; }; - let patchedContent = + const patchedContent = typeof removeData.content === "string" ? removeData.content : originalContent; - for (const te of remainingElements) { - const elementTarget = buildPatchTarget(te); - if (!elementTarget) continue; - const nextZIndex = trackZIndices.get(te.track); - if (nextZIndex == null) continue; - patchedContent = applyPatchByTarget(patchedContent, elementTarget, { - type: "inline-style", - property: "z-index", - value: String(nextZIndex), - }); - } domEditSaveTimestampRef.current = Date.now(); await saveProjectFilesWithHistory({ @@ -352,26 +302,10 @@ export function useTimelineEditing({ const relevantElements = timelineElements.filter( (te) => (te.sourceFile || activeCompPath || "index.html") === resolvedTargetPath, ); - const trackZIndices = buildTrackZIndexMap([ - ...relevantElements.map((te) => te.track), - placement.track, - ]); - - let patchedContent = originalContent; - for (const te of relevantElements) { - const elementTarget = buildPatchTarget(te); - if (!elementTarget) continue; - const nextZIndex = trackZIndices.get(te.track); - if (nextZIndex == null) continue; - patchedContent = applyPatchByTarget(patchedContent, elementTarget, { - type: "inline-style", - property: "z-index", - value: String(nextZIndex), - }); - } + const newElementZIndex = Math.max(1, relevantElements.length + 1); - patchedContent = insertTimelineAssetIntoSource( - patchedContent, + const patchedContent = insertTimelineAssetIntoSource( + originalContent, buildTimelineAssetInsertHtml({ id: newId, assetPath: resolvedAssetSrc, @@ -379,7 +313,7 @@ export function useTimelineEditing({ start: normalizedStart, duration: normalizedDuration, track: placement.track, - zIndex: trackZIndices.get(placement.track) ?? 1, + zIndex: newElementZIndex, geometry: resolveTimelineAssetInitialGeometry(originalContent), }), ); diff --git a/packages/studio/src/player/components/timelineEditing.test.ts b/packages/studio/src/player/components/timelineEditing.test.ts index 1847190c2..68df1be9a 100644 --- a/packages/studio/src/player/components/timelineEditing.test.ts +++ b/packages/studio/src/player/components/timelineEditing.test.ts @@ -4,7 +4,6 @@ import { buildPromptCopyText, buildTimelineElementAgentPrompt, buildTimelineAgentPrompt, - buildTrackZIndexMap, canOffsetTrimClipStart, getTimelineEditCapabilities, hasPatchableTimelineTarget, @@ -159,29 +158,6 @@ describe("resolveTimelineMove", () => { }); }); -describe("buildTrackZIndexMap", () => { - it("maps visually higher tracks onto higher z-index values", () => { - expect(buildTrackZIndexMap([-2, -1, 0, 3])).toEqual( - new Map([ - [-2, 4], - [-1, 3], - [0, 2], - [3, 1], - ]), - ); - }); - - it("deduplicates tracks before assigning z-index values", () => { - expect(buildTrackZIndexMap([-1, 0, -1, 3, 3])).toEqual( - new Map([ - [-1, 3], - [0, 2], - [3, 1], - ]), - ); - }); -}); - describe("canOffsetTrimClipStart", () => { it("allows front trim for clips that carry playback offset metadata", () => { expect( diff --git a/packages/studio/src/player/components/timelineEditing.ts b/packages/studio/src/player/components/timelineEditing.ts index b775690ae..d05d44a90 100644 --- a/packages/studio/src/player/components/timelineEditing.ts +++ b/packages/studio/src/player/components/timelineEditing.ts @@ -114,12 +114,6 @@ export function resolveTimelineMove( }; } -export function buildTrackZIndexMap(tracks: number[]): Map { - const uniqueTracks = Array.from(new Set(tracks)).sort((a, b) => a - b); - const maxZIndex = uniqueTracks.length; - return new Map(uniqueTracks.map((track, index) => [track, maxZIndex - index])); -} - export function resolveTimelineResize( input: TimelineResizeInput, edge: "start" | "end", diff --git a/packages/studio/src/utils/blockInstaller.ts b/packages/studio/src/utils/blockInstaller.ts index cd64e66af..fb62ce46b 100644 --- a/packages/studio/src/utils/blockInstaller.ts +++ b/packages/studio/src/utils/blockInstaller.ts @@ -5,10 +5,7 @@ import { resolveTimelineAssetInitialGeometry, } from "./timelineAssetDrop"; import { collectHtmlIds } from "./studioHelpers"; -import { - buildTrackZIndexMap, - formatTimelineAttributeNumber, -} from "../player/components/timelineEditing"; +import { formatTimelineAttributeNumber } from "../player/components/timelineEditing"; import { saveProjectFilesWithHistory } from "./studioFileHistory"; import type { EditHistoryKind } from "./editHistory"; @@ -127,8 +124,7 @@ export async function addBlockToProject( ? Math.max(...relevantElements.map((te) => te.track)) + 1 : 1); - const trackZIndices = buildTrackZIndexMap([...relevantElements.map((te) => te.track), track]); - const zIndex = trackZIndices.get(track) ?? 1; + const zIndex = Math.max(1, relevantElements.length + 1); const width = isBlock ? (block as { dimensions: { width: number } }).dimensions.width diff --git a/packages/studio/src/utils/sourcePatcher.test.ts b/packages/studio/src/utils/sourcePatcher.test.ts index 7e8787eba..8bc64e1b8 100644 --- a/packages/studio/src/utils/sourcePatcher.test.ts +++ b/packages/studio/src/utils/sourcePatcher.test.ts @@ -35,6 +35,28 @@ describe("applyPatchByTarget", () => { ); }); + it("adds inline style to a self-closing void element without malforming it", () => { + const html = `earth`; + const op: PatchOperation = { type: "inline-style", property: "z-index", value: "3" }; + + const result = applyPatch(html, "gif-img", op); + expect(result).toBe( + `earth`, + ); + expect(result).not.toContain("/ style"); + }); + + it("adds inline style to a self-closing void element matched by selector", () => { + const html = ``; + const op: PatchOperation = { type: "inline-style", property: "opacity", value: "0.5" }; + + const result = applyPatchByTarget(html, { selector: ".hero" }, op); + expect(result).toBe( + ``, + ); + expect(result).not.toContain("/ style"); + }); + it("patches inline move styles by target", () => { const html = `
`; diff --git a/packages/studio/src/utils/sourcePatcher.ts b/packages/studio/src/utils/sourcePatcher.ts index 0ef7fb7f1..2d89eea2e 100644 --- a/packages/studio/src/utils/sourcePatcher.ts +++ b/packages/studio/src/utils/sourcePatcher.ts @@ -203,9 +203,9 @@ function patchInlineStyleInTag( } else { // No existing style attribute if (value === null) return html; // nothing to remove - // Add one - const newTag = - tag.replace(/>$/, "") + ` style="${prop}: ${escapeStyleAttributeValue(value, '"')}"`; + const selfClosing = /\s*\/$/.test(tag); + const base = selfClosing ? tag.replace(/\s*\/$/, "") : tag; + const newTag = `${base} style="${prop}: ${escapeStyleAttributeValue(value, '"')}"${selfClosing ? " /" : ""}`; return html.replace(tag, newTag); } } diff --git a/packages/studio/src/utils/timelineZIndexInjection.test.ts b/packages/studio/src/utils/timelineZIndexInjection.test.ts new file mode 100644 index 000000000..e6a673c5b --- /dev/null +++ b/packages/studio/src/utils/timelineZIndexInjection.test.ts @@ -0,0 +1,155 @@ +import { describe, expect, it } from "vitest"; +import { applyPatchByTarget, applyPatch } from "./sourcePatcher"; + +/** + * Reproduction tests for https://github.com/heygen-com/hyperframes/issues/958 + * + * The bug: dragging a clip in the Studio timeline rewrites index.html with + * inline style="z-index: N" on EVERY clip, overriding the author's CSS z-index. + * Additionally, void elements (img, audio) get malformed self-closing tags. + */ + +const ISSUE_HTML = ` + +
+ +
TITLE
+
`; + +const VOID_ELEMENT_HTML = `
+ + rotating earth gif +
TITLE
+
`; + +describe("issue #958 — timeline drag must not inject inline z-index", () => { + it("reproduces the old bug: z-index injection on all clips overrides CSS layering", () => { + // Simulate the OLD behavior: buildTrackZIndexMap + loop over all clips + function buildTrackZIndexMap(tracks: number[]): Map { + const uniqueTracks = Array.from(new Set(tracks)).sort((a, b) => a - b); + const maxZIndex = uniqueTracks.length; + return new Map(uniqueTracks.map((track, index) => [track, maxZIndex - index])); + } + + const elements = [ + { id: "bg-video", track: 0 }, + { id: "title", track: 1 }, + ]; + const trackZIndices = buildTrackZIndexMap(elements.map((e) => e.track)); + + // Apply the old z-index injection loop + let broken = ISSUE_HTML; + for (const el of elements) { + const nextZIndex = trackZIndices.get(el.track); + if (nextZIndex == null) continue; + broken = applyPatch(broken, el.id, { + type: "inline-style", + property: "z-index", + value: String(nextZIndex), + }); + } + + // Verify the bug: bg-video gets z-index: 2, title gets z-index: 1 + // This INVERTS the intended layering (CSS had bg-video: 0, title: 20) + expect(broken).toContain('id="bg-video"'); + expect(broken).toContain('style="z-index: 2"'); + expect(broken).toContain('id="title"'); + expect(broken).toContain('style="z-index: 1"'); + + // The title (z-index: 1) is now BEHIND the video (z-index: 2) + // — the opposite of the author's intent (CSS z-index: 20 vs 0) + }); + + it("verifies the fix: moving a clip only patches data-start and data-track-index", () => { + // Simulate the NEW behavior: only patch the moved clip's timing attributes + const movedElement = { id: "bg-video" }; + const updates = { start: "2.5", track: "0" }; + + let fixed = applyPatchByTarget( + ISSUE_HTML, + { id: movedElement.id }, + { type: "attribute", property: "start", value: updates.start }, + ); + fixed = applyPatchByTarget( + fixed, + { id: movedElement.id }, + { type: "attribute", property: "track-index", value: updates.track }, + ); + + // The moved clip's timing changed + expect(fixed).toContain('id="bg-video" data-start="2.5" data-track-index="0"'); + + // No inline z-index was injected on ANY element + expect(fixed).not.toContain('style="z-index'); + + // The title clip is completely untouched + expect(fixed).toContain( + '
TITLE
', + ); + + // CSS z-index declarations in