[lexical][lexical-html] Bug Fix: gate trailing <br> import drop on managed line breaks#8765
[lexical][lexical-html] Bug Fix: gate trailing <br> import drop on managed line breaks#8765potatowagon wants to merge 2 commits into
Conversation
…naged line breaks Follow-up to facebook#8750 implementing @etrepum's approach from facebook#7600. facebook#6395 dropped every trailing <br> in a block on import to match HTML's non-rendering behavior, but that loses authored trailing breaks on round-trip serialization. facebook#8750 narrowed the drop to Safari's Apple-interchange-newline clipboard artifact, but that gate doesn't fire for ordinary structural/authored trailing breaks (or non-Safari pastes), so the phantom-blank-line regression persists for those. Instead, drop a trailing <br> only when it is a *managed* line break: - data-lexical-managed-linebreak="true": the terminator Lexical's reconciler already injects via LexicalDOMSlot.setManagedLineBreak, so re-importing Lexical's own exported HTML stays idempotent. - class="Apple-interchange-newline": Safari's clipboard transport artifact. An ordinary authored trailing <br> carries neither marker and is now preserved, so round-trip serialization to HTML is lossless while rendering of external pasted HTML stays faithful. Browser-agnostic for the Lexical-managed case. Applies the same gate to both LineBreakNode.importDOM and the @lexical/html coreImportRules LineBreakRule. Adds test coverage for the preserved / managed / Apple-interchange cases.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| * An ordinary authored trailing `<br>` carries neither marker and is | ||
| * preserved so that round-trip serialization to HTML stays lossless. | ||
| */ | ||
| export function isManagedLineBreak(node: Node): boolean { |
There was a problem hiding this comment.
A different name for this probably makes sense, or we should rename the one in LexicalMutations (maybe isEditorManagedLineBreak). They don't work the same way, so it may cause confusion.
There was a problem hiding this comment.
Good call — renamed the existing LexicalMutations helper to isEditorManagedLineBreak (68c830d) to avoid the collision. Added cross-referencing comments on both so the distinction is clear: the mutations one identifies a break the editor is actively managing in the live DOM (via __lexicalLineBreak / node key), while the new one inspects serialized markup (data-lexical-managed-linebreak / Apple-interchange class) to decide whether a parsed <br> should be dropped on import.
Addresses review feedback: the new isManagedLineBreak in LexicalLineBreakNode (inspects serialized markup — the data-lexical-managed-linebreak attribute / Apple-interchange class to decide whether to drop a parsed <br> on import) collided by name with the pre-existing isManagedLineBreak in LexicalMutations (identifies a line break the editor is actively managing in the live DOM via the __lexicalLineBreak slot reference / node key). They work differently, so the mutations one is renamed to isEditorManagedLineBreak and both now carry cross-referencing comments.
Description
Follow-up to #8750, implementing @etrepum's marker approach from #7600 (review 2893090447): mark Lexical's reconciler-injected terminating
<br>(data-lexical-managed-linebreak="true", already set inLexicalDOMSlot.setManagedLineBreak) soimportDOMcan drop that artifact while preserving a real, unmarked trailing break.Decision: lossless round-trip is the priority for an unmarked trailing
<br>, over matching the browser's non-rendering of a trailing block<br>. This PR therefore preserves unmarked trailing breaks and drops only the marked artifacts. The known cost is documented below.The core conflict
A trailing
<br>as the last child of a block is non-rendering in HTML (browsers show nothing for it). So the same DOM shape<p>2<br></p>means two different things depending on origin:An ordinary authored
<br>carries no marker to distinguish these, so an import rule must pick one policy for unmarked trailing breaks — you cannot have both for the same input. We choose preserve (round-trip fidelity).Behavior of each approach on a trailing block
<br>Verified empirically (drop =
<br>removed on import):<br>of a block)1<p>2<br /></p>31/2/3<br data-lexical-managed-linebreak><br class="Apple-interchange-newline">Note: #8765 is strictly stronger than #8750 for round-trip — #8750 keys only on the Apple class, so it does not drop Lexical's own
data-lexical-managed-linebreakartifact and thus fails to fix the round-trip it targeted. #8765 drops that artifact correctly.Why this is complete for round-trip (no
exportDOMchange needed)LineBreakNodehas noexportDOMoverride; the default emits a single unmarked<br>. With the "preserve unmarked" policy, both export paths round-trip losslessly:exportDOM/$generateHtmlFromNodesemits<p>2<br></p>(single, unmarked) → import preserves → 1LineBreakNode. ✅ Lossless because unmarked breaks are preserved. (Under [lexical] Bug Fix: more accurate line break pasting #6395's drop-unmarked policy this break would be lost — the bug [lexical] Bug Fix: synced the export html output with import DOM expectations and browser DOM #7600 tried to fix by doubling the exported<br>. The preserve policy sidesteps that; no export-side change required.)<p>2<br><br data-lexical-managed-linebreak></p>→ import keeps the real<br>, drops the marked artifact → 1LineBreakNode. ✅ Lossless.Export: no change required
exportDOMis intentionally left untouched. The "preserve unmarked" import policy makes the export-side doubling that #7600 introduced (emitting N+1<br>inexportDOM) unnecessary — a single unmarked<br>round-trips as-is:LineBreakNodesexportDOMHTML<p>2<br></p><br>unmarked → preserved)<p>2<br><br></p><p>2<br><br><br></p><p><br></p>Verified against the
$generateHtmlFromNodes→$generateNodesFromDOMpath and the existing block-heavy snapshots (135lexical-list+lexical-historytests pass, incl. list items / checkboxes / nested blocks). We deliberately do not stampdata-lexical-managed-linebreakonexportDOMoutput — that would be the #7600 export direction @etrepum preferred to avoid; round-trip works purely by preservation instead.Known cost
External HTML with an unmarked trailing
<br>(e.g.1<p>2<br /></p>3) is now preserved and renders a blank line, where a browser would render nothing. This is the accepted trade for lossless round-trip. Matching browser rendering here would require reverting to #6395's drop-on-import and moving the round-trip fix toexportDOM(emit N+1<br>, #7600-style) — the direction @etrepum preferred not to take.What this PR does
Drop a trailing
<br>on import only when it is a managed line break:data-lexical-managed-linebreak="true"(Lexical's reconciler artifact), orclass="Apple-interchange-newline"(Safari clipboard artifact).Applied to both
LineBreakNode.importDOMand the@lexical/htmlcoreImportRulesLineBreakRule;isManagedLineBreakis exported fromlexicalso both share it. The pre-existingisManagedLineBreakinLexicalMutations(which detects an editor-managed live-DOM break) is renamed toisEditorManagedLineBreakto avoid the name collision.cc @etrepum @levensta
Test plan
vitest --project unitpasses for the affected files. Coverage inLexicalEventHelpers.test.tsx:<br>is preserved (1<p>2<br /></p>3) — documents the accepted cost<br>(data-lexical-managed-linebreak) is ignored<br>is ignored