fix: place auto-close caret synchronously on plain contenteditable#363
Merged
bartekplus merged 2 commits intoMay 28, 2026
Merged
Conversation
The "#test-contenteditable" auto-close test typed "x" immediately after the bracket auto-close, racing the content script's deferred caret reposition (rAF + setTimeout) which moves the caret between the brackets for contenteditable targets. On fast CI the content poll matched "hello ()" before the reposition fired, so "x" landed after ")" giving a stable "hello ()x" that never matched /^hello \(x\)$/ -> 5000ms timeout. Mirror the already-stable Lexical test by waiting for the deferred reposition before typing. Plain #test-input is unaffected (synchronous caret), so it is left unchanged. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Supersedes the test-only sleep(200) deflake with a product fix for the
underlying race.
After auto-closing a bracket, the caret must move between the brackets.
For contenteditable this was done only via a deferred (rAF + setTimeout)
bridge event, because React hosts (Lexical/Slate) reconcile asynchronously
and override a synchronously-set caret. But the deferral was applied to
ALL contenteditable, so a fast follow-up keystroke on a plain
contenteditable could land before the reposition ran -> "hello ()x"
instead of "hello (x)". This affected real fast/paste typing, not just the
e2e test, and surfaced as a flaky CI timeout.
Plain contenteditable applied via the DOM path ("fallback-dom") has no
async reconciliation, so place the caret at the final offset synchronously
in the adapter's execCommand branch and skip the deferred relative
move-back for that path. Host editors (beforeinput / host-session) are
unchanged and keep their deferred repositioning.
The contenteditable e2e test no longer needs a settle delay.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
bartekplus
changed the title
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
1 participant
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.
Summary
Fixes the intermittent Tests → E2E Full failure where
Grammar Rule Engine auto-closes brackets in #test-contenteditabletimed out (to match /^hello \(x\)$/ after 5000ms). The flaky test was a symptom of a real product race, so this fixes the product rather than just padding the test with a sleep.Root cause
After auto-closing a bracket, the caret must move between the brackets. For
contenteditablethat was done only via a deferred bridge event (requestAnimationFrame+setTimeout(30ms)→Selection.modify()in the main world). That deferral exists because React hosts (Lexical/Slate) reconcile asynchronously and override a synchronously-set caret — but it was applied to all contenteditable, including plain ones.So a fast follow-up keystroke on a plain contenteditable could land before the reposition ran →
hello ()xinstead ofhello (x). This is a genuine fast-typing / paste glitch for real users (worse under main-thread load), not merely a test artifact; in CI it surfaced as the 5000ms timeout.Plain
<input>/<textarea>were unaffected (caret set synchronously viasetSelectionRange), which is why only the contenteditable test flaked.Fix
Scoped strictly to the plain-DOM apply path (
appliedBy === "fallback-dom"):ContentEditableAdapter: theexecCommandbranch returned beforesetCaret, leaving the caret after). Now it places the caret at the final offset synchronously (matching the pure-DOM path).SuggestionTextEditService: skip the deferred relative move-back forfallback-dom(the caret is already correct; a relative move-back would double-correct it).Host editors (React via
beforeinput=host-beforeinput; CKEditor via host session = noappliedBy) are untouched and keep their deferred repositioning.autoBracketCloseis the only rule that setscursorOffset, so the blast radius is exactly the auto-close-on-plain-CE case.The contenteditable e2e test no longer needs a settle delay (the earlier
sleep(200)is removed; the Lexical test keeps its own, since React stays deferred).Verification
#test-contenteditabletest: 8/8 runs pass (was racy)bun run lint,bun run typecheck,prettier --check: all cleanTest plan
Testsworkflow green on both browsersE2E Fullstays green across repeated runs🤖 Generated with Claude Code