fix(app): reset stale modifiers on focus loss, drop duplicate focus restore#27
Conversation
…estore Two regressions from the native-tab-close focus fix (#26): Search opening uninvited: closing a tab with cmd+w destroys the view before the Cmd-release flagsChanged reaches winit, so the app's stored modifiers stay stuck holding SUPER and the next plain keypress on the surviving tab resolves as a chord (plain f -> cmd+f -> Search/Find). Reset stored modifiers on Focused(false) and in the RestoreFocus handler; winit re-sends ModifiersChanged on the next real modifier press, so nothing is lost. Regression test locks the resolution semantics the reset relies on. Intermittent crash on the same gesture: the belt-and-suspenders block added to Focused(true) ran makeFirstResponder: plus an IME off/on toggle synchronously inside the winit handler on every focus gain, including mid-teardown ones — a re-entrancy/teardown race. Remove it; focus restore after tab close lives solely on the deferred RestoreFocus path, which keeps the first-responder + IME re-arm.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57361f4662
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // chord (a plain `f` → `cmd+f` → Search/Find). Clear on focus | ||
| // loss; winit re-sends `ModifiersChanged` on the next real | ||
| // modifier press, so nothing is lost. | ||
| self.modifiers = winit::keyboard::ModifiersState::empty(); |
There was a problem hiding this comment.
The code just below notes that macOS can deliver the outgoing tab/window's Focused(false) after the incoming one is already focused; clearing the global self.modifiers in that outgoing event drops still-held Cmd/Shift. If a user holds ⌘⇧ and presses ] repeatedly to cycle tabs, the first switch can succeed but the next KeyboardInput is resolved with empty modifiers (the state is only restored by a later modifier change), so the shortcut is ignored or sent to the PTY. Gate this reset to actual app deactivation or another stale-modifier recovery path rather than every focus loss.
Useful? React with 👍 / 👎.
1 participant
Summary
#26(タブ閉じ後入力不能の修正)の回帰2件を修正。
flagsChangedが破棄済みビューに落ちてwinitに届かず、保持modifiersがSUPERのまま固着。次の素のfがcmd+fと解決されSearch(Find)が開いていた。Focused(false)とRestoreFocusでmodifiersをリセット(次の実押下でwinitが再送するため副作用なし)。Focused(true)に足したbelt-and-suspendersブロックが、全フォーカス獲得時(teardown中含む)にwinitハンドラ内で同期makeFirstResponder:+IMEトグルを実行し再入/teardown競合の窓を作っていた。除去し、フォーカス復元は遅延RestoreFocus経路(firstResponder+IME再アーム維持)に一本化。※クラッシュはバックトレース未取得のため仮説ベースの対処。Test plan
cargo build/cargo clippy --workspaceクリーン、cargo test -p noa-app: 967 passed(新規回帰テストplain_f_with_reset_modifiers_does_not_resolve_to_find含む)f打鍵でSearchが開かないこと・入力が効くこと・タブ開閉連打でクラッシュしないこと