Repository navigation
fix(composer): keep the caret after the first typed character - #1316
Conversation
Typing "any" could come out as "nya". After the browser inserts a typed character, ProseMirror reads it in a microtask. A focus event in that window ran the composer's focus restore, which wrote ProseMirror's caret from before the character back into the DOM; the read then adopted it, and the next keys landed in front of the character. The restore already skips focus that arrives during native editing, but the flag cleared in a microtask, before ProseMirror's read. It now clears when the task ends. Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Bradley Axen <baxen@squareup.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket (lifecycle/performance), Mantis (correctness/concurrency/security), and Groot (product/accessibility/focus) reviews.
No actionable findings. Source assessment: 9/10 for minimalness, elegance, and correctness.
Reviewed head c9c8e695916a3b28c97985356bdc94bce966a30a, target base 4c5f9e36cd89ea37ad6e854c1e7818145d2eca16, merge-base 7d8bafc83d523edd95b06e7bfa52697829ee0e94. The feature diff contains two files; dependency differences between the endpoint trees are base-branch advancement, not changes introduced by this PR.
EditableInput.tsx:1424–1434,1795–1811: keeping the guard until a later task covers the native insertion/read window without moving caret ownership or suppressing the host focus callback. The one-shot callback only clears a ref and does not touch a destroyed editor.composer-focus.spec.mjs:40–69: the added browser case injects focus during input capture, then checks text order and focus. Native editing/event ordering justifies this browser-level test. Existing ordinary-return and explicit range-restoration paths remain intact in source, including restored-draft/error recovery callers.- Public description and commit message reviewed; no attached images or actionable disclosure finding.
Validation: source-only; no local tests, app launches, or PR code executed. Hosted checks associated with this head show CI required and all executed checks passing; Windows native validation and Vitest flake check were skipped. Read-only hosted flake logs show the new case passing 10 repetitions in Chromium and 10 in WebKit. These results do not prove native window refocus behavior. The author’s reported base-failure reproduction was not independently rerun; native Cmd+Tab/IME, rendered accessibility/themes/viewports, timer-throttling behavior, and human acceptance remain unverified. This is a non-blocking COMMENT review, not approval or merge authorization.
Typing "any" sometimes came out as "nya": after the first character, the caret jumped back in front of it.
Cause
When the browser inserts a typed character, ProseMirror reads the change in a microtask. If a focus event reaches the composer inside that window,
onFocusrestores ProseMirror's own caret, which still predates the character:editor.focus().view.focus()defers the pending DOM records by 20ms (domObserver.stop()), then writes the stale caret withselectionToDOM.The deferred read then adopts that stale caret, and the next keys land in front of the character. Natively, this happens when typing is what refocuses the composer (for example, right after returning to the window).
#909 already guards the restore with
nativeEditing, which is set inbeforeinput. But the flag cleared in a microtask, which runs before the browser has even inserted the character. It now clears when the task ends, so the guard covers the whole window.An earlier attempt deferred the restore itself to a microtask instead. That reversed the race for a focus followed by an insertion in the same task, and
composer-links.speccaught it.Tests
composer-focus.speccase: typehello, have the host focus the editor during the next keystroke'sinputevent, then typeany. It expectshello any. On main it getshello nyain both Chromium and WebKit.composer-focusandcomposer-linkspass on both engines. The full browser suite is running and I'll post its results here.Human test
In the native app, focus a composer and type a short word right after switching back to the Buzz window (Cmd+Tab back, then type straight away). The word should come out in order.
🤖 Generated with Claude Code