[lexical-history] Bug Fix: undo/redo applies to the editor that changed with a shared HistoryState - #8953
Closed
LeSingh1 wants to merge 1 commit into
Closed
[lexical-history] Bug Fix: undo/redo applies to the editor that changed with a shared HistoryState#8953LeSingh1 wants to merge 1 commit into
LeSingh1 wants to merge 1 commit into
Conversation
…ed with a shared HistoryState A shared HistoryState interleaves entries from several editors, but a push always recorded `historyState.current` — the state of whichever editor changed last. When the next change came from a different editor the stack recorded a state that had not changed, so undo restored an editor to the state it was already in and the change that should have been undone was dropped from the history entirely. Record the pre-update state of the editor that actually changed, and take the inverse entry from that entry's own editor rather than from `current`, so undo and redo stay symmetric. An empty EditorState is skipped since it cannot be restored, which mirrors the way an editor's first update is not undoable. Fixes facebook#8623
LeSingh1
requested review from
acywatson,
etrepum,
fantactuka,
ivailop7,
potatowagon and
zurfyx
as code owners
August 7, 2026 10:08
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
LeSingh1
added a commit
to LeSingh1/lexical
that referenced
this pull request
Aug 10, 2026
…s, hit targets and shared state at the editor boundary ## Description Four fixes for things that sit at the *edge* of an editor: a `HistoryState` shared between two editors, overlays positioned over the editor DOM, and the hit target of a block decorator. Each mis-handles a boundary — undo applied to the wrong editor, an overlay whose position ignores an ancestor's CSS scale or whose async callback lands after disposal, and a triple click that stops short of an unselectable block. To be upfront: this group is smaller-grained than the other consolidations. It collects the fixes that did not belong to any of the larger defect classes, so the shared thread is the editor boundary rather than a single root cause. The four are independent — each bullet below can be reviewed and checked off on its own. - **facebook#8953 — undo/redo applies to the editor that changed** (`getUndoEntry` / `getInverseEntry`, `packages/lexical-history/src/index.ts`). A shared `HistoryState` interleaves entries from several editors, but a push always recorded `historyState.current` — the state of whichever editor changed *last*. A change from a different editor therefore recorded a state that had not changed, so undo restored an editor to the state it was already in and the real change was dropped from the history. Now the pre-update state of the editor that actually changed is recorded, and the inverse entry is taken from that entry's own editor rather than from `current`, so undo and redo stay symmetric. An empty `EditorState` is skipped because it cannot be restored, mirroring the way an editor's first update is not undoable. - **facebook#8964 — triple click selects the paragraph before an embed block** (`.PlaygroundEditorTheme__embedBlock`, `packages/lexical-playground/src/themes/PlaygroundEditorTheme.css`). The theme class set `user-select: none` on the decorator wrapper itself, and a browser will not extend a triple click past an unselectable block, so triple clicking a paragraph immediately followed by an embed collapsed the selection to the start of that paragraph. `user-select` is inherited, so scoping the rule to the wrapper's children keeps the embed contents unselectable while leaving the decorator host and the wrapper selectable. - **facebook#8973 — the floating format toolbar stays aligned under CSS scale** (`getElementScale` and `setFloatingElemPosition`, `packages/lexical-playground/src/utils/`). `setFloatingElemPosition` mixed two coordinate spaces: the rects it reads are on-screen pixels, but the `translate()` it writes back is resolved in the anchor's own — unscaled — space. Under a CSS `scale()` on any ancestor the two differ by the scale factor, so the toolbar was placed at the wrong offset from the selection and the 10px/5px gaps were applied at the wrong size. The gaps are now scaled into on-screen pixels for the arithmetic and the final offset is divided back into the anchor's space. At scale 1 both factors are 1, so the computation is unchanged. - **facebook#9005 — `positionNodeOnRange` cleans up an invocation that lands after it was disposed of** (`packages/lexical-utils/src/positionNodeOnRange.ts`). `restart` was registered as a root listener but returned nothing, so Lexical was given no way to undo what an invocation did — even though `restart()` prepends a wrapper element and starts a `MutationObserver`. Root listeners are triggered from a snapshot of the registry, so a listener that an earlier listener in the same pass unregistered is still invoked once more; Lexical's escape hatch is to call the cleanup that invocation returned, which did not exist, leaving the wrapper in the document with nothing left to remove it. This is reachable straight from `selectionAlwaysOnDisplay`: change the root element while the selection is being marked and a stray highlight overlay is welded to the container, one more per root change. The root listener now returns `stop`, which is what the `RootListener` contract is for. No API or serialization change in any of the four. ## Test plan New unit tests in `@lexical/history` (shared `HistoryState` across two editors) and `@lexical/utils` (both the direct `positionNodeOnRange` contract and the `selectionAlwaysOnDisplay` symptom, each counting attached overlays), plus a new browser-project test that asserts the floating toolbar's on-screen position at scale 1 (the control), 0.5 and 2. facebook#8964 is covered by `BlockWithAlignableContents.spec.mjs`. One existing test changed shape: `SharedHistoryExtension > can create a parent editor` dispatched `UNDO_COMMAND` twice in a row to revert a single child-editor edit, because the buggy push recorded a no-op entry that had to be undone first. It now dispatches once. Its assertions are unchanged. ### Before Source fixes reverted, new tests kept: ``` $ npx vitest run --project unit packages/lexical-history packages/lexical-utils × undo and redo apply to the editor that changed 7ms × can create a parent editor 182ms × removes the overlay of an invocation that lands after it was disposed of 72ms × leaves no overlay behind when the root element changed while marking 15ms ⎯⎯⎯⎯⎯⎯⎯ Failed Tests 4 ⎯⎯⎯⎯⎯⎯⎯ AssertionError: expected LexicalEditor{ …(34) } to be LexicalEditor{ …(34) } // Object.is equality AssertionError: expected 'Child editor. Updated!' to deeply equal 'Child editor' AssertionError: expected 2 to be 1 // Object.is equality AssertionError: expected 2 to be 1 // Object.is equality Test Files 2 failed | 17 passed (19) Tests 4 failed | 251 passed (255) $ npx vitest run --project browser packages/lexical-playground/src/__tests__/browser/FloatingElemScale.test.ts × setFloatingElemPosition places the toolbar above the target at scale 0.5 84ms × setFloatingElemPosition places the toolbar above the target at scale 2 77ms ⎯⎯⎯⎯⎯⎯⎯ Failed Tests 2 ⎯⎯⎯⎯⎯⎯⎯ AssertionError: expected 80 to be close to 145, received difference is 65, but expected 0.05 AssertionError: expected 1100 to be close to 580, received difference is 520, but expected 0.05 Test Files 1 failed (1) Tests 2 failed | 1 passed (3) ``` The scale-1 control passes before and after, as intended. ### After ``` $ npx vitest run --project unit packages/lexical-history packages/lexical-utils packages/lexical-playground Test Files 39 passed (39) Tests 538 passed (538) $ npx vitest run --project browser packages/lexical-playground/src/__tests__/browser/FloatingElemScale.test.ts Test Files 1 passed (1) Tests 3 passed (3) $ npx playwright test --project=chromium HorizontalRule.spec.mjs BlockWithAlignableContents.spec.mjs 17 passed $ npx playwright test --project=firefox HorizontalRule.spec.mjs 14 passed $ npx tsc --noEmit -p . (clean) ``` Supersedes facebook#8953, facebook#8964, facebook#8973, facebook#9005, consolidated per the review feedback on facebook#9027 and facebook#9035. facebook#8967 was part of an earlier revision of this PR and has been dropped. It selected a horizontal rule when a click landed in the blank space beside it, by mapping the DOM caret offset onto the root element's raw child nodes. Those children also include the zero-size `data-lexical-decorator-boundary` images Lexical renders around block decorators, so the same click maps to a different child in different browsers: for the two-adjacent-rules case in `HorizontalRule.spec.mjs` (facebook#6775) Chromium resolves to the boundary image and Firefox to an `<hr>`. That made the existing facebook#6775 assertion browser-dependent. Getting this right needs a hit test against the rules' own geometry rather than a caret offset, which is a different change and belongs in its own PR.
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
2 participants
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.
Noting up front that you commented on the issue that this is working as designed. I'm sending it because the symptom is not just awkward semantics — history entries are lost — but close it if you'd rather redesign the entry model instead.
A shared
HistoryStateinterleaves entries from several editors, but a push always recordedhistoryState.current— the state of whichever editor changed last. When the next change came from a different editor, the stack recorded a state that had not changed, so undo restored an editor to the state it was already in and the change that should have been undone was dropped from the history entirely. With two editors, editing A then B and pressing undo does nothing, empties the stack, and leaves both edits permanently unrecoverable.This records the pre-update state of the editor that actually changed, and takes the inverse entry from that entry's own editor rather than from
current, so undo and redo stay symmetric. An emptyEditorStateis skipped because it cannot be restored, mirroring the way an editor's first update is not undoable.One existing test changed:
SharedHistoryExtension > can create a parent editorpreviously needed two undos to revert one child edit. The extra press was the no-op entry described above, so it now needs one.Fixes #8623