Keep focus out of the editor on outside selection changes (#1147) - #1210
Keep focus out of the editor on outside selection changes (#1147)#1210lyubomir-bozhinov wants to merge 2 commits into
Conversation
A caret between two stacked block images leaves a leading and trailing provisional paragraph in place. `$markAllProvisionalParagraphsDirty` ran on every `SELECTION_CHANGE_COMMAND`, so a selection change anywhere on the page marked those dirty and the reconcile wrote the editor's selection back to the DOM — pulling focus into the editor and away from wherever the user was typing. Firefox 152 fires `selectionchange` for an outside input's caret, which older Gecko and Blink do not, so the report only reproduced there: typing in a field above the editor snapped the caret back into Lexxy after the first keystroke. Guard the handler on `isEditorFocused`, mirroring the rewritable-history extension: a provisional paragraph's visibility follows the editor's own caret, so there is nothing to update — and no selection to restore — while the editor is unfocused. Fixes basecamp#1147
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR fixes a Firefox-specific focus-steal regression where selectionchange events originating outside the Lexxy editor could trigger a reconcile that restores the editor selection back into the DOM, pulling focus away from the user’s active input. The fix adds a focus guard to the provisional paragraph selection-change handler and introduces a Playwright regression test covering the reported scenario.
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.
Changes:
- Bail out of
$markAllProvisionalParagraphsDirtywhen the editor is not focused to avoid selection restoration/focus steal. - Add a new Playwright test reproducing the “caret between stacked images + outside input typing” scenario and a selection-change replay coverage path.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/extensions/provisional_paragraph_extension.js |
Adds an editor-focus guard to prevent selection-driven reconciles when the editor is unfocused. |
test/browser/tests/attachments/focus_between_stacked_images.test.js |
Adds a regression test that ensures outside-input focus is preserved when selection changes occur elsewhere on the page. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async function dispatchSelectionChange(page) { | ||
| const dispatched = await page.locator("lexxy-editor").evaluate((el) => { | ||
| const command = [ ...el.editor._commands.keys() ].find((candidate) => candidate?.type === "SELECTION_CHANGE_COMMAND") | ||
| if (!command) return false | ||
| el.editor.dispatchCommand(command, undefined) | ||
| return true | ||
| }) | ||
| expect(dispatched, "SELECTION_CHANGE_COMMAND must be resolvable for this regression to have teeth").toBe(true) | ||
| } |
There was a problem hiding this comment.
Good catch on the private internal — fixed in b4ce8a6. It now imports the real SELECTION_CHANGE_COMMAND through a small fixture module and dispatches that, so there's no more reliance on editor._commands or a dev-only .type.
I kept the direct command dispatch rather than a native selectionchange event, though. I checked, and dispatching selectionchange on document while focus is in the outside input does not reproduce this on the bundled Firefox (146) or on Chromium/WebKit: Lexical ignores a selectionchange whose selection sits outside its root, so no reconcile fires and the test would pass with or without the fix. Firefox 152 is the outlier that dispatches the command in that state — which is the whole bug. Replaying the command directly is the faithful stand-in for 152's behavior, and it keeps the regression failing-without-the-fix on all three engines in CI.
| const between = [ ...content.children ].find((child, index) => | ||
| child.classList.contains("provisional-paragraph") && content.children[index - 1]?.tagName === "FIGURE") | ||
| const range = document.createRange() | ||
| range.selectNodeContents(between) | ||
| range.collapse(true) |
There was a problem hiding this comment.
Agreed — added an explicit throw in b4ce8a6 so a changed DOM structure fails with a clear message instead of a cryptic error from inside evaluate.
| await placeCaretBetweenStackedImages(page) | ||
| }) | ||
|
|
||
| test("a selection change elsewhere on the page does not pull focus back into the editor", async ({ page, editor }) => { |
There was a problem hiding this comment.
Agreed — dropped the unused editor fixture from that test in b4ce8a6.
Dispatch the real exported SELECTION_CHANGE_COMMAND through a small fixture module instead of reaching into `editor._commands`, so the test no longer depends on a private Lexical internal. Replaying the command directly (rather than a native selectionchange event) is deliberate: Firefox below 152 ignores a selectionchange whose selection sits outside the editor, so the native event would make the regression vacuous on the bundled browser. Also throw a clear error when the provisional paragraph between the images is missing, and drop the unused `editor` fixture from the first test.
|
Closing this — #1209 already fixed it, and did it better. I only spotted the overlap now: cd72f36 landed on I confirmed the overlap rather than assuming it — running the stacked-images reproduction against Firefox 153:
Same browser both times, so #1209 is what resolves it. Thanks @jeremy — the write-up on cd72f36 was a genuinely useful read, particularly the detail that 151 and earlier move the document selection along with focus. That is exactly why this one resisted reproduction until I got a 152 build in front of it. #1204 and #1205 are unrelated and still reproduce on current |
Fixes #1147.
The bug
On a page with an input above the editor, uploading two images stacked vertically, typing between them, then focusing the outside input and typing — the caret snaps back into Lexxy after the first keystroke. Reported on Firefox; only reproduces on Firefox 152 (146 and 151 do not).
Root cause
A caret between two stacked block images leaves a leading and trailing
ProvisionalParagraphNodein place.ProvisionalParagraphExtensionmarks all provisional paragraphs dirty on everySELECTION_CHANGE_COMMANDso their caret-spacer visibility can follow the selection. That handler had no focus guard.Firefox 152 fires
selectionchangefor an outside input's caret (older Gecko and Blink do not). That dispatchesSELECTION_CHANGE_COMMAND, marks the provisional paragraphs dirty, and the resulting reconcile writes the editor's stored selection back to the DOM — pulling focus into the editor and away from the input.The fix
Bail out of
$markAllProvisionalParagraphsDirtywhen the editor isn't focused. A provisional paragraph's visibility follows the editor's own caret, so there is nothing to update — and no selection to restore — while the editor is unfocused. This mirrors the existingisEditorFocusedguard inrewritable_history_extension.js.One guard clause; no behavior change while the editor is focused.
Testing
firefox-betabuild): without the fix only the first outside keystroke lands before focus jumps back; with it, all keystrokes land and focus stays put.test/browser/tests/attachments/focus_between_stacked_images.test.js:cursor_moves_awayfailures are pre-existing and reproduce identically onmain.