Conversation
`BlinkCursor::stop`, called from `on_blur`, resets the epoch but leaves `paused` and `visible` holding whatever the last `pause` set. A blur inside the 300ms pause window — tabbing away right after a keystroke — therefore leaves `paused == true` on a stopped cursor. The next `start` then hits `blink`'s `if self.paused` early return and never moves the epoch off zero, so the refocused input has a cursor that is not blinking. The defect is masked today, and masked by accident: `pause` acts on any cursor, blinking or not, so the next keystroke starts the loop again. That accident is itself a bug — it is what makes a programmatic `set_value` blink an unfocused input forever — and the next commit removes it. This one has to land first so that removing the mask does not turn a hidden defect into a visible one. Clear `paused` and `visible` alongside the epoch, so a stopped cursor carries nothing into the next focus. As a side effect the caret now appears the moment an input is focused, rather than starting from whatever `visible` the previous focus left behind. novakduc@arch
Author
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
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
BlinkCursor::stop, called fromon_blur, resets the epoch but leavespausedand
visibleholding whatever the lastpauseset. A blur inside the 300mspause window — tabbing away right after a keystroke — therefore leaves
paused == trueon a stopped cursor. The nextstartthen hitsblink'sif self.pausedearly return, never moves the epoch off zero, and therefocused input has a cursor that is not blinking.
The defect is in the tree today and is masked by accident:
pauseacts on anycursor, blinking or not, so the next keystroke starts the loop again. That
accident is itself the bug in #3138 — it is what makes a programmatic
set_valueblink an unfocused input forever. Fixing #3138 removes the mask, sothis wants to land first, or fixing one bug turns the other from hidden into
visible.
The fix
Clear
pausedandvisiblealongside the epoch, so a stopped cursor carriesnothing into the next focus.
stopis the end of a blink loop; leaving twoflags behind from the middle of the previous one has no meaning for the next.
As a side effect the caret now appears the moment an input is focused, rather
than starting from whatever
visiblethe previous focus happened to leavebehind.
Public API
No signature changes. One public function changes what it observes:
OtpState::cursor_visiblenow returnsfalseafter a blur, where the staleflags could previously keep it
true. Both call sites (gpui-component'sOtpInputandgpui-shell'sotp_input) already gate the caret on focus, sonothing changes on screen.
Verification
Automated
blurring_a_paused_cursor_leaves_the_next_focus_blinking: start, pause,stop, start — the caret must show immediately and hide one interval later.
Confirmed red on current
main, green with this change.cargo test -p gpui-base --lib(989) andcargo clippy -p gpui-base --lib --tests -- --deny warningspass at this commit;cargo fmt --check,typosand
cargo macheteare clean.Manual
Not done.
docs/ACCESSIBILITY-UI-TESTING.mdis the required method for focusand input behavior and it is macOS-only (
./script/run-story-macos, the macOSaccessibility tree); this branch was written on Linux. The pass worth running on
the Input story: type a character, Tab away and Shift-Tab straight back within
300ms, and confirm the caret appears immediately and keeps blinking.
AI assistance
Written with Claude Code: the change and its test are AI-generated, reviewed and
run by me.
novakduc@arch