Skip to content

[lexical-history][lexical-utils][lexical-playground] Bug Fix: overlays, hit targets and shared state at the editor boundary - #9053

Open
LeSingh1 wants to merge 1 commit into
facebook:mainfrom
LeSingh1:consol/editor-boundary-ui
Open

[lexical-history][lexical-utils][lexical-playground] Bug Fix: overlays, hit targets and shared state at the editor boundary#9053
LeSingh1 wants to merge 1 commit into
facebook:mainfrom
LeSingh1:consol/editor-boundary-ui

Conversation

@LeSingh1

@LeSingh1 LeSingh1 commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

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.

  • [lexical-history] Bug Fix: undo/redo applies to the editor that changed with a shared HistoryState #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.

  • [lexical-playground] Bug Fix: triple click selects the paragraph before an embed block #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.

  • [lexical-playground] Bug Fix: keep the floating format toolbar aligned under CSS scale #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.

  • [lexical-utils] Bug Fix: positionNodeOnRange cleans up an invocation that lands after it was disposed of #9005positionNodeOnRange 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. #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 #8953, #8964, #8973, #9005, consolidated per the review feedback on
#9027 and #9035.

#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 (#6775) Chromium resolves to the boundary image and
Firefox to an <hr>. That made the existing #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.

@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown

@LeSingh1 is attempting to deploy a commit to the Meta Open Source Team on Vercel.

A member of the Team first needs to authorize it.

…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.
@LeSingh1
LeSingh1 force-pushed the consol/editor-boundary-ui branch from d8f7a8c to 3412685 Compare August 10, 2026 05:11
@LeSingh1 LeSingh1 changed the title [lexical-history][lexical-utils][lexical-extension][lexical-playground] Bug Fix: overlays, hit targets and shared state at the editor boundary [lexical-history][lexical-utils][lexical-playground] Bug Fix: overlays, hit targets and shared state at the editor boundary Aug 10, 2026
@LeSingh1

Copy link
Copy Markdown
Contributor Author

e2e was red on chromium, rich-text: HorizontalRule.spec.mjs › Clicking between consecutive block decorators creates selection (#6775).

The cause was #8967. $getHorizontalRuleBesideClick mapped the DOM caret offset onto the root element's raw childNodes, but those also contain the zero-size data-lexical-decorator-boundary images Lexical renders around block decorators. For two adjacent rules the same click therefore resolves to a different child in each browser — Chromium lands on the trailing boundary image (so nothing is selected), Firefox on an <hr> (node selection). #8967 had absorbed that by rewriting the existing #6775 assertion from a root range selection to a node selection, which then failed on Chromium.

I dropped #8967 rather than narrowing it. The caret offset in the blank space beside a block decorator is precisely what browsers disagree about — that is what #6775 / #8862 are about — so selecting the rule reliably needs a hit test against the rules' own geometry, which is a different change and belongs in its own PR. HorizontalRuleExtension.ts and HorizontalRule.spec.mjs are now identical to main.

The other four members (#8953, #8964, #8973, #9005) are unchanged. Verified locally: HorizontalRule.spec.mjs 14 passed on both chromium and firefox, BlockWithAlignableContents.spec.mjs 3 passed, unit 538 passed, tsc --noEmit -p . clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant