Toggling a code block crashes when the selection spans nested structures#1110
Open
jorgemanrubia wants to merge 1 commit into
Open
Toggling a code block crashes when the selection spans nested structures#1110jorgemanrubia wants to merge 1 commit into
jorgemanrubia wants to merge 1 commit into
Conversation
jorgemanrubia
force-pushed
the
sentry-lexical-error-19
branch
from
222934e to
f9e4294
Compare
Toggling a code block with a selection spanning nested structures (a quote and its inner paragraphs, or nested list items) collected both an element and its ancestor. The new code node was inserted after the last collected element — inside the ancestor's subtree — and converting the ancestor removed it, leaving the selection on detached nodes. Lexical then threw invariant #19 ("selection has been lost") and rolled back the update, so the button click did nothing. Filter the collected block elements down to the outermost ones: their text content already covers their descendants, and the code node inserted after the last of them can no longer sit inside another element being converted.
jorgemanrubia
force-pushed
the
sentry-lexical-error-19
branch
from
f9e4294 to
92ece54
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes BC3-JS-N7D7 — 562 events / 470 users since May 26 (153 / 149 in the last 3 days). Card: https://app.basecamp.com/2914079/buckets/41746046/card_tables/cards/9982983094
What it fixes
Clicking the code toolbar button with the selection spanning nested structures — a quote and its inner paragraphs, or nested list items — crashed with Lexical invariant #19 ("updateEditor: selection has been lost because the previously selected nodes have been removed and selection wasn't moved to another node"). Lexical rolls the update back, so the click silently did nothing. Every Sentry event goes through the same path: toolbar
dispatchButtonCommand→editor.update→ invariant thrown at commit.How
Contents#toggleCodeBlockcollects the nearest block element for every selected node, inserts the new code node after the last collected element, and hands all of them toCodeNodeInserter, which converts each to code text vianode.remove()+$createTextNode(node.getTextContent()).With nested structures the collected list contains both an element and its ancestor (e.g.
[paragraph, quote, paragraphInsideQuote]). The code node gets inserted after the last element — inside the ancestor's subtree — so converting the ancestor detaches the code node along with it. The subsequent caret inserts andselectEnd()land in a detached tree; garbage collection purges those keys, and the pending selection fails Lexical's node-map check.The fix filters the collected elements down to the outermost ones before picking the insertion point. Their
getTextContent()already covers their descendants (the inserter already skips detached descendants), so the converted output is unchanged — the code node just can no longer be created inside an element that's about to be removed.Failing-first regression tests at both layers, each verified to fail on
main’scontents.jsand pass with the fix: Playwright tests through the real toolbar button (test/browser/tests/formatting/code_block_navigation.test.js, "Code block conversion") and unit tests via the toolbar’s dispatch path (test/javascript/unit/editor/toggle_code_block.test.js, helpers promoted toeditor_helper.js).Overlap notes
Self-contained in
contents.js#toggleCodeBlock; no overlap with #1106/#1107/#1108/#1109 diffs (different crash paths: arrow-nav race, drop/paste guards, paste invariant #66, paste invariants #211/#212).While reproducing, a brute-force matrix also surfaced a separate non-#19 crash in the same command: a selection covering a horizontal divider throws "Expected node to have closest block element node" from
#blockLevelElementsInSelection. Different error signature, different Sentry issue — left out of scope here.