Guard lexical's scrollIntoViewIfNeeded against Safari's bogus RTL caret rect - #1202
Guard lexical's scrollIntoViewIfNeeded against Safari's bogus RTL caret rect#1202jeremy wants to merge 2 commits into
Conversation
…et rect On Safari, a collapsed caret in RTL text yields a degenerate, out-of-bounds bounding rect. Lexical's scrollIntoViewIfNeeded feeds that rect straight to the scroller, so every space/backspace scrolls the window up and the editor jumps off-screen. Chrome/Edge are unaffected: they return correct caret rects, and a collapsed caret reports selection.type "Caret" there rather than Safari's "Range" — the latter is what routes Lexical into the scroll path (see Lexical's own "#1482" comment). A caret that lives inside the editor cannot lie entirely outside the editor's own box; when the rect says it does, the rect is unreliable, so skip the scroll. This injects that guard into the bundled lexical at build time via a small rollup transform (the Rails-engine build bundles lexical; the npm build externalizes it). The build fails loudly if lexical's anchor ever changes, so a version bump can't silently reintroduce the bug. Upstreamed to facebook/lexical (see #2495).
There was a problem hiding this comment.
Pull request overview
This PR adds a build-time Rollup transform to patch Lexical’s scrollIntoViewIfNeeded logic to avoid Safari’s bogus RTL caret bounding rect triggering repeated upward scrollBy calls (causing the editor to jump off-screen). The workaround is injected into the bundled Lexical dependency during the Rails-engine asset build and fails the build if the expected Lexical anchor text changes.
Changes:
- Add a Rollup transform plugin that injects a geometry guard into Lexical’s scroll code (dev/prod anchor variants).
- Fail the build loudly if the Lexical anchor can’t be found (to avoid silent regressions on Lexical bumps).
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09291f6e14
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…nter - Drop the lower clause from the injected guard (Codex P2). A caret below a scrollable overflow:auto editor root legitimately has top > rootRect.bottom until Lexical scrolls the root's scrollTop; suppressing that broke auto-scroll past the fold. Only the above-the-editor case is geometrically impossible for a real in-editor caret. Now matches the upstream one-sided guard in facebook/lexical#8848. - Match lexical module IDs with a separator-agnostic regex so the transform applies on Windows builds (backslash separators) too. - Reset the applied counter in buildStart so the buildEnd anchor assertion is per-build under rollup --watch / incremental rebuilds.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a979e368e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Problem
Typing Arabic (or any RTL script) in a Lexxy comment box on Safari makes the page jump upward on every space/backspace, scrolling the editor off-screen. Reported via a customer (BC on-call card 10101960010) with a screen recording; confirmed the page scrolls up ~270px per keystroke and the editor lands at the bottom edge. Chrome/Edge are unaffected.
Root cause
The jump comes from Lexical's own
scrollIntoViewIfNeeded(LexicalSelection.ts), not from Lexxy or bc3:selection.type === "Range"(Chrome/WebKit-headless report"Caret"). Lexical's$updateDOMSelectionhas a special case for exactly this — the!(domSelection.type === 'Range' && isCollapsed)term with the "Badly interpreted range selection when collapsed - #1482" comment. On Safari that term is false, so Lexical skips its early-return, re-applies the DOM selection, and runs the scroll block on every keystroke.getRangeAt(0).getBoundingClientRect(). Safari returns a degenerate/out-of-bounds rect (negative/zero top) for a collapsed caret in RTL text — worst at whitespace/line boundaries, i.e. after space/backspace.currentTop < targetTop→scrollBy(0, negative), fired twice per keystroke → the window jumps up and the editor scrolls away from the caret.Lexxy already knows Safari caret rects are unreliable —
Selection#isRectUnreliable+ the marker-span workaround in#getReliableRectFromRange— but that guards Lexxy's own rect reads; it can't reach the rect Lexical reads internally for the scroll.Fix
A caret that lives inside the editor cannot have a rect lying entirely outside the editor's own box. When it does, the rect is bogus, so skip the scroll. This is the browser-agnostic geometry guard a Lexical maintainer floated back in 2022 (facebook/lexical#2495, closed unmerged).
Because the defect is in Lexical (a dependency) and the Rails-engine build bundles Lexical (the npm build externalizes it), this injects the guard into the bundled
lexicalat build time via a small rolluptransformplugin. It patches both the readable dev variant and the minified prod variant, and fails the build loudly if Lexical's anchor ever changes — so a version bump can't silently reintroduce the bug. It mutates nothing innode_modulesand adds no runtime cost.I chose a build-time transform over
patch-packagebecause Lexical ships its prod bundle as a single minified line, which makes the generated patch ~270 KB of noise. The transform keeps the change small, readable, and self-documenting.Validation
Reproduced and validated in real WebKit (Playwright, matching the customer's Safari 26.5), against Lexical 0.44.0 (
main). The bug needs Safari's native RTL-keyboard input path, which automation can't drive, so I faithfully simulated the two documented Safari behaviors (collapsedtype === "Range"+ a bogus caret rect) — remove either and the jump disappears, matching the customer video.scrollBy-40,-40The guard is surgical: it suppresses the spurious scroll and still scrolls to reveal a genuinely off-screen caret (real in-bounds rect below the fold).
Upstream
The bug is unfixed on Lexical
mainand latest (0.47.0). I've opened an upstream PR tofacebook/lexicalwith the same guard as a unit test, and commented on #2495. Once a fixed Lexical release is adopted, this build-time transform can be dropped.Notes
No automated regression test here: the bug only manifests under Safari's native RTL input, which neither Chrome system tests nor Playwright-WebKit can drive. The build-time assertion guards against silent breakage on a Lexical bump; the runtime behavior is covered by the upstream unit test.