From af66f3d8494e8533e3277cec1911f69587f6669f Mon Sep 17 00:00:00 2001 From: Kayshen-X Date: Sat, 9 May 2026 21:54:00 +0800 Subject: [PATCH] =?UTF-8?q?fix(shell-core):=20Phase=20C=20stop-hook=20#2?= =?UTF-8?q?=20=E2=80=94=20skip=20dropdown=20keys=20during=20IME?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex stop-hook #2: with the hidden IME textarea now focused (R1 fix in fe994c4b), every keystroke routes through it — including arrow keys the user is pressing to navigate the IME's candidate picker. The window-level keydown listener still fires on these keystrokes, and `DropdownState::apply_key` was mutating selection + opening the menu behind the IME panel. Surfaced as: "focused IME textarea lets composing keys mutate dropdown state." Spec §2.4 says: "Widgets that consume keys directly should usually skip dispatch when is_composing == true and let the ImeEvent path handle the composition instead." The plumbing for `is_composing` already runs through Phase C2.2 (W3C `KeyboardEvent.isComposing` → C1 `map_keyboard_parts(..., is_composing)` → `KeyEvent.is_composing` → C2.1 `DropdownState::apply_key`); we just weren't honoring the bit on the consuming side. Fix: add `event.is_composing` to the early-return condition in `DropdownState::apply_key`. ArrowDown/Up/Enter/Escape during a composition no-op now; the IME's candidate picker keeps the keystroke and the dropdown stays put. `TextInputState::apply_ime` is unaffected — it already only processes ImeEvent, never KeyEvent, so composing-key bleed- through was never a concern there. Test: `dropdown_apply_key_ignores_keys_during_ime_composition` asserts both ArrowDown (would advance + open) and Enter (would close) are no-ops when `is_composing` is true. Test count 20 → 21. Verification: - `cargo test -p openpencil-shell-core --test widgets_static` — 21/21 passing - `cargo check -p openpencil-shell-core --target wasm32-unknown-unknown` — green - `cargo build -p openpencil-shell-web --target wasm32-unknown-unknown --features skia --release` — green - `bash tools/check-wasm-bundle.sh` — PASS: - 0 env.* imports - 622 156 bytes gzip = 59% of 1 MiB ceiling Phase D may extend this guard pattern to other widgets that gain key handling (Tree typeahead, etc.); the spec §2.4 is_composing contract becomes a per-widget invariant. --- .../src/widgets/dropdown.rs | 22 ++++++++++--- .../tests/widgets_static.rs | 33 +++++++++++++++++++ 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/crates/openpencil-shell-core/src/widgets/dropdown.rs b/crates/openpencil-shell-core/src/widgets/dropdown.rs index 699d55aa7..8f37f946b 100644 --- a/crates/openpencil-shell-core/src/widgets/dropdown.rs +++ b/crates/openpencil-shell-core/src/widgets/dropdown.rs @@ -20,12 +20,24 @@ impl DropdownState { /// stays a pure state mutator without borrowing the Dropdown /// struct around it. /// - /// Skips when the key is released or the dropdown has no options - /// (no-op rather than wrap-around / panic). Non-navigation keys - /// fall through unchanged so widget code can layer additional - /// handlers later (e.g. typeahead in Phase D+). + /// Skips when the key is released, the dropdown has no options, + /// OR an IME composition is in progress (`event.is_composing == + /// true`). The IME-skip is required by spec §2.4: while the + /// user is composing CJK / dead-key sequences, arrow keys belong + /// to the IME's candidate-picker UI, NOT to widgets that read + /// keys directly. Without this guard, every arrow keystroke + /// during a composition would cycle the dropdown selection + /// behind the IME panel — surfaced by the codex Phase C + /// stop-hook ("focused IME textarea lets composing keys mutate + /// dropdown state"). + /// + /// Non-navigation keys fall through unchanged so widget code can + /// layer additional handlers later (e.g. typeahead in Phase D+). pub fn apply_key(&mut self, event: &crate::KeyEvent, option_count: usize) { - if event.state != crate::KeyState::Pressed || option_count == 0 { + if event.state != crate::KeyState::Pressed + || option_count == 0 + || event.is_composing + { return; } match event.key { diff --git a/crates/openpencil-shell-core/tests/widgets_static.rs b/crates/openpencil-shell-core/tests/widgets_static.rs index 2db480c99..8b038fdc5 100644 --- a/crates/openpencil-shell-core/tests/widgets_static.rs +++ b/crates/openpencil-shell-core/tests/widgets_static.rs @@ -442,3 +442,36 @@ fn dropdown_apply_key_ignores_unrelated_keys() { assert_eq!(state.selected, 1); assert!(state.open); } + +#[test] +fn dropdown_apply_key_ignores_keys_during_ime_composition() { + // Spec §2.4 widget rule: skip key dispatch while is_composing is + // true. ArrowDown during a CJK composition belongs to the IME's + // candidate-picker UI, NOT to the dropdown selection cycle. + // Codex Phase C stop-hook (#2): "focused IME textarea lets + // composing keys mutate dropdown state." + let mut state = DropdownState { + selected: 0, + open: false, + }; + let composing_arrow = KeyEvent { + is_composing: true, + ..keydown(NamedKey::ArrowDown) + }; + state.apply_key(&composing_arrow, 3); + assert_eq!(state.selected, 0, "composing ArrowDown must not advance"); + assert!(!state.open, "composing ArrowDown must not open"); + + // Same protection for Enter / Escape — IME 'commit' shouldn't + // close the dropdown. + let mut state2 = DropdownState { + selected: 1, + open: true, + }; + let composing_enter = KeyEvent { + is_composing: true, + ..keydown(NamedKey::Enter) + }; + state2.apply_key(&composing_enter, 3); + assert!(state2.open, "composing Enter must not close"); +}