Skip to content

input: Do not start the blink cursor on an unfocused input - #3140

Merged
huacnlee merged 3 commits into
longbridge:mainfrom
novakduc:input-unfocused-blink-cursor
Sep 20, 2026
Merged

huacnlee merged 3 commits into
longbridge:mainfrom
novakduc:input-unfocused-blink-cursor

Conversation

@novakduc

Copy link
Copy Markdown
Contributor

Fixes #3138. Depends on #3139 — please merge that one first.

This branch is stacked on #3139, so its commit still shows here until that PR
lands; afterwards this diff is just the pause guard and its tests. #3139 is a
one-line-each reset in stop and is not a regression on its own. This PR is,
if it merges alone: see "Why the order is forced" below.

Summary

InputState::set_value pauses the blink cursor whether or not the input is
focused, and a pause resumes blinking 300ms later. On an input that was never
focused, or that was blurred, the pause is what starts the blink loop — and
nothing ever stops it, because the blur that would call stop has already
happened.

Every blink calls cx.notify(), and InputState observes its own cursor, so
each blink repaints the whole view the input sits in. One programmatic
set_value on an off-screen input costs two full view repaints per second for
the life of the window, and a pane that seeds twenty inputs pays it twenty
times over. Nothing appears on screen while it happens: the input is not
focused, so it draws no cursor.

The chain

InputState::set_value        → replace_text → replace_text_in_range_silent
  → replace_text_in_range    → self.pause_blink_cursor(cx)   ← no focus check
    → BlinkCursor::pause                                     ← no focus check

Starting from a cursor with epoch == 0:

t what runs effect
0ms pause paused, visible, notify, epoch 0 → 1, timer 300ms
300ms resume → blink(1) 1 == self.epoch, so it proceeds: notify, timer 500ms
800ms blink(2) notify, timer 500ms
two notifies a second, indefinitely

The fix

BlinkCursor already uses epoch == 0 as its "not blinking" sentinel: new
starts there, stop (from on_blur) resets to it, and start (from
on_focus) moves off it. pause now honours that sentinel and returns early,
so only a cursor that is already blinking can be paused. A focused input is
unaffected — on_focus calls start, which leaves the epoch at 1 or more, and
there is no window in which a focused input has a zero epoch.

The guard is in pause rather than at the call site because
pause_blink_cursor has ten production callers across state.rs and
movement.rs, plus two direct BlinkCursor::pause calls; guarding one entry
point would leave the rest. It is also what the keystroke path already does —
intercept_keystrokes wraps its pause in focus_handle.is_focused(window),
so an unfocused pause is already treated as wrong there. The programmatic path
just was not covered.

Why the order is forced

stop currently leaves paused set (#3139). That defect is invisible today
only because pause acts on any cursor, so the next keystroke restarts the
loop — the very thing this PR removes. Merged before #3139, this guard would
turn that hidden defect into a caret that never blinks after a refocus:
blurring_a_paused_cursor_leaves_the_next_focus_blinking, added in #3139,
fails against this commit without it.

Public API

None. BlinkCursor and its pause are pub(crate). (#3139 carries the one
observable change, in OtpState::cursor_visible.)

Verification

Automated

  • Added pausing_a_cursor_that_is_not_blinking_does_not_start_it, the reported
    bug: a never-started cursor is paused, and must send no notification over the
    next three seconds. Red without the guard (six notifies), green with it.
  • Added test_set_value_on_unfocused_input_stays_quiet, which drives a real
    InputState through a window: after set_value settles, advancing the clock
    three seconds must not notify again. Red without the guard.
  • Confirmed the two halves separately: with input: Clear the blink state when the cursor stops #3139's stop reset alone, both
    tests above still fail. stop runs only from on_blur, and the input in
    BlinkCursor::pause starts a permanent blink loop on an unfocused input #3138 was never focused, so nothing on that path reaches it. This guard is
    what fixes the filed bug.
  • cargo test --workspace --exclude gpui-shell --features gpui-component-story/test-support --locked: 106 test binaries, 0 failures.
  • cargo clippy --workspace --exclude gpui-shell --locked -- --deny warnings,
    cargo fmt --check, typos, cargo machete, python3 script/check-ai rust
    and cargo test -p gpui-base -p gpui-component --doc: all clean.
  • The webview-dependent crates (webview, gpui-wry) and the two wasm crates
    were excluded locally for want of webkit2gtk-4.1; CI covers them.

Three existing tests changed

  • repeated_pauses_keep_cursor_visible_until_idle paused a cursor that was
    never started. It asserts the focused typing behaviour, so it now calls
    start first and makes that explicit. (Flagged in BlinkCursor::pause starts a permanent blink loop on an unfocused input #3138 as the one expected
    casualty; there turned out to be two more.)
  • test_multi_cursor_actions_reveal_hidden_carets faked the hidden blink phase
    with a fresh BlinkCursor. It now starts one and advances the clock into the
    hidden phase, which is the state it meant to describe.
  • test_cursor_layout_consumer_updates_after_selection was passing because of
    this bug: the stray blink loop supplied the frame in which its consumer read
    the freshly painted caret geometry. A notify sent from inside a draw marks the
    view dirty without asking for another frame — invalidate_view sets dirty
    and wakes the platform only when draw_phase == DrawPhase::None — so the test
    now invalidates the input explicitly before reading the baseline. The
    assertion it exists for, that a caret move reaches render consumers, is
    unchanged.

Manual

Not done. docs/ACCESSIBILITY-UI-TESTING.md is the required method for focus
and input behavior and it is macOS-only (./script/run-story-macos, the macOS
accessibility tree); this branch was written on Linux. The pass worth running on
the Input story is the one #3139 covers: 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 guard, the new tests and the changes to the three
existing tests are all AI-generated, reviewed and run by me. The bug itself was
diagnosed downstream from CPU sampling, not by the model.

novakduc@arch

`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
`InputState::set_value` pauses the blink cursor whether or not the input
is focused, and a pause resumes blinking 300ms later. On an input that
was never focused, or that was blurred, the pause is what *starts* the
blink loop, and nothing ever stops it: the blur that would call `stop`
has already happened.

Every blink calls `cx.notify()`, and `InputState` observes its own
cursor, so each blink repaints the whole view the input sits in. One
programmatic `set_value` on an off-screen input therefore costs two full
view repaints per second for the life of the window, and a pane that
seeds twenty inputs pays it twenty times over. Nothing appears on
screen: the input is not focused, so it draws no cursor.

### The fix

`BlinkCursor` already uses `epoch == 0` as its "not blinking" sentinel —
`new` starts there, `stop` resets to it, and `start` moves off it.
`pause` now honours that sentinel and returns early, so only a cursor
that is already blinking can be paused. A focused input is unaffected:
`on_focus` calls `start`, which leaves the epoch at 1 or more, and there
is no window in which a focused input has a zero epoch.

The guard is in `pause` rather than at the call site because
`pause_blink_cursor` has ten production callers across `state.rs` and
`movement.rs`, plus two direct `BlinkCursor::pause` calls; guarding one
entry point would leave the rest. It also matches the keystroke path,
which already wraps its `pause` in `focus_handle.is_focused(window)`.

This is also what stops masking the stale-pause defect fixed in the
previous commit, which is why that one comes first.

### Verification

- `cargo test -p gpui-base --lib` (991) and `cargo test -p
  gpui-component --lib` (547) pass; clippy and `cargo fmt --check` are
  clean at both commits.
- The two new tests were confirmed red without the guard and green with
  it.
- `repeated_pauses_keep_cursor_visible_until_idle` now starts the cursor
  before pausing it: it asserts the focused typing behaviour, and a
  never-started cursor no longer stands in for a focused one.
- `test_multi_cursor_actions_reveal_hidden_carets` faked the hidden
  blink phase with a fresh `BlinkCursor`; it now starts one and advances
  the clock into the hidden phase instead.
- `test_cursor_layout_consumer_updates_after_selection` was relying on
  the stray blink loop for the frame in which its consumer read the
  caret geometry. A notify sent from inside a draw marks the view dirty
  without asking for another frame, so the test now invalidates the
  input explicitly.

novakduc@arch
@huacnlee
huacnlee enabled auto-merge (squash) September 20, 2026 12:47
@huacnlee

Copy link
Copy Markdown
Member

Thank you.

@huacnlee
huacnlee merged commit becbcf2 into longbridge:main Sep 20, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BlinkCursor::pause starts a permanent blink loop on an unfocused input

2 participants