PT-4201: Standard-view copy/paste USFM fidelity - #13
tjcouch-sil wants to merge 31 commits into
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A copy that does not come from a real browser clipboard event — Ctrl+C/Ctrl+X,
the context menu's Cut/Copy, EditorRef.copy()/.cut() — dispatches the command
with a null payload, and @lexical/clipboard has to synthesize an event before
anything can be written: it appends a hidden placeholder element to the editor,
points the DOM selection at it, and runs document.execCommand("copy"). Filling
that synthesized event declines an empty selection BEFORE suppressing the
browser's own copy, so the browser copies the placeholder and the user's real
clipboard contents are replaced by a character that was never in the document.
A read-only construct makes it likelier, not special: an UnknownNode renders
contentEditable=false and its marker glyphs are decorators that take no keyboard
selection, so trying to select a figure's marker text leaves the caret in the
prose beside it. A plain collapsed caret does the same, in every view, and cut
takes the identical path.
Two layers. copySelection/cutSelection (shared-react) do not synthesize a copy
for a selection that cannot produce content, and leave the key unclaimed so the
browser's own no-op copy runs — that is what "untouched" means. And the
Standard-view handler claims a null-payload dispatch it cannot build data for
rather than declining it into @lexical/rich-text's fallback. A real clipboard
event at a collapsed caret is still declined, and a node selection still goes to
Lexical, which copies it correctly.
Copying a read-only construct works through a selection that contains it — the
shape a browser resolves a drag over one into — which reaches its display glyphs
through the same walker as prose and yields the construct's real USFM bytes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…arker kinds The paste carrier choice now states its trade-off where it is made. Choosing by which carrier an NBSP survived in is the better rule for a foreign clipboard, and unusable here: Standard view's own text/plain deliberately carries no NBSP at all, so an NBSP-presence test is true of every P10 copy and would route the editor's own round trip through html, whose decoded text drops a collapsed note's caller. Recorded in the semantics doc's deferred list too. PastePayload keeps only what a claim may act on. The unresolved carriers were exposed and unread; every claim must replay the same bytes, and a second carrier within reach invites one of them to pick differently. Doc corrections: getPastePayload named three PASTE_COMMAND handlers and described the Standard-view claim as NBSP-gated, both false since the merge; isInternal claimed that claim declines internal payloads outright, false since the attribute path claims them; $handlePasteForStandardView lost its "call inside editor.update()" contract note while gaining a $getEditor() call and a command dispatch that make it matter more. The marker-kind classifiers move to a leaf module. Both tiers ask the question and must answer it identically, and Tier 2 reaching into Tier 1 for it made the two import each other. Their only inputs are a string and a stylesheet lookup, so neither tier is needed to answer it. Also restores the multi-paragraph-html paste test's return-contract assertion, lost when it switched to a command dispatch — the content assertion alone cannot tell this handler's replay from Lexical's own html import. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rsion The copy flavors no longer split into "text/plain inverts, everything else ships the display form". text/html inverts too, collapse-awarely: an interior single display-NBSP becomes the plain space it stands for, while a run of 2+ and a fragment edge keep NBSP because a rich-text consumer collapses runs and drops edge whitespace. Only the internal Lexical flavor is still untouched, so an internal paste round-trips the display form exactly. That also sharpens why the paste carrier is chosen by presence rather than by where an NBSP survived: Standard view's own text/plain still carries no NBSP at all, while its text/html now carries them precisely where a plain space would be destroyed — so an NBSP-presence test would route every P10 round trip through html and lose the note caller. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Checking the selection before dispatching read the COMMITTED editor state, which Lexical is a microtask behind — so a selection made earlier in the same tick was invisible and the copy was silently withheld. That is the ordinary shape of the public EditorRef.copy() path: select something programmatically, then copy it. Reading the live state was not a drop-in fix either, since the context menu invokes its options from inside editor.update(). The guard is now a COPY_COMMAND/CUT_COMMAND listener registered above @lexical/rich-text's fallback and below every feature handler. A command listener runs inside the update, where the selection is authoritative for pending and committed state alike, so the staleness class is gone rather than narrowed — and one registration covers the shortcuts, the context menu and the editor ref, since all of them dispatch the same command. The three Standard-view pins asserted nothing that could fail. The dispatch's return value is true either way (rich-text always claims), the clipboard-module spy cannot see a call made through the externalized rich-text import, and the execCommand assertion was masked by @lexical/clipboard's module-level latency timer left set by the preceding test. They now watch for the dispatch reaching past the claim — immune to module state and test order — and drain that timer, and all three fail in a whole-file run when the claim is reverted. Also pins the node-selection carve-out (a copy that has content must still happen), the context-menu and editor-ref legs, records the empty-selection no-op on the public API's TSDoc, and marks the browser's drag-over-a-read-only- block selection shape as the assumption jsdom cannot verify. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…idened MarkerEditContext Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QCjocx9uZcPqB2esPh9r6f
Pasting `\fig At once they left their nets.|src="avnt016.jpg" size="span" ref="1.18"\fig*` into Standard view left a figure box holding only `\fig |src="…"\fig*`, the caption stranded after the closing glyph as paragraph prose, and an exported USJ whose `figure` object had no content at all — the caption merged into the neighbouring text run. `$textNodeInUnknownTransform` (TextSpacingPlugin) ejects any TextNode that is new in the current update out of an UnknownNode parent, so a user typing into a read-only opaque block never plants prose inside it. A Tier-2 rebuild materializes the whole figure — wrapper and caption together — in one update, so the caption looked exactly like typed text to that rule. Eject only when the wrapper predates the update, which is the shape a typed intrusion actually has. The class is invisible to a MarkerEditPlugin-only harness (the ejection lives in a sibling plugin), so the new pins mount the full Editor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QCjocx9uZcPqB2esPh9r6f
…copy An internal (`application/x-lexical-editor`) copy→paste of a figure, sidebar, periph or ref pasted the construct's full literal USFM bytes onto the screen while the exported USJ silently dropped the node and every attribute on it — a convincing display over lost data that a save then persisted with no error. `UnknownNode.excludeFromCopy` kept only a child-bearing optbreak out of the exclusion; every other kind stayed excluded, and `$appendNodesToJSON` does not drop an excluded node — it hoists its children into the parent, stranding the content-free display decorators that render each kind's marker and attribute bytes. Key both `excludeFromCopy` and the `isSelected` boundary override on the node's own child count rather than on its kind: the kinds share one display-decorator shape, so they shared one failure. Verified per kind across all three real clipboard payload shapes (plain-only, plain+html, and the full payload carrying the lexical flavor). Tables are not part of this — `table`/`table:row`/`table:cell` are their own `ImmutableTable*` nodes now and never took the exclusion — and are pinned in the same sweep to say so. The plain-carrier losses each kind genuinely has (a block construct split across lines; a `ref` wrapper USFM has no bytes for) are asserted as losses rather than left unpinned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QCjocx9uZcPqB2esPh9r6f
A sidebar's nested `\p` and a table's rows and cells are real block-level nodes, so the copy walker put a `\n` at each of them and spread ONE construct over several lines. Paste replays every `\n` as a paragraph split and Tier 2 re-tokenizes strictly one paragraph at a time, so the `\esb`/`\esbe` and `\tr`/`\th`/`\tc` assembly the fragment tokenizer already implements never saw the whole construct: a pasted sidebar came back unclosed with its paragraph hoisted out, a pasted table as empty row wrappers with the cells stranded as sibling paragraphs. Suppress the line break for a block boundary whose opaque ancestor is above it, so the construct stays byte-contiguous and a single-paragraph rebuild can reassemble it. The construct's own outermost node still starts its own line, so a table stays separated from the prose around it. USFM markers self-delimit, so the one-line form is valid USFM of the same document — just not the line-per-marker layout a writer emits. Tables needed one more thing on the paste side: usfm.sty classifies `\th1`/`\tc1`/… as CHARACTER styles, so with a real project stylesheet a cell marker reached the tokenizer as a char token and the table-cell assembly, which only ran for paragraph tokens, never saw it. ParatextData derives a cell from the marker NAME, so recognize one on either token kind, guarded on an open row. The corpus sweep's sidebar and table fixtures are un-skipped and now sweep clean. `\periph` is not fixed: the tokenizer has no periph assembly and periph renders attributes as a pipe run on a marker with no closing bytes. That is recorded and pinned as a loss rather than left to look fixed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QCjocx9uZcPqB2esPh9r6f
A paste's own update does not always get to perform the rebuild its bytes ask for. A paragraph whose re-tokenization would EJECT content out of a milestone deliberately waits for caret departure, and by then `pasteRebuildArmed` — a per-commit flag — has reset, so the departure settle rebuilt the pasted line as though the user had TYPED it: the host's `\p ` glyph and the pasted line's own `\p ` literal both survived, leaving a stray empty paragraph ahead of the correct one. `Tier2Context.pastePendedKeys` gives the arm a lifetime tied to the pended work instead of to the commit clock. A pend recorded while the paste's own update is armed records its key; the settle that routes that key to a rebuild consumes the provenance and passes it down. Keying it that way is what keeps it out of typed input: keys are only added inside a paste's own update, consumed by the first rebuild that reaches them, and pruned by the per-commit update listener as soon as they stop pending — and text typed beside the pasted bytes pends its own key, which carries none. Closes the last "Known Lossy" clipboard entry. The corpus `milestones (ts)` fixture it was filed against does not in fact reach the deferred path (its milestone ejects nothing), so it is un-skipped and swept clean; the shape that does reach it is pinned directly, together with its counter-pin that the same bytes with no paste behind them still split with an empty predecessor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QCjocx9uZcPqB2esPh9r6f
Four review follow-ups on the opaque-construct clipboard cluster. `UnknownNode.excludeFromCopy`'s rationale still read as an optbreak husk rule after the guard was generalized to child count. It now names what the rule actually excludes, including the case it was under-describing: a hidden-marker view builds no display children at all, so ANY content-less construct there — a caption-less figure, an empty ref — is childless and dropped. Recorded as a deferred item rather than changed, since a marker-free view copying marker-only constructs is a view-semantics decision. Pinning the `isSelected` boundary on a second kind turned up a real disagreement rather than confirming the shared rule. A construct whose display bytes lead starts with a decorator, so an element-type boundary point on the wrapper stays one and reaches no child; a construct with no display bytes (a `ref`) starts with a real TextNode, and Lexical normalizes that same point into a text point at the child's offset 0 — the child is then in `getNodes()` while contributing zero characters, so the wrapper and its whole content went into the lexical flavor while `text/plain` had none of it. `isSelected` now asks whether the selection covers a child's CONTENT, which is what the copy walker computes. Pinned for figure and ref in both directions. The tokenizer's character-token cell arm gains sheet-backed unit coverage for the align-infix names and both of its guards; a ranged cell is not a sheet entry at all and still arrives as a paragraph token, so that is pinned too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QCjocx9uZcPqB2esPh9r6f
…ts text The content-coverage `isSelected` predicate closed the `ref` boundary's carrier disagreement by trading it for something worse. Excluding the wrapper does not drop it quietly: `$appendNodesToJSON` hoists its children in its place, and `createUnknown` stamps `mode:"token"` on every text child, which `$sliceSelectedTextNodeContent` refuses to slice — so the zero-width child kept its full text and the copy carried `Genesis 1:1` with the `ref` node and its `loc` attribute silently gone. That is the convincing-lie shape the suite exists to prevent, and the tag-only assertion could not see it. Revert to child membership, which keeps the construct whole and makes the lexical flavor a superset of `text/plain` at that one boundary. Making it appear in neither carrier is not reachable from the node API: the children are plain TextNodes, and `exportNodeToJSON` requires every ElementNode's `exportJSON()` to return a `children` array, so a node cannot exclude itself AND its children. The residual is recorded in the deferred list with both measured payloads. Every boundary assertion now reads the construct's own content bytes, and the pins state the behavior per kind — a decorator-led construct is excluded from both carriers, a text-led one is carried whole — so the lossy variant turns them red instead of passing. Also scoped the provenance doc to what was measured: typing into a DIFFERENT node pends its own key and takes the typed path, but typing into the still- pending node itself adds no key, so the paste path runs and the dedup applies to the line those bytes are now part of. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QCjocx9uZcPqB2esPh9r6f
A copied `\periph` came back as an ordinary paragraph carrying its own attribute bytes as literal content: `usfmFragmentToUsj.ts` had assemblies for `\esb`/`\esbe` and `\tr`/`\th`/`\tc` but none for `\periph`. The suspected second cause — that the one-line opaque-construct copy removes the attribute run's only delimiter — does not hold. Measured: the one-line and line-per-marker spellings tokenize identically, because a line break is ordinary whitespace and the next `\marker` is what ends a division's marker line either way. The one-lining is in fact required: a paste replays every line break as a paragraph split and Tier 2 re-tokenizes one paragraph at a time, so only the one-line form lets a single pass see the division together with the blocks inside it. Pasting the line-per-marker spelling was measured to come back as an empty division with its paragraph beside it, and that measurement is now a pin. `\periph` therefore assembles like a sidebar: the marker line splits at its first `|` into the division title (`alt`) and a named-attribute list, and the division takes every following block until the next `\periph`, a chapter, or the fragment end. A division has no closing bytes, so there is no unterminated state to record. An attribute list that does not parse degrades the whole division to a literal paragraph, as an unfoldable figure span does. A sidebar now lands in an open division instead of beside it, which required pushing it before assigning `sidebar` — `blockTarget()` answers "the open sidebar" whenever one is open, so assigning first nested it inside itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QCjocx9uZcPqB2esPh9r6f
Deferred item 9's two attribute-context paste residuals, characterized and closed. (a) does not reproduce and is not paste-specific. A mixed selection reaching past a char span's closing glyph into following sibling text settles to byte-identical USJ whether the replacement is typed or pasted, across four selection shapes (forward and backward, run-start-to-trailing-end and run-end-to-trailing-start included). The span coming back `closed="false"` with its attribute run collapsed into content is the arithmetic of a range that covered the closer, not a defect: deleting a range containing the closing glyph deletes the closing glyph. (b) reproduces, from BOTH directions, and is a data-fidelity hole. The value-byte carve-outs — no `\c`/`\id` strip, no positional NBSP mapping — were applied to any selection that merely TOUCHED an attribute run. Those carve-outs are earned by one fact only: the bytes land in a node that stays tagged "attribute", which `$textNodeTier2Transform` skips. A touching selection loses that the moment its removal takes the range out of the run, so pasting `\c 5` over one produced a real chapter node mid-paragraph with the rest of the paragraph stranded outside it — the save-loop poisoning the strip exists to prevent — and a pasted interior NBSP was read as display whitespace and saved as a plain space instead of surviving as data. Measured with the range starting in body text and reaching into the run, and with it starting inside the run and reaching past the closer. The carve-outs now apply only to a selection wholly inside one attribute node. The insertion MECHANISM is unchanged for every touching selection — one `insertText`, never the paragraph-splitting line replay and never Lexical's rich-paste node insertion — so a multi-line payload landing in body content this way still collapses per newline rather than splitting a paragraph the removal has cut a char span in half in. The single-paragraph fixture the new pins use is what made this measurable at all: the earlier probing needed a cross-paragraph range, which does not hold in jsdom. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QCjocx9uZcPqB2esPh9r6f
…e losses Deferred item 7 excluded `2sa.usj.ts` from the copy→paste corpus sweep because it did not round-trip clean when last measured. Re-measured against the same harness: three classes of divergence remained, two of them clipboard bugs of the family this round has been closing. A collapsed note's `category` was dropped. `createNote` builds the `\cat` display run only for the editable EXPANDED layout — a deliberate view decision — so a collapsed note has no category bytes on screen for the copy walker to pick up. Same shape as the note caller, which renders as an empty glyph and is recovered by the walker for exactly this reason, so the category is recovered the same way, placed where the file has it (`\f + \cat People\cat*\fr …`, no trailing space: a space there re-tokenizes into a stray text child inside the note). A spanning table cell's `colspan` was dropped. USFM tables have no pipe attributes at all — a cell's width lives in its marker NAME (`\thc3-4`), which the tokenizer trims off into `colspan` on the way in — and the display rendered the bare marker, so the editor showed, copied, and re-tokenized a one-column cell. `tableCellMarkerWithSpan` already existed for the `UnknownNode` byte builder, whose table branch went dead when cells became `ImmutableTable*` nodes; it is now exported and shared by both. The committed 2SA Lexical fixtures are regenerated: six lines, the three spanning cells in editable and visible modes. The third class is inherent and stays: a verse's `sid` is derived from book + chapter + verse when ParatextData produces USX, and USFM has no bytes for it anywhere. Measured rather than assumed — a plain load→save of this fixture keeps all 26 of them, so the loss belongs to the carrier, not the adaptors. With those two fixed, 2SA round-trips to full USJ equality modulo the 26 verse sids, so it joins the sweep as a running case rather than a skip: skipping would trade 143 items of real-world coverage for one attribute. The carve-out is asserted on both sides (the source really carries them, the paste carries none) so it can neither be vacuous nor outlive the gap, and the re-stamped document USJ version is asserted rather than waived. The whole sweep file, 2SA included, runs in about a second. Two gaps the old exclusion reason listed are already closed and were confirmed so: the empty `\b` blank-line paragraph and the `\ref` target both round-trip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QCjocx9uZcPqB2esPh9r6f
…periph title loss The carve-out is earned by paste-equals-typing, not by the bytes never re-tokenizing: the "attribute" tag survives insertion, but the caret-departure settle re-tokenizes the paragraph, so a pasted \c in a value does become a chapter marker — identically to typing it there. Both halves are now pinned settled rather than read before the settle, so the residual is visible instead of looking safe. Periph's parsed attributes were spread after its title, and the title attribute is not reserved, so a periph line carrying that attribute silently lost its title. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QCjocx9uZcPqB2esPh9r6f
Comment-hygiene sweep of the clipboard/tokenizer work: rewrite backward-facing comments (dated live-repro narration, "used to"/"previously" change narration, personal attribution, "this branch" framing) added by this branch so they read as durable, forward-facing documentation instead of PR provenance. No logic, assertions, or names changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QCjocx9uZcPqB2esPh9r6f
`UnknownNode.isSelected` answered purely on child membership, which a `NodeSelection` never satisfies — it marks a node by its own key. A construct selected outright therefore reported unselected, and `$appendNodesToJSON` hoisted its (also unselected) children, copying nothing for it. Fall back to the base key-membership answer for a node selection while keeping the child-membership rule that makes the two clipboard carriers agree at a range boundary. `ContextMenuPlugin` is exported on its own but relied on `ClipboardPlugin` to register the empty-copy guard, so a host mounting it alone got the placeholder-overwrites-clipboard bug the guard exists to prevent. Register the guard from both plugins; a duplicate registration at the same priority is inert. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ention gaps The `\c`/`\id` strip ran to the next backslash or newline, so a paste of `x \c 5 y` landed as `x ` — content past the chapter number was destroyed with no feedback. The tokenizer reads only the next whitespace-delimited word as the number (getNextWord, usfmFragmentToUsj.ts), so removing more than that was never required to prevent the second-ChapterNode poisoning. Split into CHAPTER_TOKEN (marker + separator + one word, stopping at a backslash) and BOOK_ID_TOKEN, which keeps the wider rule because an `\id` line's whole remainder genuinely is payload. The pin that had encoded the loss now asserts the content survives. `Editor.tsx`'s structureProtectionMode pass-through had no coverage: Editor and MarkerEditPlugin both default the prop to "off", so deleting the line left all 4,234 tests green while silently starving StructureKeyboardPlugin's sanitizer on a protected document's paste. Threaded the option through mountStandardViewEditor and pinned it against a real Editor mount. Also from the review: - Reuse shared-react's $hasCopyableSelection in $handleCopyForStandardView instead of respelling its negation. The two layers must agree on what "nothing to copy" means, including the node-selection carve-out, and one shared predicate is what keeps them agreeing. - Derive the four new marker regexes from ENGINE_MARKER_NAME_BYTES. Two spellings had already diverged — `[a-z0-9-]i` excludes `_`, so `\my_marker` was a marker to the strip and not to the NBSP rules, and its separator NBSP survived into content as a literal `~`. - Collapse isParaKindMarker/isCharKindMarker onto one body. The module exists so the two tiers cannot disagree; two copies of the same five lines is how they would come to. - Drop the `$` sigil from normalizePastedNbsp and stripPastedChapterAndBookId. Both are pure string functions with no editor-context requirement to document, which CLAUDE.md requires of every exported `$`-prefixed one. - Strip spec-section IDs and provenance narration from comments, per CLAUDE.md's rule that code must stand on its own. - Move the clipboard semantics doc out of docs/superpowers, which .gitignore:67 ignores — it was tracked only by a force-add, so edits to the durable contract would never have shown in git status. Fix its dead cross-reference and the stale "not yet run" row for the e2e spec. - Pin the Lexical version $selectionToUsfmText mirrors its walk from. New pins were each verified by reverting the fix and confirming they go red; the suite is green at shared 697, shared-react 1674, platform-editor 1864, with typecheck, lint and format clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This repository commits each published package's rolled-up build output, which the eten-tech-foundation repository does not, so the dist inherited by rebasing onto this main predates every commit on this branch. Rebuilt with `npm run rebuild-committed-output`. Also record the `EditorRef.copy()`/`cut()` behavior change in the changelog. The signatures are unchanged, so a consumer upgrading gets no compile-time signal that an empty-selection copy no longer writes to the clipboard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…undaries In Standard view a read-only construct's display bytes — a figure's `\fig ` opener, its `|src="…"` attribute run, its `\fig*` closer — are ImmutableTypedTextNode decorators whose glyph text is an ordinary DOM text child of the decorator's own element. Lexical 0.43's $internalResolveSelectionPoint resolves a DOM point inside a decorator's element to null (the walk up from the text reaches the decorator, which is not a TextNode), $internalResolveSelectionPoints then nulls the WHOLE selection, and on commit updateDOMSelection calls removeAllRanges() because the previous selection was inside the editor. Measured effects: a mouse drag begun on the `\fig ` glyph left the editor with no selection at all, so Ctrl+C reached only the empty-copy guard and the clipboard was never written; a drag from the caption into the attribute run stopped dead at the caption's end. DecoratorBoundarySelectionPlugin (shared-react, mounted unconditionally) handles SELECTION_CHANGE_COMMAND at CRITICAL — Lexical dispatches it even for a selection it resolved to null, while the DOM selection is still intact — and moves each offending end to the decorator's boundary in its parent, the one form Lexical does resolve. A range grows outward so the glyph is wholly in or wholly out; a caret takes the nearer side. The snapped DOM points are turned into a Lexical selection in the same update via $createRangeSelectionFromDom, because moving the DOM selection alone would still leave a null pending selection for the reconciler to clear. It only acts when $getSelection() is null (a resolved selection cannot have an end inside a decorator, and reading the DOM over one would clobber a programmatically placed caret), and it steps around the note caller: TrailingNoteCaretGuardPlugin recognizes a landing on a collapsed note's caller precisely by the selection being unresolved, and supplying one there disabled that repair and left the caret in the note's hidden body (measured, pinned). Pins: six platform tests against a real `\fig` document (five were red before the plugin), two shared-react guard-rail tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ments, keep paste rules under protection
Three hand-QA findings against Standard-view copy/paste, all rooted in
which clipboard carrier a consumer reads.
Paratext 9 paste is HTML-FIRST (IHtmlEditorCopyPaste.cs: the CF_HTML
fragment is pasteHTML'd whenever present, and Standard-Reverse.xslt reads
the fragment's text() as USFM). Standard view's text/html was Lexical's
DOM export, which is lossy in two independent ways:
ImmutableNoteCallerNode.exportDOM carries a collapsed note's caller only
as a data-caller attribute, and UnknownNode.exportDOM returns
{element: null}, so figures, sidebars, periphs, refs and optbreaks are
absent from the html altogether. A P10→P9 paste of
`\x - \xo 1:26: \xo*\xt 1Cor 11:7\xt*\x*` therefore arrived as `\x \xo …`
with no caller and rendered nothing. text/html is now the same USFM bytes
as text/plain (usfmToClipboardHtml: HTML-escaped, one
`<p><span style="white-space: pre-wrap;">…</span></p>` per line — the
block shape Lexical's own TextNode export produces, so this editor's own
html re-import path still sees one block per line). invertDisplayNbspInHtml
is gone with the DOM export it normalized. The internal
application/x-lexical-editor flavor is unchanged.
P9's own text/plain reduces a note to the caller glyph it displays (`a`,
or `*` for a `-` caller); the note's bytes ride the html as an escaped
`<!--usfm:…-->` comment inside a span P9 marks `exclude`. getPastePayload
preferred text/plain, so a P9→P10 footnote paste inserted a literal `a`.
paratext9HtmlToUsfm ports Standard-Reverse.xslt over the DOM (text,
usfm: comments with %XXXX unescaping, exclude/include modes, div/tr/br
newlines, span.attribute, usfmopen/usfmclosed with the nested `+`) and
wins over text/plain whenever the html carries P9's signature — a usfm:
comment, or a usfm_<name> class together with usfmopen/usfmclosed, which
no word-processor, browser or P10 html carries.
Under structure protection (Simple interface mode) $handlePasteForStandardView
declined every paste to StructureKeyboardPlugin's html sanitizer. That
carrier could not express a figure (the loss the hand QA saw: the same
clipboard pastes a `\fig` in Power mode and nothing in Simple mode), it
skipped stripPastedChapterAndBookId (a protected paste of `\c 7` created a
second chapter node, the save-loop poisoning the strip exists to prevent),
it skipped the positional NBSP rule, and it would have ignored the P9
decode entirely. The marker engine has no protection gate — a `\v` or `\p`
byte tokenizes into a real marker in either mode, typed or pasted — so the
sanitizer never protected Standard view against marker bytes, only against
verse/para NODES in a DOM import that Standard view's html no longer
produces. The claim now handles a protected document with the same bytes
as an unprotected one, except that it declines a selection
$shouldBlockSelectionReplacement refuses (so that refusal keeps one owner)
and never replays paragraphs: a multi-line payload joins with single
spaces, the sanitizer's own convention for a removed paragraph boundary.
Exposes structureKeyboard.utils from shared-react's index for that
predicate; shared-react is private, so nothing new reaches the published
surface.
Pins: paratext9Clipboard.utils.test.ts (18), the html/plain parity
describe in clipboardCopyFidelity, the P9→P10 paste describe in
markerPasteFidelity, protectedFigurePaste.test.tsx (12 — the same-editor
copy builds the figure under protection, `\c`/`\id` are stripped in both
modes, the P9 comment decodes under protection, a legacy DOM-export html
inserts prose and no figure), and the structure-protection describe in
whitespaceDisplay.plugin.utils.test.tsx, whose Editor→plugin wiring pin
stays falsifiable as a mode difference (a two-line paste yields one
paragraph protected, two unprotected). Semantics doc and changelog updated
to state the new contracts.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rebuilt with `pnpm rebuild-committed-output` after the clipboard and selection changes; the type surface is unchanged (no d.ts or api.md delta). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…to a decorator Rewriting the DOM selection to the decorator's boundary was the part a real browser rejected. Chromium extends a mouse drag only from a base it placed itself: when a press lands in a `\fig ` glyph, the browser's own caret sits in the glyph's text, and replacing it with the scripted element point beside the decorator meant every subsequent mouse move re-placed a collapsed caret under the pointer — no range ever formed, and with a caret Ctrl+C fires no copy event at all. (Chromium also clamps a drag begun inside a `contenteditable="false"` island to that island, so a drag from the glyph selects at most the whole figure; that is the browser's rule, not ours.) The snap now builds the Lexical selection from the snapped points directly (a selection-shaped object handed to $createRangeSelectionFromDom, which reads only the four anchor/focus fields) and, while a pointer button is down, marks it clean so the commit leaves the browser's DOM selection exactly where the browser put it. Raw interior DOM points are confined to the drag: on pointerup/pointercancel the snapped selection is marked dirty in an update whose event type Lexical does not re-derive from the DOM, so the boundary form is written out through the ordinary reconciler path; a snap with no button down (Shift+Arrow, programmatic) writes immediately. Without that materialization the next event-less editor.update() would re-read the interior points, resolve null, and its commit's removeAllRanges() would take the visible selection with it. SKIP_DOM_SELECTION_TAG was not usable for the drag case: a selection-only repair dirties no nodes, so the tag survives into the next commit and suppresses the DOM write for an unrelated caret move (measured). Pins: the DOM points stay untouched while the button is down, are materialized on release, are written immediately with no button down, and the selection survives an event-less update afterwards; the copy pins now dispatch a real `copy` event so the update clones the state selection the way a browser copy does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rebuilt with `pnpm rebuild-committed-output` after the selection-snap rework; the type surface is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rough a read-only construct The `application/x-lexical-editor` flavor exists to carry a construct whole. Lexical cannot slice the token-mode text a read-only construct is built from ($sliceSelectedTextNodeContent skips token and segmented nodes), so a selection ending partway through a figure's caption wrote a COMPLETE figure into that flavor — full caption, every attribute — while text/plain and text/html carried exactly the selected bytes. A paste that reads the internal flavor (a native paste event; the keyboard, context-menu and EditorRef paste paths rebuild the payload from navigator.clipboard.read(), which never exposes it) therefore inserted a second whole figure instead of the four characters selected, and a selection running from prose into a caption invented the rest of the caption and the figure's attributes. $getStandardViewClipboardData now writes only the two text flavors when either end of the selection lies inside an opaque construct ($selectionReachesIntoOpaqueBlock); a selection with both ends outside still ships the internal flavor and round-trips the construct intact. One recorded narrowing: a selection whose end sits at a single-child `ref`'s own end is inside the construct, so that copy now carries the ref's text without its wrapper and `loc` — not a shape a browser drag produces while the construct renders contenteditable=false. Pins: partialConstructClipboard.test.tsx (7; four were red before the change, the text-only paste shape and the whole-construct round trip were green before and after); three boundary pins in unknownClipboardFidelity restated. Semantics doc S2 and the changelog state the rule. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Rebuilt with `pnpm rebuild-committed-output` after the cut-through clipboard flavor change; the type surface is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A foreign Lexical editor's clipboard was treated as internal. `isInternal`
answered the mere presence of `application/x-lexical-editor`, but Lexical's own
fast path is namespace-scoped, so a payload from the app's `"Commenting"` editor
fell through to the `text/html` DOM import — a path that runs neither the
`\c`/`\id` strip nor the positional NBSP rule. `getPastePayload` now mirrors
`$insertDataTransferForRichText`'s actual condition.
The char-stack line replay ignored structure protection. It is registered after
the Standard-view claim at the same priority and before `StructureKeyboardPlugin`
mounts, so the one paste that handler stands aside from reached it and was
replayed with `removeText()` plus a paragraph split per line — deleting the verse
marker protection had just refused to let a paste replace.
`LEADING_MARKER_LITERAL` accepted a closer, inherited from the shared terminated-
marker shape. A leading `\zbold*` therefore read as the paragraph marker `zbold`
(unknown markers are paragraphs), so the host paragraph's own glyph was dropped:
an `\s1` heading came back as a plain `\p`.
A copy that produced no bytes still overwrote the clipboard with an empty string.
The event is now claimed but nothing is written, which is what the collapsed-
selection leg already did.
A CR inside a decoded Paratext 9 note survived `usfmFromComment` and became a
hard line break, splitting the note across two paragraphs.
Also: pointer tracking in `DecoratorBoundarySelectionPlugin` listened for the
press on the root while the release was on the document, so a press landing just
outside the contenteditable root started a drag the plugin never saw; and a
secondary-button press could leave the flag stuck when the native context menu
swallowed the `pointerup`.
Tests: the two internal-paste pins built a `"{}"` payload that was never
same-namespace, so they asserted the old presence-only rule; the optbreak
carrier-agreement pin asserted `not.toContain` against a flavor that is always
empty for that selection, and `UnknownNode.isSelected`'s range branch — the whole
reason the override exists — had no pin anywhere; a mixed-selection pin asserted
only that the document survived, never that the paste landed; and the accepted
Paratext 9 leading-space parity was parked in `KNOWN_LOSSY`, where it was skipped
and protected nothing. Each is now falsifiable, verified by reverting the code it
pins. Two pre-existing `possibly undefined` type errors in the protected-paste
suite are fixed with one `ParaContent` alias.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…fused
`\periph` has no closing marker, so the extent of a peripheral division is a
property of the marker itself and not of its attributes. Refusing the marker
line's attribute list was demoting the whole division to an ordinary paragraph,
which re-parented every block it contained up to the root.
Paratext 9 reads it the other way round: a failed `SetAttributes` leaves the
marker-line text alone (`ParatextData/UsfmToken.cs`) and the USX writer still
opens `<periph>` for any `\periph` token, with an empty `id`
(`ParatextData/UsxUsfmParserSink.cs`). The refused bytes now survive as the
division's own title instead, where the author can still see and fix them, and
they serialize back byte-for-byte because `alt` renders as literal marker content
rather than a pipe pair.
This also removes a live-typing glitch: Standard view re-tokenizes a marker line
as it is typed, so typing `|id="cover"` onto a periph line used to un-nest every
block under it on the `|` keystroke and re-nest them several keystrokes later.
Splitting the marker line at its FIRST `|` is unchanged — that matches P9's own
`text.IndexOf('|')`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…marker Pasting a whole-paragraph copy at an existing paragraph's content start was deleting the HOST paragraph's marker. A whole-paragraph copy carries its own `\p ` literal, so the line ends up with two paragraph markers, and `$withoutRedundantOwnPrefix` resolved that by dropping the host's glyph — an `\s1` heading pasted into became a `\p`, and content that belonged to the host ended up under the pasted marker. Nobody asked for that: a paste inserts what was pasted, and if the result is two paragraph markers in a row, that is what the bytes say. Paratext 9 reads them the same way — `NormalizeTokenUsfm` (ParatextData/UsfmToken.cs) emits a line break before every Paragraph token — and so does typing the same bytes here, so the split-with-empty-predecessor shape is now universal and paste has no rule of its own. Removing it takes the whole paste-provenance apparatus with it: the `isPasteRebuild` parameter through `$buildParaFragment`/`$rebuildParas`/ `$requestTier2ForNode`, `Tier2Context.pasteRebuildArmed`, the `pastePendedKeys` set that carried "this rebuild came from a paste" across a deferred settle, its per-commit pruning, consume-on-use, and the arming callback. The read-only settle and the mutating rebuild now call `$buildParaFragment` with the same arguments, so the two can no longer disagree about a deferred paste's shape — a divergence that let `EditorRef.getUsj()` predict a paragraph the settle would not produce. That exposed a real defect the dedup had been masking. A multi-line paste replays each line break as a paragraph split, and the engine injects a marker prefix onto every fresh split paragraph so it is not read as marker-deleted and merged back — including onto lines that already carry their own marker. For a marker with a separator and content the injected prefix is resolved away; for `\b`, which has neither, both survived and every blank line in a pasted document gained an empty `\p` in front of it (16 of them in the 2sa fixture). The split branch now skips the injection when the paragraph's own text already opens with a paragraph-marker literal. Enter pressed directly in front of a marker literal is the same shape and gets the same answer. Tests: the round-trip suites seeded an empty `\p` as the paste's insertion host and expected the paste to swallow it; they now assert the host is there and strip it, so a genuine stray paragraph can no longer pass as the harness's own. The two pins that covered the dedup are rewritten to cover what replaced it — a pasted marker literal splitting rather than retagging, and a deferred settle producing the identical document for pasted and typed bytes — plus a new pin that a multi-line paste invents no marker for a line that already has one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The kind-rule module's doc comments still named Tier 2's own-marker-prefix dedup as the second user of `isParaKindMarker`; that caller is now `$suppliesOwnParaMarker` in the paragraph-prefix transform, and the leaf-module rationale is restated to match. `$pendDeferredRebuild` only existed to carry the paste provenance, so its two call sites go back to the inline `pendingKeys.add` every other pend in the file uses. Also removes a stray blank line from `$buildParaFragment`'s doc comment. Committed platform dist rebuilt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
lyonsil
left a comment
lyonsil
left a comment
There was a problem hiding this comment.
Review of the copy/paste USFM fidelity work on this branch — the Standard-view clipboard path, the
paste normalization passes, the Paratext 9 decoder, the marker-edit engine's paragraph handling, and
the new decorator-boundary selection plugin. 25 findings, most of them in
whitespaceDisplay.plugin.utils.ts and MarkerEditPlugin.tsx.
Each comment leads with a severity and a short status. The severity is the one settled on after the
finding was checked against the code, not a first impression. checked and confirmed means the
behaviour was reproduced or traced to its cause in this branch — where a comment says something was
run, it was run. Every comment ends with a concrete Fix:; where a fix could not be stated as a
verified edit, it is stated instead as the constraint any fix has to satisfy, and several name an
accompanying test, doc, or changelog update the change would need.
Findings that could not be attached to a line in this diff
#4 (high · checked and confirmed) libs/shared/src/nodes/features/UnknownNode.ts:253 - In Formatted and Paragraph-structure views, copying part of a figure's caption puts the entire figure on the clipboard.
What happens: the over-carry is mitigated only inside $getStandardViewClipboardData (whitespaceDisplay.plugin.utils.ts:836, the $selectionReachesIntoOpaqueBlock omission), and $handleCopyForStandardView is registered only when isStandardView (MarkerEditPlugin.tsx:872). In hidden-marker views Lexical's own copy runs, so $appendNodesToJSON decides everything: the new excludeFromCopy at :253 answers false for any child-bearing construct and the new isSelected at :288 answers true when the caption child is in getNodes(). createUnknown stamps mode: "token" on every text child and $sliceSelectedTextNodeContent refuses to slice those, so the whole construct is serialized.
Why it matters: the hazard the CHANGELOG describes for Standard view — a native paste inserts a second figure and a save persists it — is now reachable in the other editable views.
Fix: do not implement this by changing UnknownNode.excludeFromCopy/isSelected — Lexical hoists an excluded wrapper's children, and the caption child is mode: "token" and unsliceable, so that would emit the caption's full text as loose prose with no wrapper or attributes. Instead add a view-agnostic guard beside $selectionReachesIntoOpaqueBlock in libs/shared-react/src/plugins/usj/OpaqueBlockGuardPlugin.tsx (or wired into libs/shared-react/src/plugins/usj/clipboard.utils.ts) that runs on COPY_COMMAND/CUT_COMMAND in every editable view and, when $selectionReachesIntoOpaqueBlock() is true, keeps application/x-lexical-editor off the clipboard for that copy — mirroring $getStandardViewClipboardData — leaving Lexical's text/plain as the fallback. Pin a Formatted-view partial-caption copy in unknownClipboardFidelity.test.tsx, update docs/clipboard-semantics.md and packages/platform/CHANGELOG.md since this changes clipboard behaviour outside Standard view, and rebuild the committed packages/platform/dist.
How this was checked: The premise holds. MarkerEditPlugin.tsx:564 (if (!isEnabled || !viewOptions) return;) early-returns the whole registration effect for Formatted and Paragraph-structure views, both markerMode: "hidden" (view-options.utils.ts:212-236), so the COPY/CUT handlers at :872-895 never mount there, and OpaqueBlockGuardPlugin explicitly leaves copying untouched. Lexical's stock COPY handler then runs $appendNodesToJSON, which calls excludeFromCopy('html') — never 'clone' — returning false for a child-bearing figure at UnknownNode.ts:253-254, while isSelected at :288-298 returns true because the partially selected caption TextNode is in getNodes(). $sliceSelectedTextNodeContent refuses to slice a token-mode TextNode (LexicalSelection.dev.mjs:257), and createUnknown (usj-editor.adaptor.ts:850-852) stamps mode: "token" on every child regardless of markerMode. So the whole figure, with the caption's full untouched text, serializes into application/x-lexical-editor from a selection covering only part of the caption.
Not attached to a line: no longer on a changed line; cannot be posted inline
#5 (high · checked and confirmed) packages/platform/src/editor/markerEdit/MarkerEditPlugin.tsx:365 - Pasting a \c-bearing multi-line payload into an expanded note deletes the note's selected content and inserts nothing.
What happens: :365 tests the UNSTRIPPED payload.text for a newline, and only then does :378 compute normalizePastedNbsp(stripPastedChapterAndBookId(pastedText)) — and the strip drops whole lines (whitespaceDisplay.plugin.utils.ts:344). For "\\c 5\n\\id MAT" both lines go, leaving lines = [""]. $handlePasteLinesInNote (markerEditNote.utils.ts:113) removes the selection via $removeSelectionTouchingExpandedNote, inserts "", and returns "handled", so :390 calls preventDefault() and the CRITICAL claim consumes the command.
Why it matters: content is deleted with nothing pasted. The single-line variant is wrong too: "\\c 5\nfootnote" collapses to one line but still takes the \fp path and its stripLeadingParagraphMarker, which the genuine single-line path never applies.
Fix: decide emptiness before any selection mutation — $removeSelectionTouchingExpandedNote's selection.removeText() runs as a side effect of merely checking whether the paste applies, so declining afterwards cannot undo it. In the in-note CRITICAL claim, when isStandardView, compute const noteText = pastedText.includes("\n") ? normalizePastedNbsp(stripPastedChapterAndBookId(pastedText)) : pastedText; up front; if it is empty, call event?.preventDefault() and return true before calling $handlePasteLinesInNote, leaving the selection untouched. Do not copy the order at $handlePasteForStandardView:576 — it carries the same defect. Re-run the newline test on noteText rather than pastedText, so "\c 5\nfootnote" collapses to the ordinary single-line path instead of $handlePasteLinesInNote's stripLeadingParagraphMarker. Pin both shapes in noteEnterFp.test.tsx. Rebuild the committed packages/platform/dist (node scripts/rebuild-committed-output.mjs).
How this was checked: Confirmed empirically. In the in-note CRITICAL PASTE_COMMAND claim, the newline test at MarkerEditPlugin.tsx:365 runs on the unstripped pastedText; the strip happens only afterward at :378-379. For "\c 5\n\id MAT", stripPastedChapterAndBookId drops both lines, so lines is [""]. $handlePasteLinesInNote (markerEditNote.utils.ts:113) first calls $removeSelectionTouchingExpandedNote, which unconditionally does selection.removeText() before it knows the pasted content is empty; $insertLineTextAtCaret("") is then a no-op and the claim returns "handled", so preventDefault() fires at :395 and the command is consumed. Verified with the renderStandardEditorWithUnclosedNote fixture: selecting "no" out of a note's "A note" content and pasting "\c 5\n\id MAT" left the note as "A te" — the selection deleted, nothing inserted. The single-line variant also diverges as described: "\c 5\nfootnote" still enters the multi-line branch and gets stripLeadingParagraphMarker, which a genuine single-line paste of "footnote" never receives.
Not attached to a line: no longer on a changed line; cannot be posted inline
#10 (high · checked and confirmed) packages/platform/src/editor/markerEdit/whitespaceDisplay.plugin.utils.ts:902 - A copy with nothing to put on the clipboard still wipes the clipboard when it arrives as a real browser copy event.
What happens: the null-dispatch branch at :893 skips copyToClipboard entirely for an empty payload, so the clipboard is untouched. The native-event branch does not: :902 cancels the browser's own copy unconditionally and :903 then writes nothing. A canceled copy event still commits its DataTransfer, and Blink's SystemClipboard::WriteDataObject replaces the whole clipboard — so an empty object clears it.
Why it matters: the comment at :880 promises "leaving whatever the user copied last intact". It is reachable from any host path that delivers a real ClipboardEvent — an Electron Edit-menu Copy, or a host that does not route through ClipboardPlugin's keydown.
Fix: in $handleCopyForStandardView, restructure the native-event leg so event.preventDefault() and the setData loop both sit inside the !isEmptyPayload branch, leaving preventDefault() uncalled when the payload is empty so the browser's own no-op-on-nothing-to-copy default runs — mirroring the collapsed-caret branch at :872-877, which already declines without calling it. Pin both dispatch legs in the empty-copy describe of whitespaceDisplay.plugin.utils.test.tsx: the null-event leg, and a real ClipboardEvent-shaped stub with a spyable preventDefault, asserting it is NOT called on the native leg when the payload is empty. Add a line to the empty-copy rule in docs/clipboard-semantics.md covering a real native ClipboardEvent reaching a non-collapsed, zero-byte selection, not only the collapsed-caret case already documented. Rebuild the committed packages/platform/dist (node scripts/rebuild-committed-output.mjs).
How this was checked: Confirmed the asymmetry directly: git diff against the merge base shows this PR added the isEmptyPayload guard around the null-dispatch leg's copyToClipboard call (:893) and around the native-event leg's setData loop (:903-904), but left the native leg's event.preventDefault() at :902 unconditional, exactly as it was pre-PR. The native-event leg is genuinely reachable: Lexical's core attaches a real copy DOM listener to the editor root and dispatches COPY_COMMAND with the actual ClipboardEvent, and MarkerEditPlugin.tsx:879-891 forwards it unchanged into $handleCopyForStandardView at COMMAND_PRIORITY_HIGH, above Lexical's own fallback — so any host path delivering a genuine native copy event, an Electron or OS Edit-menu Copy being the standard example, reaches this branch. Within this repo's own UI, ClipboardPlugin and ContextMenuPlugin only dispatch via copySelection/cutSelection, which pass null (clipboard.utils.ts:60,66). The sibling collapsed-caret branch a few lines above (:872-877) establishes the intended precedent: for a real ClipboardEvent with nothing to copy it returns false without calling preventDefault, precisely so the browser writes nothing — docs/clipboard-semantics.md:27 states this outright. The empty-payload branch breaks that pattern.
Not attached to a line: no longer on a changed line; cannot be posted inline
#27 (low · checked and confirmed) packages/platform/src/editor/markerEdit/whitespaceDisplay.plugin.utils.ts:199 - One Ctrl+V parses the pasted HTML six times before a byte is inserted.
What happens: getPastePayload unconditionally computes htmlText at :201 (a DOMParser.parseFromString plus three querySelectorAll sweeps) and paratext9Text at :221 (a second parse, a comment TreeWalker, and a querySelectorAll("*") over every element) before isInternal or the selection is consulted. Three handlers each call it for the same event: $handlePasteForStandardView (:548), the in-note CRITICAL claim (MarkerEditPlugin.tsx:362) and the char-stack claim (MarkerEditPlugin.tsx:433). htmlText is only ever read by plainText || htmlText, which essentially no real clipboard reaches.
Why it matters: pasting a Word, Outlook or browser payload — hundreds of KB is routine — blocks the main thread inside editor.update() for six full parses. The base commit did one.
Fix: gate paratext9HtmlToUsfm behind html.includes("usfm:") || html.includes("usfmopen") || html.includes("usfmclosed") — every branch of hasParatext9Signature requires one of those literal byte runs, so this cannot silently disable the P9 decode. Make the local htmlText in getPastePayload (:201) compute on demand inside the plainText || htmlText fallback; it is not a PastePayload field, so no consumer is affected. For the repeat calls, prefer computing the payload once in the shared caller over a module-level cache: Lexical's triggerCommandListeners hands every priority tier the same payload reference for one dispatchCommand, so no cache is needed. If a WeakMap is used anyway, guard the key — read and write it only when event !== null && typeof event === "object", because WeakMap throws on a null key and this function's signature admits null. Add a call-count assertion (spy on DOMParser.prototype.parseFromString) in whitespaceDisplay.plugin.utils.test.tsx proving the protected + char-stack path parses once per event, and rebuild the committed packages/platform/dist.
Not attached to a line: no longer on a changed line; cannot be posted inline
(AI-assisted, with my guidance)
| // confined to one commit: `$commitPendingUpdates` only empties `editor._updateTags` when the update | ||
| // dirtied nodes, and a selection repair dirties none — so the tag would survive into the next | ||
| // commit and swallow the DOM write for the caret move after it. | ||
| selection.dirty = !isPointerDown; |
There was a problem hiding this comment.
#3 - high · checked and confirmed
A drag that starts on a read-only glyph dies mid-drag whenever another selection-change listener touches a node in the same commit.
What happens: $commitPendingUpdates (installed lexical@0.43.0, Lexical.dev.mjs:8497) sets needsUpdate = editor._dirtyType !== NO_DIRTY_NODES — true whenever any node was dirtied this commit — and the DOM-selection write at :8588 (if (needsUpdate || pendingSelection === null || pendingSelection.dirty)) then runs regardless of pendingSelection.dirty === false. onSelectionChange dispatches SELECTION_CHANGE_COMMAND from inside that same update, and every priority runs inline in the one commit. $syncCaretHost (transientCaretHost.ts, reached from the unconditionally mounted EmptyVerseCaretGuardPlugin/TrailingNoteCaretGuardPlugin, Editor.tsx:1127/1164) evicts a stale placeholder host and dirties a node in that same commit, so selection.dirty = !isPointerDown at :184 is ignored and the scripted element point is written over Chromium's drag base.
Why it matters: the plugin's own doc at :216 says writing that base "stops the drag dead" — no range forms and Ctrl+C reaches the empty-copy guard. It fails intermittently, keyed on whether a caret host happened to be present.
Fix: do not move the snap into a nested editor.update() — a nested call from inside a command handler is queued to a later, separate commit, so the current commit still has pendingSelection === null and updateDOMSelection calls domSelection.removeAllRanges(), destroying the drag base on every unresolved arrival and failing the existing button-down pin. Keep $setSelection(selection) and selection.dirty = !isPointerDown synchronous exactly as today (:170-185), and additionally call $addUpdateTag(SKIP_DOM_SELECTION_TAG) (from "lexical"; $addUpdateTag is already used in transientCaretHost.ts, so the import is proven reachable) in the same CRITICAL handler whenever isPointerDown is true — the outer guard that reads it is not gated by needsUpdate. Because editor._updateTags is auto-cleared only when the commit dirties a node, register an update listener that removes just that tag on the next commit whose tags carry it, so it cannot swallow the DOM write for a later, unrelated selection change; that field is private, so say why at the site. Pin a drag with a stale caret host armed, plus a follow-up selection change proving the tag did not leak, in decoratorBoundarySelection.test.tsx. Rebuild the committed packages/platform/dist (node scripts/rebuild-committed-output.mjs).
How this was checked: Read the installed Lexical source directly. $commitPendingUpdates (Lexical.dev.mjs:8497) sets needsUpdate = editor._dirtyType !== NO_DIRTY_NODES, and the DOM-selection write at :8588 calls updateDOMSelection when needsUpdate is true regardless of pendingSelection.dirty === false. onSelectionChange (:2190/:2288) dispatches SELECTION_CHANGE_COMMAND from inside the same updateEditorSync, and triggerCommandListeners runs every priority inline in that one active update, so all listeners genuinely share one commit. Of the two coincident listeners, NoteNodePlugin.tsx:151 bails at its own gate (viewOptions?.noteMode !== "expandInline", :307) and no view mode ever sets that value, so it is dead code today and does not contribute. $syncCaretHost (transientCaretHost.ts, used by the unconditionally mounted EmptyVerseCaretGuardPlugin/TrailingNoteCaretGuardPlugin, Editor.tsx:1127/1164) is live, and the failure reproduces: resting the caret in an empty verse so a placeholder host is planted, holding the pointer down, then collapsing the DOM caret inside a figure's \fig glyph, the committed selection came back dirty: false — the plugin correctly detected the drag — yet the DOM's anchorNode was rewritten from the raw glyph text node to the figure's own element at offset 0, and the placeholder host was gone from the tree in that same commit.
Other findings in this file: #21
| isMaterializePending.current = false; | ||
| // Re-dirty the snapped selection so Lexical's ordinary reconciler pass writes the boundary form | ||
| // into the DOM, now that no drag needs the browser's own base any more. | ||
| editor.update(() => { |
There was a problem hiding this comment.
#21 - low · checked and confirmed
The pointer-release handler opens an editor.update() from inside a DOM event listener, which this repo's CLAUDE.md forbids.
What happens: CLAUDE.md (repo root, Code Style) states: "Don't call editor.update(...) from inside a listener (event handler, update listener, etc.) — nested updates risk infinite cascade loops. Lexical's command system (editor.registerCommand) is the canonical entry point for mutating state in response to user interactions." release is registered as a capture-phase document listener for pointerup and pointercancel at :300–:302, and its body opens an update at :285.
Why it matters: there is precedent in the codebase (NoteNodePlugin.tsx:128, ContextMenuPlugin.tsx:251), so this is a convention violation rather than a live cascade risk. It is order-fragile: if the release lands while another update is in flight, Lexical queues the callback, window.event is no longer the pointerup the doc comment at :234 relies on, and the update can re-derive its selection from the DOM. The same pattern already exists at NoteNodePlugin.tsx:128 and ContextMenuPlugin.tsx:251, so this continues a mixed convention rather than inventing one — though ActiveTextPlugin.tsx:52 shows the team deliberately avoids the same trap elsewhere.
Fix: do not switch to editor.dispatchCommand on the grounds that it removes the window.event dependency — it does not: dispatchCommand resolves through triggerCommandListeners's updateEditorSync, which reaches the same $beginUpdate/$internalCreateSelection(editor, null) path, and therefore the same windowObj.event fallback, whenever this editor is not already the active one — the ordinary case here. Instead add a comment immediately above release at DecoratorBoundarySelectionPlugin.tsx:279 stating that this editor.update() is a deliberate, precedented exception to the CLAUDE.md rule (matching NoteNodePlugin.tsx:128 and ContextMenuPlugin.tsx:251), and name the constraint it depends on: it must run synchronously off a trusted pointerup/pointercancel dispatch with no update of this editor already active, because Lexical queues the call otherwise (the editor._updating gate in updateEditor) and the queued replay, drained via $processNestedUpdates, invokes the callback without calling $internalCreateSelection again — at which point window.event no longer reflects that pointerup.
How this was checked: CLAUDE.md's Code Style section (line 268) reads verbatim: "Don't call editor.update(...) from inside a listener (event handler, update listener, etc.) — nested updates risk infinite cascade loops. Lexical's command system (editor.registerCommand) is the canonical entry point for mutating state in response to user interactions." No exception is carved out. release (DecoratorBoundarySelectionPlugin.tsx:279-289) is registered as a capture-phase pointerup/pointercancel document listener at :300-302 and opens editor.update() at :285-288; the whole file is new in this PR. The class doc at :236-239 explains why the update must happen at this moment: inside a pointerup listener, Lexical's $internalCreateRangeSelection reads windowObj.event when no explicit event is passed, and pointerup is not one of the types that makes it re-derive the selection from the DOM, so the update clones the already-snapped selection instead of re-resolving interior points. That is load-bearing, not stylistic. The cited precedent checks out: NoteNodePlugin.tsx:163 wires a dblclick listener whose body calls editor.update(...) at :127-128, and ContextMenuPlugin.tsx:260 wires a capture-phase keydown whose Enter branch calls editor.update(...) at :251.
Other findings in this file: #3
| // together. Ejecting there strands the construct's own text outside it: a figure's caption | ||
| // lands after the `\fig*` glyph as ordinary paragraph prose, and the exported USJ keeps a | ||
| // `figure` object with no content at all. | ||
| if (wasNodeCreated(editor, node.getKey()) && !wasNodeCreated(editor, unknownNode.getKey())) |
There was a problem hiding this comment.
#22 - low · checked and confirmed
Text planted inside a figure is left there instead of being ejected whenever the same commit also rebuilds that figure.
What happens: wasNodeCreated is !$getNodeByKey(key) read against the committed state (libs/shared-react/src/nodes/usj/node-react.utils.ts:359), so it is true for any node whose key is new in the running update. A Tier-2 rebuild replaces a paragraph's children with fresh keys, so in any commit that rebuilds the host paragraph the new !wasNodeCreated(unknownNode) conjunct at :195 is false and the ejection is disabled for that whole commit.
Why it matters: this transform is the backstop for edits OpaqueBlockGuardPlugin's selection-based refusals do not see — it keys on $getSelection(), which is stale for DROP and absent for collab. A stray TextNode planted by a drop, a collab op or a rich paste in such a commit stays inside the read-only construct and serializes into the figure's USJ content.
Fix: decide on the TEXT node's provenance rather than the wrapper's key age. Any replacement must distinguish "this text arrived with the wrapper the materializer just built" from "this text is new in a commit that happened to rebuild the wrapper", without reading key age. Extend the boundary pin in TextSpacingPlugin.test.tsx to a rebuild-plus-intrusion commit. Rebuild the committed packages/platform/dist (node scripts/rebuild-committed-output.mjs) so verify-committed-dist stays green.
| // editable-marker view's own `text/html` is not a DOM export at all (Standard view renders the | ||
| // copy walker's USFM bytes — `usfmToClipboardHtml`, platform's whitespaceDisplay.plugin.utils.ts), | ||
| // and where Lexical's exporter DOES run, `$appendNodesToHTML` (`@lexical/html`) computes this same | ||
| // `excludeFromCopy('html')` value but returns early on `exportDOM()`'s unconditional |
There was a problem hiding this comment.
#23 - low · checked and confirmed
In Formatted and Paragraph-structure views the two readable clipboard flavors still decode to different documents.
What happens: exportDOM (:182) still returns {element: null} unconditionally, and $appendNodesToHTML (@lexical/html) returns early on a null element before it ever consults excludeFromCopy — which the new comment at :216 itself states. The CHANGELOG's fidelity fix lands only through $getStandardViewClipboardData, registered solely under isStandardView (MarkerEditPlugin.tsx:872).
Why it matters: in the hidden-marker views the stock Lexical copy runs, so a copy spanning a figure ships text/html with the construct absent while text/plain (selection.getTextContent()) carries its display text. The "both readable flavors decode to one document" promise holds only in Standard view.
Fix: prefer documenting the existing scope over changing exportDOM. Add a line to the text/html entry in packages/platform/CHANGELOG.md and to the top of docs/clipboard-semantics.md stating that Formatted, Paragraph-structure and Unformatted copies still go through Lexical's native $generateHtmlFromNodes, which drops any UnknownNode/ImmutableNoteCallerNode subtree from text/html — unchanged from before this PR. If a functional fix is pursued instead, it must not stop at UnknownNode.exportDOM returning a DocumentFragment: StructureKeyboardPlugin's protected-mode $handlePaste imports text/html directly, so the change must also pin a protected-mode paste of a figure in StructureKeyboardPlugin.test.tsx, asserting the result is either the reconstructed figure or a decline — never loose caption text — and still update both documents.
Other findings in this file: #24
| // boundary rather than a structural loss. That residual stands deliberately: closing it needs a | ||
| // lever Lexical does not offer — `exportNodeToJSON` requires every ElementNode's `exportJSON()` | ||
| // to return a `children` array, so a node cannot say "drop me AND my children". | ||
| override isSelected(selection?: BaseSelection | null): boolean { |
There was a problem hiding this comment.
#24 - low · checked and confirmed
UnknownNode.isSelected() reports a figure as selected when the caret is merely resting inside its caption with nothing selected.
What happens: super.isSelected is consulted only on the $isNodeSelection arm at :288, so for a RangeSelection the override at :298 replaces the base entirely. Lexical's base returns false for any element-type range that isCollapsed(); a collapsed caret inside a figure caption puts that caption in getNodes(), so figure.isSelected() returns true with nothing selected. Conversely a childless construct answers false for a range that fully contains it. The base's inline-decorator-at-parent-end carve-out is dropped as well.
Why it matters: isSelected is public API — $removeNode calls it with no argument, and host or plugin code asking for decoration state now gets a different meaning here than from every other node in the tree.
Fix: keep the $isNodeSelection gate on the base branch and add a collapsed-selection short-circuit before the child-membership test in UnknownNode.isSelected: after the existing $isNodeSelection(targetSelection) && super.isSelected(targetSelection) branch, if (targetSelection.isCollapsed()) return false;, then the existing getChildren().some(...) test. A NodeSelection.isCollapsed() is hard-coded false (Lexical.dev.mjs:5837-5839), so this cannot suppress the node-selection branch, and no extra narrowing is needed. Do NOT make super.isSelected unconditional — that turns the pin at UnknownNode.test.ts:461-482 red and does not fix the defect anyway, because the child-membership clause still matches. Verified: all 24 existing tests pass, the collapsed-caret probe now returns false, and a childless construct inside a non-collapsed range still returns false. Add a collapsed-caret-in-caption case to the isSelected() describe in UnknownNode.test.ts, and rebuild the committed packages/platform/dist.
Other findings in this file: #23
| } | ||
| prevWasElement = false; | ||
| if ($isImmutableNoteCallerNode(node)) { | ||
| if (node !== lastNode || !selection.isCollapsed()) text += ` ${$noteCallerText(node)}`; |
There was a problem hiding this comment.
#19 - medium · checked and confirmed
A copy that starts at a collapsed note's caller begins with a stray space and a caller that has no opening marker.
What happens: the leading space at :735 stands for the mandatory separator after \f/\x, but the guard copied from Lexical's decorator branch only suppresses the trailing-boundary case (node !== lastNode). There is no node !== firstNode arm, and $getStandardViewClipboardData already refused a collapsed selection, so !selection.isCollapsed() is always true and the branch always emits. A selection anchored on the caller therefore produces " + \\fr 1:1 \\ft …\\f*".
Why it matters: the doc comment at :648 asserts "the sibling caller rule guards its own boundary the same way" — it guards only one end. Pasted back, the stray space lands in body content ahead of an unmatched \f*. The anchor is genuinely reachable: every node inside a collapsed note except the caller's own glyph is display: none, so a click at the left edge of the caller's rendered button lands exactly there, and neither NoteShellCaretGuardPlugin nor ArrowNavigationPlugin corrects it — both scope their shell-atomicity handling to expanded notes only.
Fix: at whitespaceDisplay.plugin.utils.ts:735, keep the caller's character and drop only the invented leading space when the opening marker is not part of the selection — the space and $noteCallerText(node) are one template string today, so they must be split rather than gated together: text += (node !== firstNode ? " " : "") + $noteCallerText(node); inside the existing node !== lastNode || !selection.isCollapsed() guard. Gating the whole expression on node !== firstNode would drop the caller character itself, losing selected content. Pin both shapes in clipboardCopyFidelity.test.tsx: a selection starting at the caller, which must carry no leading space, and the whole-note case, which must still produce the opening marker followed by its separator and caller. Add a line to docs/clipboard-semantics.md noting the caller's leading space is conditional on the opening marker being in the selection. Rebuild the committed packages/platform/dist (node scripts/rebuild-committed-output.mjs).
How this was checked: Reproduced directly: with the selection anchored as an element point right before the collapsed note's caller (whitespaceDisplay.plugin.utils.ts:735), excluding the note's opening \f marker, and the focus at the end of the note, the text/plain payload came back as " - \cat People\cat*\fr 1:1 \ft Caller test.\f*" — a leading space and the caller with no \f in front of it. That anchor is reachable: every node inside a collapsed note except the caller's own glyph is display: none (usj-nodes.css, .note.collapsed span[data-lexical-text="true"]), so a click at the left edge of the caller's rendered button lands exactly there, and neither NoteShellCaretGuardPlugin nor ArrowNavigationPlugin corrects it — both scope their shell-atomicity handling to getIsCollapsed() === false, expanded notes only (NoteShellCaretGuardPlugin.tsx:34, ArrowNavigationPlugin.tsx:568).
Other findings in this file: #1, #2, #9, #16, #17, #20, #28, #29, #30, #31
| text += $isNoteInternalDisplaySeparator(node) | ||
| ? "" | ||
| : nodeText.replaceAll(NBSP, " ") + $collapsedNoteCategoryBytes(node, node === lastNode); | ||
| } else if ( |
There was a problem hiding this comment.
#20 - medium · checked and confirmed
A copy that includes the \id line puts a literal non-breaking space on the clipboard.
What happens: createBook (packages/platform/src/editor/adaptors/usj-editor.adaptor.ts:309) builds the book line in markerMode: "editable" as createImmutableTypedText("marker", openingMarkerText(marker) + " " + code + NBSP), and ImmutableTypedTextNode extends DecoratorNode. The committed fixture confirms the byte: libs/test-data/src/data/2sa.lexical.editable.ts:22 is "\\id 2SA\u00A0". The decorator branch at :760 does text += node.getTextContent() with no inversion, while the sibling $isTextNode branch at :755 does nodeText.replaceAll(NBSP, " ").
Why it matters: it contradicts the documented rule that no NBSP reaches either readable flavor; pasting into Paratext 9 or a text editor delivers a non-breaking space where the marker's structural separator belongs, and USFM's byte form for a data NBSP is ~. It is exactly this narrow: a sweep of every other createImmutableTypedText site found the editable chapter and verse glyphs are TextNodes that already invert correctly, and every raw-NBSP decorator branch is gated to non-Standard-view modes — createBook alone pushes its decorator for editable mode too.
Fix: in whitespaceDisplay.plugin.utils.ts, change the decorator/linebreak branch at :756-761 to text += node.getTextContent().replaceAll(NBSP, " "); — NBSP is already imported in this file. Add the missing calling-context line to $selectionToUsfmText's TSDoc, which CLAUDE.md requires of every exported $-prefixed function and which this new function lacks: it is read-only, so say it is safe from editor.getEditorState().read(...) or inside a command handler. Pin a copy spanning a book's \id line in clipboardCopyFidelity.test.tsx — test-data's usj2Sa already carries the byte — asserting the text/plain payload has a plain space after the book code. Add an [Unreleased] › ### Fixed entry to packages/platform/CHANGELOG.md, and a bullet to docs/clipboard-semantics.md naming this case and its pin, matching that file's existing per-case bullets. Rebuild the committed packages/platform/dist (node scripts/rebuild-committed-output.mjs).
How this was checked: The $isDecoratorNode/$isLineBreakNode branch of $selectionToUsfmText (whitespaceDisplay.plugin.utils.ts:756-761) does text += node.getTextContent() with no NBSP inversion, while the sibling $isTextNode branch at :755 does nodeText.replaceAll(NBSP, " "). The book's \id line is built by createBook (usj-editor.adaptor.ts:309) as an ImmutableTypedTextNode, which extends DecoratorNode and whose getTextContent() returns this.__text verbatim, NBSP included. The committed fixture confirms the raw byte: libs/test-data/src/data/2sa.lexical.editable.ts:22 hexdumps as c2 a0. Building a minimal book+para USJ, mounting it through MarkerEditPlugin in Standard view, selecting the whole document and dispatching COPY_COMMAND produced a text/plain payload with a live, literal NBSP right after the book code (hex ...325341c2a02d2054...). docs/clipboard-semantics.md states the governing rule outright: text/plain inverts every display-NBSP to a plain space and leaves a data-NBSP as the ~ the display already shows, with the consequence that no NBSP reaches either readable flavor. The \id line's trailing NBSP is a structural display separator, not a data-NBSP, so this violates a documented guarantee rather than taking an intentional exception.
Other findings in this file: #1, #2, #9, #16, #17, #19, #28, #29, #30, #31
| * The `text/html` clipboard flavor for `usfm` — the SAME bytes as `text/plain`, HTML-escaped and | ||
| * laid out one block per line. | ||
| * | ||
| * Standard view's fidelity carrier is USFM text (`docs/clipboard-semantics.md`, S1/S3), and the two |
There was a problem hiding this comment.
#30 - low · checked and confirmed
A spec-section label ships in a source comment, which this repo's CLAUDE.md forbids.
What happens: the TSDoc at :770 reads "Standard view's fidelity carrier is USFM text (docs/clipboard-semantics.md, S1/S3)". CLAUDE.md (repo root, Code Style) states: "Keep issue-tracker references out of code and comments: no Jira IDs (e.g. PT-4187), internal task/QA labels (Task 8), or spec section numbers (§5.5). Code must stand on its own."
Why it matters: the section numbers are unresolvable from the code and go stale the moment the document is reordered. The docs/clipboard-semantics.md path itself is fine and the rule does not bar it.
Fix: drop the S1/S3 label at :770 and keep the document path, naming the rule the comment relies on instead of its section number.
How this was checked: The TSDoc at whitespaceDisplay.plugin.utils.ts:770 reads "Standard view's fidelity carrier is USFM text (docs/clipboard-semantics.md, S1/S3)", which carries the spec-section labels S1/S3. CLAUDE.md forbids exactly this: "Keep issue-tracker references out of code and comments: no Jira IDs (e.g. PT-4187), internal task/QA labels (Task 8), or spec section numbers (§5.5). Code must stand on its own." The sections do exist in docs/clipboard-semantics.md (at :11 and :58), and the document path itself is allowed by the same rule; the section labels are what it bars. git diff confirms this line was added by this PR.
Other findings in this file: #1, #2, #9, #16, #17, #19, #20, #28, #29, #31
| return usfm | ||
| .split("\n") | ||
| .map((line) => line.replaceAll("&", "&").replaceAll("<", "<").replaceAll(">", ">")) | ||
| .map((line) => |
There was a problem hiding this comment.
#31 - low · checked and confirmed
A copy containing an empty paragraph yields one fewer paragraph through text/html than through text/plain.
What happens: the doc at :787 asserts "nothing the copy walker produces contains one" (a blank line). That is not what the mirrored walk does: $selectionToUsfmText sets prevWasElement = !node.isEmpty() at :730, so an empty non-inline element between two non-empty ones contributes \n when visited and fails to suppress the next block's \n, giving "a\n\nb". usfmToClipboardHtml at :799 then emits <p>a</p><p></p><p>b</p>, and htmlPasteText's .replace(/\n+/g, "\n") at :105 collapses the pair back. A trailing \n is trimmed by the same tail.
Why it matters: it breaks this file's own rule that a consumer's flavor choice cannot change the content it receives. It is reachable wherever a top-level ParaNode is genuinely empty — a showParaMarkerPrefixes: false surface, or a paragraph mid-rebuild. The showParaMarkerPrefixes: false popover's scaffold paragraph normally stands alone rather than sandwiched between two content paragraphs, so the everyday trigger is more likely a mid-rebuild transient than that surface.
Fix: narrow the promise instead of chasing a round trip htmlPasteText cannot give. Its collapse-and-trim at whitespaceDisplay.plugin.utils.ts:105 is shared by every paste path in this file — getPastePayload's fallback and both MarkerEditPlugin claims — and exists to tolerate messy nested HTML from real clipboards; loosening it to preserve this file's own flat per-line output would have to be re-verified against all of them, for a benefit that only matters when a top-level paragraph is genuinely empty. State the limit where the guarantee is already stated: in docs/clipboard-semantics.md, next to the "the two decode to the same document text" line, record that an interior or trailing blank line in the selection does not survive the text/html leg — text/plain carries it exactly, text/html collapses it, by the nature of htmlPasteText's shared newline handling. Replace the proposed htmlPasteText(usfmToClipboardHtml(x)) === x assertion with a pin in the usfmToClipboardHtml mechanics describe asserting the actual output for "a\n\nb" and "a\nb\n" — that the round trip returns "a\nb", not the original — so the limitation is enforced rather than merely written down. Leave usfmToClipboardHtml's <p></p> emission and the pin at :503-508 untouched. This adds only a doc line and a test, so it needs no CHANGELOG entry and no dist rebuild; the alternative route of changing htmlPasteText's collapsing logic does change clipboard behaviour and picks up both, plus a re-run of every paste test that depends on the current collapsing.
How this was checked: Running usfmToClipboardHtml then htmlPasteText on "a\n\nb", "a\nb\n" and "a\n\n\nb" (whitespaceDisplay.plugin.utils.ts:795-811 and :93-105) shows all three round-trip to "a\nb" — the interior blank line and the trailing blank line are both lost when a consumer reads text/html instead of text/plain. The doc comment's claim that this cannot happen ("nothing the copy walker produces contains one", :787-789) is also wrong on its own terms: building a genuinely empty top-level ParaNode between two non-empty ones and running the real copy path ($selectionToUsfmText via $getStandardViewClipboardData) produces text/plain: "a\n\nb" directly, because prevWasElement = !node.isEmpty() at :730 treats a childless element the same as "no element seen yet". A childless top-level paragraph is not hypothetical: showParaMarkerPrefixes: false (libs/shared-react/src/views/view-options.utils.ts:120-137) is a real shipped option the footnote-editor popover uses, named explicitly in note-ops-popover-roundtrip.test.tsx:62-69.
Other findings in this file: #1, #2, #9, #16, #17, #19, #20, #28, #29, #30
| // an empty `<p>`. The event is still CLAIMED — the selection is this handler's to answer — it | ||
| // just writes nothing, leaving whatever the user copied last intact. A cut still removes the | ||
| // range: the bytes it would have carried are the ones that do not exist, not the nodes. | ||
| const isEmptyPayload = !data["text/plain"] && !data["application/x-lexical-editor"]; |
There was a problem hiding this comment.
#9 - high · checked and confirmed
The empty-copy guard does not fire for the case its own comment names, so copying an optbreak husk replaces the clipboard with an empty string.
What happens: the comment names "an optbreak husk … selected by the two element points either side of it". For that selection both ends are outside the husk, so $selectionReachesIntoOpaqueBlock() is false and $getStandardViewClipboardData (:836) still calls $getLexicalContent. $getLexicalContent returns null only when getNodes().length === 0 (@lexical/clipboard); here it is non-empty, so it returns the string {"namespace":"…","nodes":[]}. isEmptyPayload at :887 is therefore true && false === false, and the payload is written with "text/plain": "".
Why it matters: the same holds for any range covering only nodes the walker drops, such as a note's internal NBSP separator. The user's clipboard is replaced with an empty string instead of being left alone.
Fix: key the gate on the readable bytes — change whitespaceDisplay.plugin.utils.ts:887 to const isEmptyPayload = !data["text/plain"];, so a lexical flavor with an empty nodes array cannot mask an empty copy. Pin the husk selection (an optbreak with its own // child removed, selected by two element points on the parent flanking it) in optbreakClipboardFidelity.test.tsx. Add a line to the empty-copy rule in docs/clipboard-semantics.md stating that it also covers a non-collapsed selection whose only covered node is a childless husk construct, not just a collapsed caret or a null-payload dispatch. Rebuild the committed packages/platform/dist (node scripts/rebuild-committed-output.mjs).
How this was checked: Reproduced the exact scenario the comment above the check names. In a live editor, removing an optbreak's own // display child creates the childless husk the comment describes; setting a selection with two element points on the parent paragraph flanking it matches "the two element points either side of it". With that selection $selectionReachesIntoOpaqueBlock() returns false, so $getStandardViewClipboardData (whitespaceDisplay.plugin.utils.ts:836-842) proceeds to call $getLexicalContent(editor), which returned the literal string {"namespace":"TestEditor","nodes":[]} — non-empty and truthy even though it encodes zero nodes. That is assigned to data["application/x-lexical-editor"] while data["text/plain"] is "". At :887 the guard evaluates to true && false = false, so it does not fire and the handler writes the near-empty payload. On the common Ctrl+C and context-menu path (copyToClipboard(editor, null, data) at :893) that round-trips through a synthesized native copy event and overwrites the real system clipboard's text/plain with an empty string.
Other findings in this file: #1, #2, #16, #17, #19, #20, #28, #29, #30, #31
2 participants
Implements PT-4201 (WI-14): copy/paste USFM fidelity in Standard view.
What this fixes
Verified against the current
main, which still had none of these fixes.Copy
text/plainis byte-faithful USFM: note callers emitted (were silently dropped and rewritten to+on re-paste), no phantom separator spaces,AttributeRunNode-transparent.#to the clipboard.@lexical/clipboard's execCommand hack injects a hidden<span>#</span>and bails beforepreventDefault()on an empty selection, so Chromium copied the placeholder. Reported for\fig, but it fired for a plain collapsed caret in every view and in scribe too, and for cut.excludeFromCopynow keys on child count rather than tag: previously a copied figure/sidebar/periph/ref pasted its full literal USFM bytes on screen while the USJ silently dropped the node and its attributes.Paste
~) replacing the blanket mapping that produced\f~ \fr~1:1 …unknown-marker soup.\c/\idstripped anywhere in a pasted line — a pasted\cused to poison the save loop ("Multiple chapter markers present") with the error only in the renderer log.\fig's caption is no longer removed — a shared text-spacing transform ejected the freshly-materialized caption out of its wrapper. Only reproducible on a fullEditormount, which is why engine-level tests never saw it. The misplaced gray box was the same defect. It was also silently breaking\refpastes.A paste's provenance now survives a deferred Tier-2 settle, so a pended rebuild keeps the own-prefix dedup (typed input keeps its existing behavior, pinned both ways).Removed in the code-review round below — the dedup it carried was deleting the host paragraph's marker. See "Paste no longer edits the host paragraph's marker".Review follow-ups (this round)
UnknownNode.isSelecteddropped the node-selection case. The override answered purely on whether any of the node's own children were inselection.getNodes(), which is the right test for the range boundary it was written for but is never satisfied by aNodeSelection— that marks a node by its own key and puts no children in the list. A construct selected outright therefore reported unselected, and$appendNodesToJSONhoists an excluded wrapper's (also unselected) children in its place, so the copy would have carried nothing for it. The base key-membership answer now runs first for a node selection; the child-membership rule is unchanged for everything else. Not reachable from today's UI — both$createNodeSelectionsites target verse markers — but it is a trap for whoever adds node-selection over figures or tables.ContextMenuPlugindepended onClipboardPluginfor its empty-copy guard. Both shipped editors mount the pair, but the plugin is exported on its own, and a host mounting it alone got exactly the placeholder-overwrites-clipboard bug the guard exists to prevent. It now registers the guard itself; a duplicate registration at the same priority is inert (the first listener to claim an empty selection stops propagation).Both are covered by tests that go red when the fix is reverted — including a
ContextMenuPlugin-without-ClipboardPluginmount.Verification
Full workspace green (
--skip-nx-cache): platform-editor 1828, shared 683, shared-react 1666, utilities 56; typecheck and lint clean. Semantics documented indocs/superpowers/specs/2026-08-06-clipboard-semantics.md, with accepted asymmetries and remaining deferrals listed explicitly.Re-verified after the follow-ups above:
shared697,shared-react1674 (+1 skipped),platform-editor1857 (+4 skipped);nx format:check, lint and typecheck clean.The companion e2e spec runs and passes through the real OS clipboard (paranext-core #2656), against a build of this branch.
Every fix here was reviewed by reverting it and confirming the pins go red. Two review rounds caught defects in my own earlier attempts, including one that would have shipped silent data loss.
AI-assisted — Claude Code (Opus 5). Task ledger and per-task reports in
.superpowers/sdd/(untracked).🤖 Generated with Claude Code
https://claude.ai/code/session_01QCjocx9uZcPqB2esPh9r6f
Hand-QA follow-ups (2026-09-16/17)
Four findings from the first hand-QA pass (
ddea1549+40d06486,a1dede31, dist44d69dbd/e91c930c):$internalResolveSelectionPointresolves a DOM point inside a decorator's element tonull, nulls the whole selection, and the reconcilerremoveAllRanges()— a\fig's\fig/|src="…"/\fig*glyphs areImmutableTypedTextNodedecorators whose text is a DOM child of the decorator, so a drag whose focus entered the attribute run collapsed. NewDecoratorBoundarySelectionPlugin(shared-react, mounted in every view) handlesSELECTION_CHANGE_COMMANDat CRITICAL and gives Lexical the selection it refused: each offending end snapped to the decorator's boundary in its parent (a range grows outward so the glyph is wholly in; a caret takes the nearer side), built via$createRangeSelectionFromDomfrom a selection-shaped object. (2) Measured in the Electron app: Chromium extends a mouse drag only from a base it placed itself, so the plugin must NOT rewrite the DOM selection while a button is down — it marks the snapped selection clean during a drag and materializes it into the DOM onpointerup/pointercancel(an event type Lexical does not re-derive from the DOM), and writes immediately for keyboard/programmatic arrivals. Chromium also clamps any drag begun inside acontenteditable="false"island to that island, so a drag from the\figglyph selects at most the whole figure — the browser's rule, recorded in the comments and the e2e. The note caller is excluded (its landings belong toTrailingNoteCaretGuardPlugin; measured conflict, pinned).IHtmlEditorCopyPaste.cs, thenStandard-Reverse.xsltreads the fragment'stext()as USFM). Standard view'stext/htmlwas Lexical's DOM export, which carries a collapsed note's caller only asdata-callerand omits everyUnknownNode(exportDOM→{element: null}), so P9 received\x \xo …\x*.text/htmlis now the same USFM bytes astext/plain(usfmToClipboardHtml;invertDisplayNbspInHtmlremoved with the export it normalized). Asymmetries Flaky interaction tests: ScriptureReferencePlugin and marker-palette each failed once in CI #3/Consider committing built dist/ like platform-bible-react, so consumers stop building at install time #4 indocs/clipboard-semantics.mdare closed by construction.text/plainis the visible text (a note = its caller), while the note's bytes ride the html as an escaped<!--usfm:…-->comment inside anexcludespan. Newparatext9HtmlToUsfmportsStandard-Reverse.xsltover the DOM and wins overtext/plainwhenever the html carries P9's signature (ausfm:comment, orusfm_<name>+usfmopen/usfmclosed— nothing P10/Word/Docs writes).\figdid nothing — not intentional. Under structure protection the Standard-view claim declined every paste toStructureKeyboardPlugin's html sanitizer, whose carrier could not express a figure; the same decline also skipped the\c/\idstrip (a protected paste of\c 7created a second chapter node — the save-loop poisoning the strip exists to prevent) and the NBSP rule, and would have skipped the P9 decode. The engine has no protection gate for marker bytes (typed or pasted\v/\ptokenize identically in both modes), so the sanitizer only ever guarded DOM verse/para nodes that Standard view's html no longer produces. The claim now handles a protected document with the same bytes as an unprotected one, except it declines a selection$shouldBlockSelectionReplacementrefuses (one owner for that rule) and never replays paragraphs (lines join with a single space). Documented in S3.Pins:
decoratorBoundarySelection.test.tsx(11 — DOM untouched while the button is down, materialized on release, written immediately otherwise, survives an event-less update),DecoratorBoundarySelectionPlugin.test.tsx(2),paratext9Clipboard.utils.test.ts(18), html/plain parity inclipboardCopyFidelity, P9→P10 inmarkerPasteFidelity,protectedFigurePaste.test.tsx(12), the rewritten structure-protection describe inwhitespaceDisplay.plugin.utils.test.tsx(5; the Editor→plugin wiring pin stays falsifiable as a mode difference). Full suites: shared 697, shared-react 1670 (+1 skipped), platform-editor 1903 (+4 skipped); lint, prettier and per-package typecheck clean;verify-committed-dist✓.Known, deliberately left (pre-existing, reported to TJ): in Simple-mode Standard view a paste with the caret parked at the end of a
\vglyph is refused outright by$selectionContainsVerseMarker(a text-point caret inside the mutableVerseNodecounts as containment).Real-browser verification: paranext-core's
clipboard-usfm-round-tripisolated e2e, extended with the four drag/flavour/P9 steps, passes against diste91c930clinked into the app — 1 passed (2.4 min), first attempt.Code-review round (2026-09-17)
A
/code-reviewpass over both PRs, then triage. Three commits:24b030d2(review fixes),a0efe7df(periph),1b2163ac(paste behaviour).Paste no longer edits the host paragraph's marker — behaviour change, please read
Pasting a whole-paragraph copy at an existing paragraph's content start was deleting the host paragraph's marker. A whole-paragraph copy carries its own
\pliteral, so the line ends up with two paragraph markers, and$withoutRedundantOwnPrefixresolved that by dropping the host's glyph: paste\q1 asdfat\p ‖fdsaand you got one\q1 asdffdsaparagraph, with the\pgone and content that belonged to it now under\q1. An\s1heading pasted into came back as a plain\p.That was never a product decision — it was added to un-skip a round-trip test whose target document is an empty
\pscaffold, and the "stray empty paragraph" it removed was that scaffold. It is also not what Paratext 9 does:NormalizeTokenUsfm(ParatextData/UsfmToken.cs) emits a line break before every Paragraph token, so P9 gives you the empty\pand then the pasted paragraph. A paste now inserts what was pasted and nothing else, and if that means two paragraph markers in a row, that is what the bytes say — the same shape typing them produces.Removing it takes the whole paste-provenance apparatus with it: the
isPasteRebuildparameter through$buildParaFragment/$rebuildParas/$requestTier2ForNode,Tier2Context.pasteRebuildArmed, thepastePendedKeysset that carried "this rebuild came from a paste" across a deferred settle, its per-commit pruning, and consume-on-use. The read-only settle and the mutating rebuild now call$buildParaFragmentwith identical arguments, which also closes a review finding: the two could disagree about a deferred paste's shape, lettingEditorRef.getUsj()predict a paragraph the settle would not produce.It also exposed a real defect the dedup had been masking. A multi-line paste replays each line break as a paragraph split, and the engine injects a marker prefix onto every fresh split paragraph — including onto lines that already carry their own marker. For a marker with a separator and content the injected prefix resolves away; for
\b, which has neither, both survived, so every blank line in a pasted document gained an empty\pin front of it — 16 of them in the 2sa fixture.$paraMarkerDeletionTransform's split branch now skips the injection when the paragraph's own text already opens with a paragraph-marker literal. Enter pressed directly in front of a marker literal is the same shape and gets the same answer.The round-trip suites seeded that empty
\phost and expected the paste to swallow it; they now assert it is there and strip it, so a genuine stray paragraph can no longer pass as the harness's own.Correctness fixes (
24b030d2)isInternalanswered the mere presence ofapplication/x-lexical-editor, but Lexical's fast path is namespace-scoped ($insertDataTransferForRichTextrequirespayload.namespace === editor._config.namespace). A payload from the app's own"Commenting"editor (CommentPlugin.tsx) made every claim stand aside, then fell through to thetext/htmlDOM import — which runs neither the\c/\idstrip nor the NBSP rule, so a\ccopied out of a comment box reached the document unstripped.getPastePayloadnow mirrors Lexical's actual condition. (A sibling test already called this the "same-namespace-flavor guard", so the code was behind its own documented intent.)StructureKeyboardPluginmounts, so the one paste that handler declines reached it and was replayed withremoveText()plus a split per line. Measured with the guard removed: a protected paste deletes\v 18and the\ndspan and splits the paragraph — exactly what protection had just refused.LEADING_MARKER_LITERALaccepted a closer (inherited from the shared terminated-marker shape), so a leading\zbold*read as the paragraph markerzbold— unknown markers are paragraph-kind — and the host's glyph was dropped: an\s1heading became\p. This matched P9's own rule exactly once fixed:// End tokens are always end tokens, even if unknown(UsfmToken.cs). (Moot now that the dedup is gone, but the classification fix stands.)<p>— the same loss the collapsed-selection guard exists to prevent, reached through the non-collapsed door. The event is claimed and nothing is written.usfmFromCommentand became a hard line break, splitting the note across two paragraphs.DecoratorBoundarySelectionPluginpointer tracking: the press was heard on the root while the release was on the document, so a press landing just outside the contenteditable root started a drag the plugin never saw; and a secondary-button press could leave the flag stuck when the native context menu swallowed thepointerup. Both fixed.\periphkeeps its division when the attribute list is refused (a0efe7df)\periphhas no closing marker, so a peripheral division's extent is a property of the marker, not of its attributes. Refusing the marker line's attribute list was demoting the whole division to an ordinary paragraph and re-parenting every block under it to the root. P9 reads it the other way: a failedSetAttributesleaves the marker-line text alone and the USX writer still opens<periph>with an emptyid. The refused bytes now survive as the division's own title and serialize back byte-for-byte. This also removes a live-typing glitch — typing|id="cover"onto a periph line used to un-nest everything under it on the|keystroke.Splitting the marker line at its FIRST
|is unchanged: that matches P9's owntext.IndexOf('|').Test defects found and fixed
not.toContainagainst a flavor that is always""for that selection (the opaque-block rule suppresses it), so revertingUnknownNode.isSelectedleft it green. The override's range branch — its whole reason for existing — had no pin anywhere; two now live inUnknownNode.test.ts, verified to fail when the override is reverted.KNOWN_LOSSY, hence skipped, hence protecting nothing despite the file calling it "NOT a fix candidate". It is now pinned positively."{}"as the lexical flavor — never same-namespace — so they pinned the old presence-only rule.possibly undefinedtype errors in the protected-paste suite (present before this round) are fixed.Every behaviour fix above was verified by reverting it and watching the new test fail.
Verification
shared700,shared-react1670 (+1 skipped),platform-editor1914 (+3 skipped); per-package typecheck, eslint and prettier clean;verify-committed-dist✓.This change is