Skip to content

fix(core): splitElementInHtml data-end + text-content inner wrapper #1273

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
vanceingalls wants to merge 3 commits into from
+121 −6
Closed

fix(core): splitElementInHtml data-end + text-content inner wrapper #1273

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
19fbaeb
fix(core): splitElementInHtml data-end + text-content inner wrapper
vanceingalls Jun 8, 2026
370a719
fix(core): review fixes for setElementDuration + text-content + playb…
vanceingalls Jun 8, 2026
b72dc5a
test(core): add splitElementInHtml + text-content regression tests (#…
vanceingalls Jun 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 79 additions & 0 deletions packages/core/src/studio-api/helpers/sourceMutation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
removeElementFromHtml,
patchElementInHtml,
probeElementInSource,
splitElementInHtml,
} from "./sourceMutation.js";

describe("removeElementFromHtml", () => {
Expand Down Expand Up @@ -126,6 +127,29 @@ describe("patchElementInHtml", () => {
expect(result).not.toContain("Hello World");
});

it("patches text content when inner wrapper is <p> not <div>", () => {
const html = `<div id="el" data-start="0" data-end="5"><p>Original</p></div>`;
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(/<p[^>]*>Replaced<\/p>/);
});

it("patches text content when inner wrapper is <span>", () => {
const html = `<div id="el" data-start="0" data-end="5"><span>Original</span></div>`;
const { html: result } = patchElementInHtml(html, { id: "el" }, [
{ type: "text-content", property: "", value: "Replaced" },
]);

expect(result).toContain("Replaced");
expect(result).toMatch(/<span[^>]*>Replaced<\/span>/);
});

it("applies multiple operations in one call", () => {
const { html: result } = patchElementInHtml(FIXTURE, { id: "hero" }, [
{ type: "inline-style", property: "color", value: "blue" },
Expand Down Expand Up @@ -362,6 +386,61 @@ describe("probeElementInSource", () => {
});
});

describe("splitElementInHtml", () => {
it("splits a data-end element and removes stale data-duration from both halves", () => {
const html = `<!doctype html><html><body><div id="el" data-start="0" data-end="10"><div>Text</div></div></body></html>`;
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 = `<!doctype html><html><body><div id="el" data-start="0" data-duration="10"><div>Text</div></div></body></html>`;
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 = `<!doctype html><html><body><div id="el" data-start="0" data-end="10" data-playback-start="2"><div>Text</div></div></body></html>`;
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 = `<!doctype html><html><body><div id="el" data-start="0" data-end="10"><div>Text</div></div></body></html>`;

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 = `<!doctype html><html><body><div id="el" data-start="0" data-end="10"><div>Text</div></div></body></html>`;
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.
Expand Down
48 changes: 42 additions & 6 deletions packages/core/src/studio-api/helpers/sourceMutation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,13 @@ export function patchElementInHtml(
}
break;
case "text-content":
if (op.value != null) htmlEl.textContent = op.value;
if (op.value != null) {
// 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;
}
}
Expand All @@ -219,6 +225,36 @@ 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 {
if (usesDataEnd) {
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
}
}

export function splitElementInHtml(
source: string,
target: SourceMutationTarget,
Expand All @@ -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 };
}
Expand All @@ -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")
Expand All @@ -251,15 +286,16 @@ 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),
);
}

// 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) {
Expand Down
Loading