fix(sdk): keep <br> line breaks editable and uncorrupted in setText (#2742)

A `<br>` is a void element but `resolveSingleChildTextTarget` treated a lone
`<br>` child as the element's text target. So `getOwnText(<h1>A<br>B</h1>)`
read the `<br>`'s (empty) textContent → `text: null`, which consumers surface
as "not editable", and `setOwnText` wrote into the `<br>`, corrupting it
(serialized as invalid `</br>`).

Exclude void elements from the single-child text target; read `<br>` as "\n";
and rebuild the text/`<br>` run from the newline-separated value on write,
reusing existing `<br>` nodes so their identity (data-hf-id) survives an
in-place edit.
This commit is contained in:
Hblee
2026-07-23 08:34:30 -07:00
committed by GitHub
parent 222aec45ab
commit dd7378bbd9
2 changed files with 114 additions and 4 deletions
+58 -4
View File
@@ -302,18 +302,55 @@ function isHTMLElementTarget(el: Element): boolean {
return "style" in el;
}
function resolveSingleChildTextTarget(el: Element): Element | null {
const inner = el.children.length === 1 ? el.firstElementChild : null;
return inner && isHTMLElementTarget(inner) ? inner : null;
// Void elements can't hold text, so a lone `<br>`/`<img>`/`<hr>`/… child is
// never the element's text target. Treating one as such made getOwnText read
// "" (→ `text: null`, surfaced as "not editable") and setOwnText write into the
// void element, corrupting it — e.g. a `<br>` gained a text child and
// serialized as invalid `</br>`.
const VOID_TEXT_TARGET_TAGS = new Set([
"AREA",
"BASE",
"BR",
"COL",
"EMBED",
"HR",
"IMG",
"INPUT",
"LINK",
"META",
"PARAM",
"SOURCE",
"TRACK",
"WBR",
]);
function isBrElement(node: Node): boolean {
return node.nodeType === 1 && (node as Element).tagName === "BR";
}
/** Read the text target used by SDK setText. */
/** A flat text leaf: no element children, or only `<br>` line breaks
* (e.g. `<h1>First<br>Second</h1>`). Such an element's entire content is its
* own text, so setText owns it end-to-end. Vacuously true for zero children. */
function isTextOrBrLeaf(el: Element): boolean {
return Array.from(el.children).every((child) => child.tagName === "BR");
}
function resolveSingleChildTextTarget(el: Element): Element | null {
const inner = el.children.length === 1 ? el.firstElementChild : null;
if (!inner || !isHTMLElementTarget(inner)) return null;
if (VOID_TEXT_TARGET_TAGS.has(inner.tagName)) return null;
return inner;
}
/** Read the text target used by SDK setText. `<br>` line breaks are surfaced as
* "\n" so a multi-line leaf round-trips through setOwnText. */
export function getOwnText(el: Element): string {
const singleChild = resolveSingleChildTextTarget(el);
if (singleChild) return singleChild.textContent ?? "";
let text = "";
el.childNodes.forEach((n) => {
if (n.nodeType === 3) text += (n as Text).nodeValue ?? "";
else if (isBrElement(n)) text += "\n";
});
return text;
}
@@ -327,6 +364,23 @@ export function setOwnText(el: Element, text: string): void {
}
const doc = el.ownerDocument;
// Flat text leaf (text and/or `<br>` only): rebuild the text/`<br>` run from
// the "\n"-separated value — the inverse of getOwnText. Existing `<br>` nodes
// are reused in order so their identity (data-hf-id, etc.) survives an edit
// that keeps the line, and a new one is only minted when a line is added.
// Never writes text into a `<br>`, so no `</br>` corruption.
if (isTextOrBrLeaf(el)) {
const spareBrs = Array.from(el.children).filter((child) => child.tagName === "BR");
while (el.firstChild) el.removeChild(el.firstChild);
const lines = text.split("\n");
lines.forEach((line, index) => {
if (index > 0) el.appendChild(spareBrs.shift() ?? doc.createElement("br"));
if (line) el.appendChild(doc.createTextNode(line));
});
return;
}
const children = Array.from(el.childNodes);
// Track original position of the first text node so we restore there, not at firstChild.
let firstTextIdx = -1;
+56
View File
@@ -262,6 +262,62 @@ describe("setText", () => {
it("override-set key maps correctly", () => {
expect(pathToKey("/elements/hf-title/text")).toBe("hf-title.text");
});
// A `<br>` line break is a void element, not the element's text target. It
// must not make the heading read as empty (→ non-editable) nor be corrupted
// by setText. See getOwnText / setOwnText in model.ts.
describe("<br> line-break leaves", () => {
const brDoc = () =>
parseMutable(
'<div data-hf-id="hf-s" data-hf-root><h1 data-hf-id="hf-br">Centrifugal<br>Force</h1></div>',
);
it("reads a <br>-split heading as editable text (newline-joined), not empty", () => {
const parsed = brDoc();
// The inverse patch value is the pre-edit text — proves getOwnText read
// the heading as "Centrifugal\nForce" rather than "" (which surfaced as
// `text: null` → "not editable" in the studio panel).
const { inverse } = applyOp(parsed, { type: "setText", target: "hf-br", value: "x" });
expect(inverse[0]).toMatchObject({ value: "Centrifugal\nForce" });
});
it("preserves the <br> when setting text (no </br> corruption)", () => {
const parsed = brDoc();
applyOp(parsed, { type: "setText", target: "hf-br", value: "Centripetal\nForce" });
const html = serializeDocument(parsed);
expect(html).toContain("Centripetal");
expect(html).toContain("Force");
expect(html).toContain("<br");
// The <br> must stay empty — never gains a text child.
const br = parsed.document.querySelector('[data-hf-id="hf-br"] br');
expect(br?.textContent).toBe("");
});
it("drops the line break when the new text has no newline", () => {
const parsed = brDoc();
applyOp(parsed, { type: "setText", target: "hf-br", value: "Centrifugal Force" });
const h1 = parsed.document.querySelector('[data-hf-id="hf-br"]');
expect(h1?.querySelector("br")).toBeNull();
expect(h1?.textContent).toBe("Centrifugal Force");
});
it("reuses the existing <br> node so an in-place edit round-trips exactly", () => {
const parsed = parseMutable(
'<div data-hf-id="hf-s" data-hf-root><h1 data-hf-id="hf-br">Centrifugal<br data-hf-id="hf-x5px">Force</h1></div>',
);
const before = serializeDocument(parsed);
// Same line count → the <br> (and its data-hf-id) is preserved, and undo
// is byte-exact.
const { inverse } = applyOp(parsed, {
type: "setText",
target: "hf-br",
value: "Angular\nMomentum",
});
expect(serializeDocument(parsed)).toContain('<br data-hf-id="hf-x5px">');
applyPatchesToDocument(parsed, inverse);
expect(serializeDocument(parsed)).toBe(before);
});
});
});
// ─── setAttribute ─────────────────────────────────────────────────────────────