diff --git a/crates/openpencil-shell-web/src/event/keyboard.rs b/crates/openpencil-shell-web/src/event/keyboard.rs index be29c0954..35a32d5bb 100644 --- a/crates/openpencil-shell-web/src/event/keyboard.rs +++ b/crates/openpencil-shell-web/src/event/keyboard.rs @@ -56,6 +56,34 @@ fn map_key_value(key: &str) -> KeyValue { "ArrowDown" => KeyValue::Named(NamedKey::ArrowDown), "ArrowLeft" => KeyValue::Named(NamedKey::ArrowLeft), "ArrowRight" => KeyValue::Named(NamedKey::ArrowRight), + // Navigation extensions (codex Phase C gate CONCERN): + "Home" => KeyValue::Named(NamedKey::Home), + "End" => KeyValue::Named(NamedKey::End), + "PageUp" => KeyValue::Named(NamedKey::PageUp), + "PageDown" => KeyValue::Named(NamedKey::PageDown), + // Function keys F1-F12. jian's NamedKey covers them but + // KeyCode does not — `map_key_code` falls through to + // Unknown(code) for these on purpose. + "F1" => KeyValue::Named(NamedKey::F1), + "F2" => KeyValue::Named(NamedKey::F2), + "F3" => KeyValue::Named(NamedKey::F3), + "F4" => KeyValue::Named(NamedKey::F4), + "F5" => KeyValue::Named(NamedKey::F5), + "F6" => KeyValue::Named(NamedKey::F6), + "F7" => KeyValue::Named(NamedKey::F7), + "F8" => KeyValue::Named(NamedKey::F8), + "F9" => KeyValue::Named(NamedKey::F9), + "F10" => KeyValue::Named(NamedKey::F10), + "F11" => KeyValue::Named(NamedKey::F11), + "F12" => KeyValue::Named(NamedKey::F12), + // Modifier keys themselves (W3C `key` is just "Shift" etc; + // `code` is "ShiftLeft" / "ShiftRight" — handled in + // `map_key_code`). + "Shift" => KeyValue::Named(NamedKey::Shift), + "Control" => KeyValue::Named(NamedKey::Control), + "Alt" => KeyValue::Named(NamedKey::Alt), + "Meta" => KeyValue::Named(NamedKey::Meta), + "CapsLock" => KeyValue::Named(NamedKey::CapsLock), _ => { // `KeyValue::Char(char)` covers single-codepoint keys // (e.g. "a", "1", "你"). Anything else (multi-codepoint @@ -118,6 +146,26 @@ fn map_key_code(code: &str) -> KeyCode { "ArrowDown" => KeyCode::ArrowDown, "ArrowLeft" => KeyCode::ArrowLeft, "ArrowRight" => KeyCode::ArrowRight, + // Navigation extensions (codex Phase C gate CONCERN): + "Home" => KeyCode::Home, + "End" => KeyCode::End, + "PageUp" => KeyCode::PageUp, + "PageDown" => KeyCode::PageDown, + // Modifier physical codes — distinguish left vs right per + // jian KeyCode. Note: "MetaLeft" / "MetaRight" cover the + // Cmd key on macOS, the Win key on Windows, and the Super + // key on Linux (per Jian Modifiers::CMD). + "ShiftLeft" => KeyCode::ShiftLeft, + "ShiftRight" => KeyCode::ShiftRight, + "ControlLeft" => KeyCode::ControlLeft, + "ControlRight" => KeyCode::ControlRight, + "AltLeft" => KeyCode::AltLeft, + "AltRight" => KeyCode::AltRight, + "MetaLeft" => KeyCode::MetaLeft, + "MetaRight" => KeyCode::MetaRight, + // F1-F12 KeyCode variants don't exist in jian (NamedKey + // covers them); fall to Unknown(code) for now. Phase D+ + // may upstream F-key codes if a widget needs them. _ => KeyCode::Unknown(code.to_string()), } } diff --git a/crates/openpencil-shell-web/src/lib.rs b/crates/openpencil-shell-web/src/lib.rs index c5cb33340..4c777e04a 100644 --- a/crates/openpencil-shell-web/src/lib.rs +++ b/crates/openpencil-shell-web/src/lib.rs @@ -240,100 +240,126 @@ pub fn mount(canvas_id: &str) -> Result { // which point the registration target gets parameterized. let win_target: web_sys::EventTarget = window.clone().into(); - // ----- keyboard: keydown + keyup → WidgetHost::apply_key ----- - { - let inner_kd = inner.clone(); - add_listener::( - &win_target, - "keydown", - &mut listeners, - move |evt: KeyboardEvent| { - let key_event = keyboard::map_keyboard_parts( - &evt.key(), - &evt.code(), - evt.location(), - evt.repeat(), - true, // pressed - modifiers_from_keyboard(&evt), - evt.is_composing(), - ); - let mut inner = inner_kd.borrow_mut(); - inner.host.apply_key(&key_event); - let _ = inner.repaint(); - }, - )?; - } - { - let inner_ku = inner.clone(); - add_listener::( - &win_target, - "keyup", - &mut listeners, - move |evt: KeyboardEvent| { - let key_event = keyboard::map_keyboard_parts( - &evt.key(), - &evt.code(), - evt.location(), - evt.repeat(), - false, // released - modifiers_from_keyboard(&evt), - evt.is_composing(), - ); - let mut inner = inner_ku.borrow_mut(); - inner.host.apply_key(&key_event); - let _ = inner.repaint(); - }, - )?; - } + // Codex Phase C gate BLOCK: registering N listeners with `?` + // is NOT exception-safe — if registration #K fails, the + // partially-built `listeners` vec drops without our `Drop for + // WebShell` ever firing (we never reach the `Ok(WebShell { + // ... })` line), and the K-1 already-registered DOM callbacks + // outlive their wasm-bindgen Closures. Wrap all registrations + // in an inner closure that, on Err, drains the partial vec + // and unregisters everything we managed to land before + // surfacing the error. The unregister loop is the same one + // `Drop for WebShell` runs. + let registration: Result<(), JsValue> = (|listeners: &mut Vec| { + // ----- keyboard: keydown + keyup → WidgetHost::apply_key ----- + { + let inner_kd = inner.clone(); + add_listener::( + &win_target, + "keydown", + listeners, + move |evt: KeyboardEvent| { + let key_event = keyboard::map_keyboard_parts( + &evt.key(), + &evt.code(), + evt.location(), + evt.repeat(), + true, // pressed + modifiers_from_keyboard(&evt), + evt.is_composing(), + ); + let mut inner = inner_kd.borrow_mut(); + inner.host.apply_key(&key_event); + let _ = inner.repaint(); + }, + )?; + } + { + let inner_ku = inner.clone(); + add_listener::( + &win_target, + "keyup", + listeners, + move |evt: KeyboardEvent| { + let key_event = keyboard::map_keyboard_parts( + &evt.key(), + &evt.code(), + evt.location(), + evt.repeat(), + false, // released + modifiers_from_keyboard(&evt), + evt.is_composing(), + ); + let mut inner = inner_ku.borrow_mut(); + inner.host.apply_key(&key_event); + let _ = inner.repaint(); + }, + )?; + } - // ----- IME: compositionstart / update / end → WidgetHost::apply_ime ----- - // Phase C1 ime mappers do the UTF-16→UTF-8 selection remap when - // the browser supplies an IME-highlighted segment via - // `getTargetRanges()`; current browsers expose that range via - // a method we cannot call on `CompositionEvent` directly through - // web-sys 0.3.94 without an extra raw `Reflect::get` shim. - // For Step 1b we forward `data` only; selection lands in Phase D - // alongside the DOM mirror that already needs Reflect::get. - { - let inner_cs = inner.clone(); - add_listener::( - &win_target, - "compositionstart", - &mut listeners, - move |_evt: CompositionEvent| { - let mut inner = inner_cs.borrow_mut(); - inner.host.apply_ime(&ime::composition_start()); - let _ = inner.repaint(); - }, - )?; - } - { - let inner_cu = inner.clone(); - add_listener::( - &win_target, - "compositionupdate", - &mut listeners, - move |evt: CompositionEvent| { - let text = evt.data().unwrap_or_default(); - let mut inner = inner_cu.borrow_mut(); - inner.host.apply_ime(&ime::composition_update(text, None)); - let _ = inner.repaint(); - }, - )?; - } - { - let inner_ce = inner.clone(); - add_listener::( - &win_target, - "compositionend", - &mut listeners, - move |evt: CompositionEvent| { - let text = evt.data().unwrap_or_default(); - let mut inner = inner_ce.borrow_mut(); - inner.host.apply_ime(&ime::composition_end(text)); - let _ = inner.repaint(); - }, - )?; + // ----- IME: compositionstart / update / end → WidgetHost::apply_ime ----- + // Phase C1 ime mappers do the UTF-16→UTF-8 selection remap when + // the browser supplies an IME-highlighted segment via + // `getTargetRanges()`; current browsers expose that range via + // a method we cannot call on `CompositionEvent` directly through + // web-sys 0.3.94 without an extra raw `Reflect::get` shim. + // For Step 1b we forward `data` only; selection lands in Phase D + // alongside the DOM mirror that already needs Reflect::get. + { + let inner_cs = inner.clone(); + add_listener::( + &win_target, + "compositionstart", + listeners, + move |_evt: CompositionEvent| { + let mut inner = inner_cs.borrow_mut(); + inner.host.apply_ime(&ime::composition_start()); + let _ = inner.repaint(); + }, + )?; + } + { + let inner_cu = inner.clone(); + add_listener::( + &win_target, + "compositionupdate", + listeners, + move |evt: CompositionEvent| { + let text = evt.data().unwrap_or_default(); + let mut inner = inner_cu.borrow_mut(); + inner.host.apply_ime(&ime::composition_update(text, None)); + let _ = inner.repaint(); + }, + )?; + } + { + let inner_ce = inner.clone(); + add_listener::( + &win_target, + "compositionend", + listeners, + move |evt: CompositionEvent| { + let text = evt.data().unwrap_or_default(); + let mut inner = inner_ce.borrow_mut(); + inner.host.apply_ime(&ime::composition_end(text)); + let _ = inner.repaint(); + }, + )?; + } + Ok(()) + })(&mut listeners); + + if let Err(e) = registration { + // Unwind partial registration. Same body as `Drop for + // WebShell`; kept inline rather than refactored into a free + // function because the lifetimes only line up when the + // `Listener` vec is local to mount(). + for l in listeners.drain(..) { + let _ = l + .target + .remove_event_listener_with_callback(l.name, l.closure.as_ref().unchecked_ref()); + } + return Err(e); } Ok(WebShell { inner, listeners }) diff --git a/crates/openpencil-shell-web/tests/dom_event_mapping.rs b/crates/openpencil-shell-web/tests/dom_event_mapping.rs index 3139b3eec..ddc2f8a5c 100644 --- a/crates/openpencil-shell-web/tests/dom_event_mapping.rs +++ b/crates/openpencil-shell-web/tests/dom_event_mapping.rs @@ -93,6 +93,76 @@ fn keyboard_unidentified_for_multi_char_keys() { assert_eq!(event.code, KeyCode::Unknown("Pause".to_string())); } +#[test] +fn keyboard_navigation_keys_mapped() { + // Codex Phase C gate Round 3 BLOCK: every NamedKey/KeyCode variant + // jian exposes must round-trip through the mapper rather than + // falling to Unidentified / Unknown(...). + for (k, expected) in [ + ("Home", NamedKey::Home), + ("End", NamedKey::End), + ("PageUp", NamedKey::PageUp), + ("PageDown", NamedKey::PageDown), + ("CapsLock", NamedKey::CapsLock), + ] { + let event = map_keyboard_parts(k, k, 0, false, true, Modifiers::empty(), false); + assert_eq!(event.key, KeyValue::Named(expected), "key {k}"); + } +} + +#[test] +fn keyboard_function_keys_mapped() { + use openpencil_shell_core::NamedKey::*; + for (k, expected) in [ + ("F1", F1), + ("F2", F2), + ("F3", F3), + ("F4", F4), + ("F5", F5), + ("F6", F6), + ("F7", F7), + ("F8", F8), + ("F9", F9), + ("F10", F10), + ("F11", F11), + ("F12", F12), + ] { + let event = map_keyboard_parts(k, k, 0, false, true, Modifiers::empty(), false); + assert_eq!(event.key, KeyValue::Named(expected), "fn-key {k}"); + } +} + +#[test] +fn keyboard_modifier_keys_mapped_to_named_values() { + // W3C `key` for modifier keys themselves is just the bare name; + // `code` carries left/right ("ShiftLeft" etc) which map_key_code + // covers. Verify both halves round-trip. + for (k, c, expected_key, expected_code) in [ + ("Shift", "ShiftLeft", NamedKey::Shift, KeyCode::ShiftLeft), + ("Shift", "ShiftRight", NamedKey::Shift, KeyCode::ShiftRight), + ( + "Control", + "ControlLeft", + NamedKey::Control, + KeyCode::ControlLeft, + ), + ( + "Control", + "ControlRight", + NamedKey::Control, + KeyCode::ControlRight, + ), + ("Alt", "AltLeft", NamedKey::Alt, KeyCode::AltLeft), + ("Alt", "AltRight", NamedKey::Alt, KeyCode::AltRight), + ("Meta", "MetaLeft", NamedKey::Meta, KeyCode::MetaLeft), + ("Meta", "MetaRight", NamedKey::Meta, KeyCode::MetaRight), + ] { + let event = map_keyboard_parts(k, c, 0, false, true, Modifiers::empty(), false); + assert_eq!(event.key, KeyValue::Named(expected_key), "mod key {k}"); + assert_eq!(event.code, expected_code, "mod code {c}"); + } +} + // --------------------------------------------------------------------- // IME // ---------------------------------------------------------------------