From 19fbaeb2963afd146837f92a6674093a4a5fc257 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sun, 7 Jun 2026 23:34:13 -0700 Subject: [PATCH 1/3] fix(core): splitElementInHtml data-end + text-content inner wrapper --- .../src/studio-api/helpers/sourceMutation.ts | 45 ++++++++++++++++--- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/packages/core/src/studio-api/helpers/sourceMutation.ts b/packages/core/src/studio-api/helpers/sourceMutation.ts index 8cdc5ae05..802525ec9 100644 --- a/packages/core/src/studio-api/helpers/sourceMutation.ts +++ b/packages/core/src/studio-api/helpers/sourceMutation.ts @@ -195,7 +195,15 @@ export function patchElementInHtml( } break; case "text-content": - if (op.value != null) htmlEl.textContent = op.value; + if (op.value != null) { + // The generator wraps text in an inner
; set content there to preserve structure. + const inner = htmlEl.firstElementChild; + const textTarget = + inner && inner.tagName.toLowerCase() === "div" + ? (inner as unknown as HTMLElement) + : htmlEl; + textTarget.textContent = op.value; + } break; } } @@ -219,6 +227,34 @@ export interface SplitElementResult { newId: string | null; } +function resolveElementTiming(el: Element): { + start: number; + duration: number; + usesDataEnd: boolean; +} { + const start = parseFloat(el.getAttribute("data-start") ?? "0") || 0; + // Generator writes data-end; legacy elements use data-duration. Support both. + const usesDataEnd = el.hasAttribute("data-end"); + const duration = usesDataEnd + ? parseFloat(el.getAttribute("data-end") ?? "") - start || 0 + : parseFloat(el.getAttribute("data-duration") ?? "0") || 0; + return { start, duration, usesDataEnd }; +} + +function setElementDuration( + el: Element, + start: number, + duration: number, + usesDataEnd: boolean, +): void { + const rounded = String(Math.round((start + duration) * 1000) / 1000); + if (usesDataEnd) { + el.setAttribute("data-end", rounded); + } else { + el.setAttribute("data-duration", String(Math.round(duration * 1000) / 1000)); + } +} + export function splitElementInHtml( source: string, target: SourceMutationTarget, @@ -229,8 +265,7 @@ export function splitElementInHtml( const el = findTargetElement(document, target); if (!el || !isHTMLElement(el)) return { html: source, matched: false, newId: null }; - const start = parseFloat(el.getAttribute("data-start") ?? "0") || 0; - const duration = parseFloat(el.getAttribute("data-duration") ?? "0") || 0; + const { start, duration, usesDataEnd } = resolveElementTiming(el); if (duration <= 0 || splitTime <= start || splitTime >= start + duration) { return { html: source, matched: false, newId: null }; } @@ -241,7 +276,7 @@ export function splitElementInHtml( const clone = el.cloneNode(true) as HTMLElement; clone.setAttribute("id", newId); clone.setAttribute("data-start", String(Math.round(splitTime * 1000) / 1000)); - clone.setAttribute("data-duration", String(Math.round(secondDuration * 1000) / 1000)); + setElementDuration(clone, splitTime, secondDuration, usesDataEnd); // Adjust media trim offset for the second half const playbackStartAttr = el.hasAttribute("data-playback-start") @@ -259,7 +294,7 @@ export function splitElementInHtml( } // Trim the original element's duration - el.setAttribute("data-duration", String(Math.round(firstDuration * 1000) / 1000)); + setElementDuration(el, start, firstDuration, usesDataEnd); // Insert clone after original if (el.nextSibling) { From 370a719bb884d57e10eac1ca0f10c7cece350648 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Mon, 8 Jun 2026 00:39:38 -0700 Subject: [PATCH 2/3] fix(core): review fixes for setElementDuration + text-content + playback-rate - setElementDuration: remove stale sibling timing attr on write - text-content: target single element child regardless of tag name - data-playback-rate: Number.isFinite guard replaces || 1 - rename `rounded` to `endTime` (it was an end-time, not a duration) Co-Authored-By: Claude Sonnet 4.6 --- .../src/studio-api/helpers/sourceMutation.ts | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/core/src/studio-api/helpers/sourceMutation.ts b/packages/core/src/studio-api/helpers/sourceMutation.ts index 802525ec9..a95df70d0 100644 --- a/packages/core/src/studio-api/helpers/sourceMutation.ts +++ b/packages/core/src/studio-api/helpers/sourceMutation.ts @@ -196,12 +196,10 @@ export function patchElementInHtml( break; case "text-content": if (op.value != null) { - // The generator wraps text in an inner
; set content there to preserve structure. - const inner = htmlEl.firstElementChild; - const textTarget = - inner && inner.tagName.toLowerCase() === "div" - ? (inner as unknown as HTMLElement) - : htmlEl; + // The generator wraps text in a single inner element; target it to preserve outer structure. + // Only unwrap one level when there is exactly one element child (the text container). + const inner = htmlEl.children.length === 1 ? htmlEl.firstElementChild : null; + const textTarget = inner ? (inner as unknown as HTMLElement) : htmlEl; textTarget.textContent = op.value; } break; @@ -247,11 +245,13 @@ function setElementDuration( duration: number, usesDataEnd: boolean, ): void { - const rounded = String(Math.round((start + duration) * 1000) / 1000); if (usesDataEnd) { - el.setAttribute("data-end", rounded); + const endTime = String(Math.round((start + duration) * 1000) / 1000); + el.setAttribute("data-end", endTime); + el.removeAttribute("data-duration"); // clean up legacy sibling attr } else { el.setAttribute("data-duration", String(Math.round(duration * 1000) / 1000)); + el.removeAttribute("data-end"); // clean up if previously migrated } } @@ -286,7 +286,8 @@ export function splitElementInHtml( : null; if (playbackStartAttr) { const currentTrim = parseFloat(el.getAttribute(playbackStartAttr) ?? "0") || 0; - const rate = parseFloat(el.getAttribute("data-playback-rate") ?? "1") || 1; + const rateRaw = parseFloat(el.getAttribute("data-playback-rate") ?? ""); + const rate = Number.isFinite(rateRaw) ? rateRaw : 1; clone.setAttribute( playbackStartAttr, String(Math.round((currentTrim + firstDuration * rate) * 1000) / 1000), From b72dc5af52f93d15044de79ca289d11dca3fba95 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Mon, 8 Jun 2026 01:14:13 -0700 Subject: [PATCH 3/3] test(core): add splitElementInHtml + text-content regression tests (#1273) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Covers the three bugs fixed in the parent commit: - setElementDuration sibling cleanup (data-end removes data-duration, vice versa) - text-content op with non-div inner wrappers (p, span) - data-playback-rate absent → trim offset must not be NaN Co-Authored-By: Claude Sonnet 4.6 --- .../studio-api/helpers/sourceMutation.test.ts | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/packages/core/src/studio-api/helpers/sourceMutation.test.ts b/packages/core/src/studio-api/helpers/sourceMutation.test.ts index c23b45911..256310b0d 100644 --- a/packages/core/src/studio-api/helpers/sourceMutation.test.ts +++ b/packages/core/src/studio-api/helpers/sourceMutation.test.ts @@ -3,6 +3,7 @@ import { removeElementFromHtml, patchElementInHtml, probeElementInSource, + splitElementInHtml, } from "./sourceMutation.js"; describe("removeElementFromHtml", () => { @@ -126,6 +127,29 @@ describe("patchElementInHtml", () => { expect(result).not.toContain("Hello World"); }); + it("patches text content when inner wrapper is

not

", () => { + const html = `

Original

`; + const { html: result, matched } = patchElementInHtml(html, { id: "el" }, [ + { type: "text-content", property: "", value: "Replaced" }, + ]); + + expect(matched).toBe(true); + expect(result).toContain("Replaced"); + expect(result).not.toContain("Original"); + // Outer div structure preserved — p tag still wraps the text + expect(result).toMatch(/]*>Replaced<\/p>/); + }); + + it("patches text content when inner wrapper is ", () => { + const html = `
Original
`; + const { html: result } = patchElementInHtml(html, { id: "el" }, [ + { type: "text-content", property: "", value: "Replaced" }, + ]); + + expect(result).toContain("Replaced"); + expect(result).toMatch(/]*>Replaced<\/span>/); + }); + it("applies multiple operations in one call", () => { const { html: result } = patchElementInHtml(FIXTURE, { id: "hero" }, [ { type: "inline-style", property: "color", value: "blue" }, @@ -362,6 +386,61 @@ describe("probeElementInSource", () => { }); }); +describe("splitElementInHtml", () => { + it("splits a data-end element and removes stale data-duration from both halves", () => { + const html = `
Text
`; + const { html: result, matched, newId } = splitElementInHtml(html, { id: "el" }, 5, "el-b"); + + expect(matched).toBe(true); + expect(newId).toBe("el-b"); + // First half: data-end="5", no data-duration + expect(result).toMatch(/id="el"[^>]*data-end="5"/); + expect(result).not.toMatch(/id="el"[^>]*data-duration/); + // Second half: data-start="5", data-end="10", no data-duration + expect(result).toMatch(/id="el-b"[^>]*data-start="5"/); + expect(result).toMatch(/id="el-b"[^>]*data-end="10"/); + expect(result).not.toMatch(/id="el-b"[^>]*data-duration/); + }); + + it("splits a data-duration element and removes stale data-end from both halves", () => { + const html = `
Text
`; + const { html: result, matched } = splitElementInHtml(html, { id: "el" }, 5, "el-b"); + + expect(matched).toBe(true); + // First half: data-duration="5", no data-end + expect(result).toMatch(/id="el"[^>]*data-duration="5"/); + expect(result).not.toMatch(/id="el"[^>]*data-end/); + // Second half: data-duration="5", no data-end + expect(result).toMatch(/id="el-b"[^>]*data-duration="5"/); + expect(result).not.toMatch(/id="el-b"[^>]*data-end/); + }); + + it("produces correct media trim offset when data-playback-rate is absent", () => { + const html = `
Text
`; + const { html: result, matched } = splitElementInHtml(html, { id: "el" }, 4, "el-b"); + + expect(matched).toBe(true); + // Split at t=4, firstDuration=4, rate defaults to 1 → new trim = 2 + 4*1 = 6 + expect(result).toContain('data-playback-start="6"'); + // Must not produce NaN + expect(result).not.toContain("NaN"); + }); + + it("returns matched:false when split time is outside element bounds", () => { + const html = `
Text
`; + + expect(splitElementInHtml(html, { id: "el" }, 0, "el-b").matched).toBe(false); + expect(splitElementInHtml(html, { id: "el" }, 10, "el-b").matched).toBe(false); + expect(splitElementInHtml(html, { id: "el" }, 11, "el-b").matched).toBe(false); + }); + + it("returns matched:false when target not found", () => { + const html = `
Text
`; + const { matched } = splitElementInHtml(html, { id: "nonexistent" }, 5, "el-b"); + expect(matched).toBe(false); + }); +}); + // T7 — data-hf-id targeting (spec for R1). // R1 adds `hfId?: string` to SourceMutationTarget and a `[data-hf-id="…"]` branch // in findTargetElement (sourceMutation.ts:34). Convert from it.todo in the R1 PR.