diff --git a/crates/neomacs-display-runtime/src/render_thread/input.rs b/crates/neomacs-display-runtime/src/render_thread/input.rs index 2f0666b413..e7fc58e571 100644 --- a/crates/neomacs-display-runtime/src/render_thread/input.rs +++ b/crates/neomacs-display-runtime/src/render_thread/input.rs @@ -3,6 +3,7 @@ use crate::backend::wgpu::{ NEOMACS_ALT_MASK, NEOMACS_CTRL_MASK, NEOMACS_HYPER_MASK, NEOMACS_META_MASK, NEOMACS_SUPER_MASK, }; +use neovm_core::keyboard::FrontendKey; use winit::keyboard::{Key, NamedKey, NativeKey}; use super::RenderApp; @@ -42,15 +43,27 @@ pub(super) struct MenuBarHit { } impl RenderApp { - /// Translate winit key to X11 keysym. + /// Preserve the toolkit's distinction between text and key identities. + pub(super) fn translate_key_input(key: &Key) -> Option { + match key { + Key::Character(text) => text.chars().next().map(FrontendKey::Character), + _ => { + let keysym = Self::translate_keysym(key); + (keysym != 0).then_some(FrontendKey::Keysym(keysym)) + } + } + } + + /// Translate named and unidentified toolkit keys to X11/native keysyms. /// - /// Every key gets an identity here, the way GNU's backends hand + /// Text is handled by `translate_key_input`; dead keys are compose state. + /// Named keys get an identity here, the way GNU's backends hand /// `keyboard.c` whatever the toolkit reported and let `modify_event_symbol` /// name it. Only a modifier is not a keystroke at all — those arrive /// through `ModifiersChanged` — and a key this table does not spell yet is /// logged rather than dropped in silence, so the gap is visible instead of /// looking like "unsupported". - pub(super) fn translate_key(key: &Key) -> u32 { + pub(super) fn translate_keysym(key: &Key) -> u32 { match key { Key::Named(named) => match named { // Function keys. The X11 block is contiguous from XK_F1 @@ -248,7 +261,8 @@ impl RenderApp { 0 } }, - Key::Character(c) => c.chars().next().map(|ch| ch as u32).unwrap_or(0), + // Text is handled by translate_key_input, never interpreted as a keysym. + Key::Character(_) => 0, // A key winit could not name at all. X11 and Wayland hand over // the raw keysym, which is already this port's identity; the // platforms whose native key is a scancode or a virtual-key code @@ -284,7 +298,7 @@ impl RenderApp { /// includes the policy-cooked `A-' and `H-' bits — GNU cooks /// `parse_solitary_modifier("alt")` to a distinct modifier bit /// (`src/keyboard.c:7941`). - pub(super) fn translate_committed_text(text: &str, modifiers: u32) -> Option> { + pub(super) fn translate_committed_text(text: &str, modifiers: u32) -> Option> { let command_modifiers_active = modifiers & (NEOMACS_CTRL_MASK | NEOMACS_META_MASK @@ -296,18 +310,13 @@ impl RenderApp { return None; } - let keysyms: Vec = text + let keys: Vec = text .chars() .filter(|ch| !ch.is_control()) - .map(|ch| ch as u32) - .filter(|keysym| *keysym != 0) + .map(FrontendKey::Character) .collect(); - if keysyms.is_empty() { - None - } else { - Some(keysyms) - } + if keys.is_empty() { None } else { Some(keys) } } /// Return whether a `KeyboardInput` event should use its committed-text diff --git a/crates/neomacs-display-runtime/src/render_thread/input/tests/mod.rs b/crates/neomacs-display-runtime/src/render_thread/input/tests/mod.rs index f0cf61a31b..641c8196f3 100644 --- a/crates/neomacs-display-runtime/src/render_thread/input/tests/mod.rs +++ b/crates/neomacs-display-runtime/src/render_thread/input/tests/mod.rs @@ -2009,7 +2009,7 @@ fn translate_key_f1_through_f12() { ]; for (named, keysym) in expected { assert_eq!( - RenderApp::translate_key(&Key::Named(named)), + RenderApp::translate_keysym(&Key::Named(named)), keysym, "F-key mismatch for {:?}", named @@ -2037,7 +2037,7 @@ fn translate_key_navigation_keys() { ]; for (named, keysym) in cases { assert_eq!( - RenderApp::translate_key(&Key::Named(named)), + RenderApp::translate_keysym(&Key::Named(named)), keysym, "Navigation key mismatch for {:?}", named @@ -2052,19 +2052,19 @@ fn translate_key_navigation_keys() { #[test] fn translate_key_arrow_keys() { assert_eq!( - RenderApp::translate_key(&Key::Named(NamedKey::ArrowLeft)), + RenderApp::translate_keysym(&Key::Named(NamedKey::ArrowLeft)), 0xff51 ); assert_eq!( - RenderApp::translate_key(&Key::Named(NamedKey::ArrowUp)), + RenderApp::translate_keysym(&Key::Named(NamedKey::ArrowUp)), 0xff52 ); assert_eq!( - RenderApp::translate_key(&Key::Named(NamedKey::ArrowRight)), + RenderApp::translate_keysym(&Key::Named(NamedKey::ArrowRight)), 0xff53 ); assert_eq!( - RenderApp::translate_key(&Key::Named(NamedKey::ArrowDown)), + RenderApp::translate_keysym(&Key::Named(NamedKey::ArrowDown)), 0xff54 ); } @@ -2075,7 +2075,10 @@ fn translate_key_arrow_keys() { #[test] fn translate_key_space() { - assert_eq!(RenderApp::translate_key(&Key::Character(" ".into())), 0x20); + assert_eq!( + RenderApp::translate_key_input(&Key::Character(" ".into())), + Some(FrontendKey::Character(' ')) + ); } // =================================================================== @@ -2085,15 +2088,15 @@ fn translate_key_space() { #[test] fn translate_key_other_named() { assert_eq!( - RenderApp::translate_key(&Key::Named(NamedKey::PrintScreen)), + RenderApp::translate_keysym(&Key::Named(NamedKey::PrintScreen)), 0xff61 ); assert_eq!( - RenderApp::translate_key(&Key::Named(NamedKey::ScrollLock)), + RenderApp::translate_keysym(&Key::Named(NamedKey::ScrollLock)), 0xff14 ); assert_eq!( - RenderApp::translate_key(&Key::Named(NamedKey::Pause)), + RenderApp::translate_keysym(&Key::Named(NamedKey::Pause)), 0xff13 ); } @@ -2114,7 +2117,7 @@ fn translate_key_modifier_keys_suppressed() { ]; for named in modifiers { assert_eq!( - RenderApp::translate_key(&Key::Named(named)), + RenderApp::translate_keysym(&Key::Named(named)), 0, "Modifier {:?} should be suppressed (return 0)", named @@ -2131,8 +2134,8 @@ fn translate_key_ascii_characters() { for ch in 'a'..='z' { let key = Key::Character(SmolStr::new(ch.to_string())); assert_eq!( - RenderApp::translate_key(&key), - ch as u32, + RenderApp::translate_key_input(&key), + Some(FrontendKey::Character(ch)), "Character key mismatch for '{}'", ch ); @@ -2144,8 +2147,8 @@ fn translate_key_digit_characters() { for ch in '0'..='9' { let key = Key::Character(SmolStr::new(ch.to_string())); assert_eq!( - RenderApp::translate_key(&key), - ch as u32, + RenderApp::translate_key_input(&key), + Some(FrontendKey::Character(ch)), "Digit key mismatch for '{}'", ch ); @@ -2154,23 +2157,11 @@ fn translate_key_digit_characters() { #[test] fn translate_key_special_characters() { - let specials = vec![ - ('!', 0x21), - ('@', 0x40), - ('#', 0x23), - ('/', 0x2f), - ('-', 0x2d), - ('=', 0x3d), - ('[', 0x5b), - (']', 0x5d), - (';', 0x3b), - ('\'', 0x27), - ]; - for (ch, code) in specials { + for ch in ['!', '@', '#', '/', '-', '=', '[', ']', ';', '\''] { let key = Key::Character(SmolStr::new(ch.to_string())); assert_eq!( - RenderApp::translate_key(&key), - code, + RenderApp::translate_key_input(&key), + Some(FrontendKey::Character(ch)), "Special char mismatch for '{}'", ch ); @@ -2179,18 +2170,24 @@ fn translate_key_special_characters() { #[test] fn translate_key_unicode_character() { - // Multi-byte Unicode characters should return the Unicode code point + // The toolkit's Unicode scalar must retain character identity. let key = Key::Character(SmolStr::new("\u{00e9}")); // e-acute - assert_eq!(RenderApp::translate_key(&key), 0xe9); + assert_eq!( + RenderApp::translate_key_input(&key), + Some(FrontendKey::Character('é')) + ); let key = Key::Character(SmolStr::new("\u{4e2d}")); // CJK character - assert_eq!(RenderApp::translate_key(&key), 0x4e2d); + assert_eq!( + RenderApp::translate_key_input(&key), + Some(FrontendKey::Character('中')) + ); } #[test] fn translate_key_empty_character_string() { let key = Key::Character(SmolStr::new("")); - assert_eq!(RenderApp::translate_key(&key), 0); + assert_eq!(RenderApp::translate_key_input(&key), None); } // =================================================================== @@ -2200,20 +2197,45 @@ fn translate_key_empty_character_string() { #[test] fn translate_key_dead_returns_zero() { let key = Key::Dead(None); - assert_eq!(RenderApp::translate_key(&key), 0); + assert_eq!(RenderApp::translate_keysym(&key), 0); } #[test] fn translate_key_unidentified_returns_zero() { let key = Key::Unidentified(winit::keyboard::NativeKey::Unidentified); - assert_eq!(RenderApp::translate_key(&key), 0); + assert_eq!(RenderApp::translate_keysym(&key), 0); +} + +#[test] +fn fullwidth_committed_text_reaches_core_as_a_character() { + let transported = RenderApp::translate_committed_text(",", 0).unwrap(); + let event = + neovm_core::keyboard::render_key_transport_to_input_event(transported[0], 0, true, 42) + .unwrap(); + assert!( + matches!(event, neovm_core::keyboard::InputEvent::KeyPress { key, emacs_frame_id: 42 } if key.key == neovm_core::keyboard::Key::Char(',')) + ); +} + +#[test] +fn fullwidth_logical_keys_reach_core_as_characters() { + for character in ",();-ヲᅧ".chars() { + let toolkit_key = Key::Character(character.to_string().into()); + let key = RenderApp::translate_key_input(&toolkit_key).unwrap(); + let event = + neovm_core::keyboard::render_key_transport_to_input_event(key, 0, true, 42).unwrap(); + assert!( + matches!(event, neovm_core::keyboard::InputEvent::KeyPress { key, emacs_frame_id: 42 } + if key.key == neovm_core::keyboard::Key::Char(character)) + ); + } } #[test] fn translate_committed_text_prefers_uppercase_ascii_without_command_modifiers() { assert_eq!( RenderApp::translate_committed_text("A", 0), - Some(vec!['A' as u32]) + Some(vec![FrontendKey::Character('A')]) ); } @@ -2221,7 +2243,7 @@ fn translate_committed_text_prefers_uppercase_ascii_without_command_modifiers() fn translate_committed_text_prefers_shifted_punctuation_without_command_modifiers() { assert_eq!( RenderApp::translate_committed_text("!", 0), - Some(vec!['!' as u32]) + Some(vec![FrontendKey::Character('!')]) ); } @@ -2859,7 +2881,7 @@ fn translate_key_f13_through_f35() { ]; for (named, keysym) in expected { assert_eq!( - RenderApp::translate_key(&Key::Named(named)), + RenderApp::translate_keysym(&Key::Named(named)), keysym, "{named:?}" ); @@ -2879,7 +2901,7 @@ fn translate_key_misc_function_band() { ]; for (named, keysym) in expected { assert_eq!( - RenderApp::translate_key(&Key::Named(named)), + RenderApp::translate_keysym(&Key::Named(named)), keysym, "{named:?}" ); @@ -2899,7 +2921,7 @@ fn translate_key_xf86_band() { ]; for (named, keysym) in expected { assert_eq!( - RenderApp::translate_key(&Key::Named(named)), + RenderApp::translate_keysym(&Key::Named(named)), keysym, "{named:?}" ); @@ -2912,20 +2934,20 @@ fn translate_key_xf86_band() { #[test] fn translate_key_keeps_unmapped_native_keys() { assert_eq!( - RenderApp::translate_key(&Key::Unidentified(NativeKey::Xkb(0x1008ff50))), + RenderApp::translate_keysym(&Key::Unidentified(NativeKey::Xkb(0x1008ff50))), 0x1008ff50, "the raw keysym is the identity" ); assert_eq!( - RenderApp::translate_key(&Key::Unidentified(NativeKey::MacOS(0x24))), + RenderApp::translate_keysym(&Key::Unidentified(NativeKey::MacOS(0x24))), neovm_core::keyboard::native_key_macos(0x24), ); assert_eq!( - RenderApp::translate_key(&Key::Unidentified(NativeKey::Windows(0x5d))), + RenderApp::translate_keysym(&Key::Unidentified(NativeKey::Windows(0x5d))), neovm_core::keyboard::native_key_windows(0x5d), ); assert_eq!( - RenderApp::translate_key(&Key::Unidentified(NativeKey::Android(4))), + RenderApp::translate_keysym(&Key::Unidentified(NativeKey::Android(4))), neovm_core::keyboard::native_key_android(4), ); } @@ -2946,7 +2968,7 @@ fn translate_key_suppresses_modifiers() { NamedKey::Hyper, ] { assert_eq!( - RenderApp::translate_key(&Key::Named(modifier)), + RenderApp::translate_keysym(&Key::Named(modifier)), 0, "{modifier:?}" ); @@ -3044,7 +3066,7 @@ fn translate_key_names_the_media_launch_and_ime_families() { (NamedKey::ZoomOut, 0x1008ff8c), // XF86ZoomOut ] { assert_eq!( - RenderApp::translate_key(&Key::Named(key)), + RenderApp::translate_keysym(&Key::Named(key)), expected, "{key:?}" ); diff --git a/crates/neomacs-display-runtime/src/render_thread/tests.rs b/crates/neomacs-display-runtime/src/render_thread/tests.rs index fb4a63e9fe..0256b94121 100644 --- a/crates/neomacs-display-runtime/src/render_thread/tests.rs +++ b/crates/neomacs-display-runtime/src/render_thread/tests.rs @@ -556,87 +556,102 @@ fn make_test_device() -> Option { #[test] fn test_translate_key_named() { assert_eq!( - RenderApp::translate_key(&Key::Named(NamedKey::Escape)), + RenderApp::translate_keysym(&Key::Named(NamedKey::Escape)), 0xff1b ); assert_eq!( - RenderApp::translate_key(&Key::Named(NamedKey::Enter)), + RenderApp::translate_keysym(&Key::Named(NamedKey::Enter)), 0xff0d ); - assert_eq!(RenderApp::translate_key(&Key::Named(NamedKey::Tab)), 0xff09); assert_eq!( - RenderApp::translate_key(&Key::Named(NamedKey::Backspace)), + RenderApp::translate_keysym(&Key::Named(NamedKey::Tab)), + 0xff09 + ); + assert_eq!( + RenderApp::translate_keysym(&Key::Named(NamedKey::Backspace)), 0xff08 ); assert_eq!( - RenderApp::translate_key(&Key::Named(NamedKey::Delete)), + RenderApp::translate_keysym(&Key::Named(NamedKey::Delete)), 0xffff ); assert_eq!( - RenderApp::translate_key(&Key::Named(NamedKey::Home)), + RenderApp::translate_keysym(&Key::Named(NamedKey::Home)), 0xff50 ); - assert_eq!(RenderApp::translate_key(&Key::Named(NamedKey::End)), 0xff57); assert_eq!( - RenderApp::translate_key(&Key::Named(NamedKey::PageUp)), + RenderApp::translate_keysym(&Key::Named(NamedKey::End)), + 0xff57 + ); + assert_eq!( + RenderApp::translate_keysym(&Key::Named(NamedKey::PageUp)), 0xff55 ); assert_eq!( - RenderApp::translate_key(&Key::Named(NamedKey::PageDown)), + RenderApp::translate_keysym(&Key::Named(NamedKey::PageDown)), 0xff56 ); assert_eq!( - RenderApp::translate_key(&Key::Named(NamedKey::ArrowLeft)), + RenderApp::translate_keysym(&Key::Named(NamedKey::ArrowLeft)), 0xff51 ); assert_eq!( - RenderApp::translate_key(&Key::Named(NamedKey::ArrowUp)), + RenderApp::translate_keysym(&Key::Named(NamedKey::ArrowUp)), 0xff52 ); assert_eq!( - RenderApp::translate_key(&Key::Named(NamedKey::ArrowRight)), + RenderApp::translate_keysym(&Key::Named(NamedKey::ArrowRight)), 0xff53 ); assert_eq!( - RenderApp::translate_key(&Key::Named(NamedKey::ArrowDown)), + RenderApp::translate_keysym(&Key::Named(NamedKey::ArrowDown)), 0xff54 ); - assert_eq!(RenderApp::translate_key(&Key::Character(" ".into())), 0x20); + assert_eq!( + RenderApp::translate_key_input(&Key::Character(" ".into())), + Some(neovm_core::keyboard::FrontendKey::Character(' ')) + ); } #[test] fn test_translate_key_character() { assert_eq!( - RenderApp::translate_key(&Key::Character("a".into())), - 'a' as u32 + RenderApp::translate_key_input(&Key::Character("a".into())), + Some(neovm_core::keyboard::FrontendKey::Character('a')) ); assert_eq!( - RenderApp::translate_key(&Key::Character("A".into())), - 'A' as u32 + RenderApp::translate_key_input(&Key::Character("A".into())), + Some(neovm_core::keyboard::FrontendKey::Character('A')) ); assert_eq!( - RenderApp::translate_key(&Key::Character("1".into())), - '1' as u32 + RenderApp::translate_key_input(&Key::Character("1".into())), + Some(neovm_core::keyboard::FrontendKey::Character('1')) ); } #[test] fn test_translate_key_function_keys() { - assert_eq!(RenderApp::translate_key(&Key::Named(NamedKey::F1)), 0xffbe); - assert_eq!(RenderApp::translate_key(&Key::Named(NamedKey::F12)), 0xffc9); assert_eq!( - RenderApp::translate_key(&Key::Named(NamedKey::Insert)), + RenderApp::translate_keysym(&Key::Named(NamedKey::F1)), + 0xffbe + ); + assert_eq!( + RenderApp::translate_keysym(&Key::Named(NamedKey::F12)), + 0xffc9 + ); + assert_eq!( + RenderApp::translate_keysym(&Key::Named(NamedKey::Insert)), 0xff63 ); assert_eq!( - RenderApp::translate_key(&Key::Named(NamedKey::PrintScreen)), + RenderApp::translate_keysym(&Key::Named(NamedKey::PrintScreen)), 0xff61 ); } #[test] fn test_translate_key_unknown() { - assert_eq!(RenderApp::translate_key(&Key::Dead(None)), 0); + assert_eq!(RenderApp::translate_keysym(&Key::Dead(None)), 0); } #[test] diff --git a/crates/neomacs-display-runtime/src/render_thread/window_events.rs b/crates/neomacs-display-runtime/src/render_thread/window_events.rs index d1709926dd..fb0699d3ac 100644 --- a/crates/neomacs-display-runtime/src/render_thread/window_events.rs +++ b/crates/neomacs-display-runtime/src/render_thread/window_events.rs @@ -3,6 +3,7 @@ use super::modifier_sides::{TrackedModifier, TrackedSide}; use super::state::effective_window_scale_factor; use crate::thread_comm::InputEvent; use neomacs_display_protocol::{ModifierEventKind, TransportModifierBits}; +use neovm_core::keyboard::FrontendKey; use winit::event::{ElementState, KeyEvent, WindowEvent}; use winit::event_loop::ActiveEventLoop; use winit::keyboard::PhysicalKey; @@ -319,7 +320,14 @@ impl RenderApp { if let Some(target) = self.focused_webview { use winit::platform::scancode::PhysicalKeyExtScancode; - let key_value = Self::translate_key(&logical_key); + // The external WebView API has its own numeric key-value + // contract. Keep this conversion at that boundary; the + // editor transport retains the typed identity below. + let key_value = match Self::translate_key_input(&logical_key) { + Some(FrontendKey::Character(character)) => character as u32, + Some(FrontendKey::Keysym(keysym)) => keysym, + None => 0, + }; if key_value != 0 && let Some(system) = self.webview_system.as_mut() { @@ -386,7 +394,7 @@ impl RenderApp { ordinary_modifiers ); self.comms.send_input(InputEvent::Key { - keysym: control_keysym, + key: FrontendKey::Keysym(control_keysym), modifiers: ordinary_modifiers, pressed: true, emacs_frame_id: self.emacs_frame_for_window_event(window_id), @@ -394,23 +402,23 @@ impl RenderApp { self.record_idle_dim_activity(window_id); self.record_typing_speed_keypress(window_id); handled_via_text = true; - } else if let Some(keysyms) = + } else if let Some(keys) = Self::translate_committed_text(s, ordinary_modifiers) { tracing::debug!( - "KeyboardInput committed text path: text={:?} keysyms={:?} mods=0x{:x}", + "KeyboardInput committed text path: text={:?} keys={:?} mods=0x{:x}", s, - keysyms, + keys, ordinary_modifiers ); - for keysym in keysyms { + for key in keys { tracing::debug!( - "Queueing text key event: keysym=0x{:04x} mods=0x{:x}", - keysym, + "Queueing text key event: key={:?} mods=0x{:x}", + key, ordinary_modifiers ); self.comms.send_input(InputEvent::Key { - keysym, + key, modifiers: ordinary_modifiers, pressed: true, emacs_frame_id: self.emacs_frame_for_window_event(window_id), @@ -429,32 +437,19 @@ impl RenderApp { // another character first, and the frame window asks // it not to (`apply_option_key_policy`), so no // per-event substitution is needed here. - let mut keysym = Self::translate_key(&logical_key); - // Shift-only chords still reach the keystroke path - // the way GNU keeps them ordinary keys; the - // space-fallback gate below now includes the - // policy-cooked alt/hyper bits. - let mut key_modifiers = if keysym != 0 { - self.cooked_key_modifiers(keysym) - } else { - 0 - }; - if keysym == 0 && key_modifiers != 0 { - use winit::keyboard::KeyCode; - keysym = match physical_key { - PhysicalKey::Code(KeyCode::Space) => 0x20, - _ => 0, + let key = Self::translate_key_input(&logical_key); + if let Some(key) = key { + let key_modifiers = match key { + FrontendKey::Character(_) => { + self.cook_modifiers(ModifierEventKind::Ordinary).bits() + } + FrontendKey::Keysym(keysym) => self.cooked_key_modifiers(keysym), }; - if keysym != 0 { - key_modifiers = self.cooked_key_modifiers(keysym); - } - } - if keysym != 0 { tracing::debug!( - "KeyboardInput translated path: logical_key={:?} physical_key={:?} keysym=0x{:04x} mods=0x{:x} pressed={}", + "KeyboardInput translated path: logical_key={:?} physical_key={:?} key={:?} mods=0x{:x} pressed={}", logical_key, physical_key, - keysym, + key, key_modifiers, state == ElementState::Pressed ); @@ -472,7 +467,7 @@ impl RenderApp { } let (receipt, token) = self.comms.send_input_with_receipt(InputEvent::Key { - keysym, + key, modifiers: key_modifiers, pressed: state == ElementState::Pressed, emacs_frame_id: self.emacs_frame_for_window_event(window_id), @@ -695,10 +690,9 @@ impl RenderApp { ws.render.clear_ime_preedit() }; for ch in text.chars() { - let keysym = ch as u32; - if keysym != 0 { + if ch != '\0' { self.comms.send_input(InputEvent::Key { - keysym, + key: FrontendKey::Character(ch), modifiers: 0, pressed: true, emacs_frame_id: self.emacs_frame_for_window_event(window_id), diff --git a/crates/neomacs-display-runtime/src/thread_comm.rs b/crates/neomacs-display-runtime/src/thread_comm.rs index 64f485f327..bcce516900 100644 --- a/crates/neomacs-display-runtime/src/thread_comm.rs +++ b/crates/neomacs-display-runtime/src/thread_comm.rs @@ -129,7 +129,7 @@ pub enum InputEvent { emacs_frame_id: u64, }, Key { - keysym: u32, + key: neovm_core::keyboard::FrontendKey, modifiers: u32, pressed: bool, /// Emacs frame_id of the window that produced the key event @@ -1054,7 +1054,7 @@ impl RenderComms { if neomacs_display_protocol::input_latency::enabled() { let target = match &event { InputEvent::Key { - keysym: 0xff55 | 0xff56, + key: neovm_core::keyboard::FrontendKey::Keysym(0xff55 | 0xff56), pressed: true, emacs_frame_id, .. @@ -1192,7 +1192,7 @@ impl RenderComms { let receipt = if matches!( &event, InputEvent::Key { - keysym: 0xff55 | 0xff56, + key: neovm_core::keyboard::FrontendKey::Keysym(0xff55 | 0xff56), pressed: true, .. } | InputEvent::PositionedPointer(PositionedPointerInput { diff --git a/crates/neomacs-display-runtime/src/thread_comm/tests/mod.rs b/crates/neomacs-display-runtime/src/thread_comm/tests/mod.rs index 3b77f4fe71..d133b2660b 100644 --- a/crates/neomacs-display-runtime/src/thread_comm/tests/mod.rs +++ b/crates/neomacs-display-runtime/src/thread_comm/tests/mod.rs @@ -152,7 +152,7 @@ fn thread_comms_input_channel_roundtrip() { let comms = ThreadComms::new(); let event = InputEvent::Key { - keysym: 65, // 'A' + key: neovm_core::keyboard::FrontendKey::Keysym(65), // 'A' modifiers: 0, pressed: true, emacs_frame_id: 0, @@ -163,12 +163,12 @@ fn thread_comms_input_channel_roundtrip() { let received = comms.input_rx.try_recv().unwrap(); match received { InputEvent::Key { - keysym, + key, modifiers, pressed, emacs_frame_id, } => { - assert_eq!(keysym, 65); + assert_eq!(key, neovm_core::keyboard::FrontendKey::Keysym(65)); assert_eq!(modifiers, 0); assert!(pressed); assert_eq!(emacs_frame_id, 0); @@ -301,7 +301,7 @@ fn thread_comms_input_channel_bounded_capacity() { // Fill up the input channel to capacity for _ in 0..INPUT_CHANNEL_CAPACITY { let event = InputEvent::Key { - keysym: 0, + key: neovm_core::keyboard::FrontendKey::Keysym(0), modifiers: 0, pressed: false, emacs_frame_id: 0, @@ -311,7 +311,7 @@ fn thread_comms_input_channel_bounded_capacity() { // Next try_send should fail (channel full) let result = comms.input_tx.try_send(InputEvent::Key { - keysym: 0, + key: neovm_core::keyboard::FrontendKey::Keysym(0), modifiers: 0, pressed: false, emacs_frame_id: 0, @@ -434,19 +434,19 @@ fn render_comms_send_input_delivers_event() { #[test] fn input_event_key_construction() { let event = InputEvent::Key { - keysym: 0xFF0D, // Return - modifiers: 4, // Ctrl + key: neovm_core::keyboard::FrontendKey::Keysym(0xFF0D), // Return + modifiers: 4, // Ctrl pressed: true, emacs_frame_id: 0, }; match event { InputEvent::Key { - keysym, + key, modifiers, pressed, emacs_frame_id, } => { - assert_eq!(keysym, 0xFF0D); + assert_eq!(key, neovm_core::keyboard::FrontendKey::Keysym(0xFF0D)); assert_eq!(modifiers, 4); assert!(pressed); assert_eq!(emacs_frame_id, 0); @@ -683,7 +683,7 @@ fn a_file_drop_reports_the_dropped_paths_and_nothing_that_stands_in_for_a_posn() #[test] fn input_event_clone() { let original = InputEvent::Key { - keysym: 42, + key: neovm_core::keyboard::FrontendKey::Keysym(42), modifiers: 8, pressed: false, emacs_frame_id: 0, @@ -691,12 +691,12 @@ fn input_event_clone() { let cloned = original.clone(); match cloned { InputEvent::Key { - keysym, + key, modifiers, pressed, emacs_frame_id, } => { - assert_eq!(keysym, 42); + assert_eq!(key, neovm_core::keyboard::FrontendKey::Keysym(42)); assert_eq!(modifiers, 8); assert!(!pressed); assert_eq!(emacs_frame_id, 0); @@ -708,7 +708,7 @@ fn input_event_clone() { #[test] fn input_event_debug() { let event = InputEvent::Key { - keysym: 65, + key: neovm_core::keyboard::FrontendKey::Keysym(65), modifiers: 0, pressed: true, emacs_frame_id: 0, @@ -1539,19 +1539,19 @@ fn channel_sends_multiple_input_events_in_order() { let events = vec![ InputEvent::Key { - keysym: 1, + key: neovm_core::keyboard::FrontendKey::Keysym(1), modifiers: 0, pressed: true, emacs_frame_id: 0, }, InputEvent::Key { - keysym: 2, + key: neovm_core::keyboard::FrontendKey::Keysym(2), modifiers: 0, pressed: true, emacs_frame_id: 0, }, InputEvent::Key { - keysym: 3, + key: neovm_core::keyboard::FrontendKey::Keysym(3), modifiers: 0, pressed: true, emacs_frame_id: 0, @@ -1641,7 +1641,7 @@ fn cross_thread_input_event_delivery() { let handle = std::thread::spawn(move || { render.send_input(InputEvent::Key { - keysym: 0x61, // 'a' + key: neovm_core::keyboard::FrontendKey::Keysym(0x61), // 'a' modifiers: 0, pressed: true, emacs_frame_id: 0, @@ -1659,7 +1659,9 @@ fn cross_thread_input_event_delivery() { // Both events should be receivable on the Emacs side let evt1 = emacs.input_rx.try_recv().unwrap(); match evt1 { - InputEvent::Key { keysym, .. } => assert_eq!(keysym, 0x61), + InputEvent::Key { key, .. } => { + assert_eq!(key, neovm_core::keyboard::FrontendKey::Keysym(0x61)) + } other => panic!("Expected Key, got {:?}", other), } diff --git a/crates/neomacs-display-runtime/src/tty_input.rs b/crates/neomacs-display-runtime/src/tty_input.rs index 79761c9962..01390832bc 100644 --- a/crates/neomacs-display-runtime/src/tty_input.rs +++ b/crates/neomacs-display-runtime/src/tty_input.rs @@ -96,34 +96,35 @@ fn map_key_event(event: KeyEvent) -> Option { let mut modifiers = map_modifiers(event.modifiers); - let keysym = match event.code { + let native = |keysym| Some(neovm_core::keyboard::FrontendKey::Keysym(keysym)); + let key = match event.code { KeyCode::Char(c) if event.modifiers.contains(KeyModifiers::CONTROL) && tty_control_char_keysym(c).is_some() => { modifiers = without_control(modifiers); - tty_control_char_keysym(c) + tty_control_char_keysym(c).map(neovm_core::keyboard::FrontendKey::Keysym) } - KeyCode::Char(c) => Some(c as u32), + KeyCode::Char(c) => Some(neovm_core::keyboard::FrontendKey::Character(c)), KeyCode::F(n) if (1..=12).contains(&n) => { - Some(0xffbe + (n as u32 - 1)) // F1=0xffbe … F12=0xffc9 + native(0xffbe + (n as u32 - 1)) // F1=0xffbe … F12=0xffc9 } KeyCode::F(_) => None, // unsupported function key - KeyCode::Esc => Some(XK_ESCAPE), - KeyCode::Enter => Some(XK_RETURN), - KeyCode::Tab => Some(XK_TAB), - KeyCode::Backspace => Some(0x7f), - KeyCode::Delete => Some(0xffff), - KeyCode::Insert => Some(0xff63), - KeyCode::Home => Some(0xff50), - KeyCode::End => Some(0xff57), - KeyCode::PageUp => Some(0xff55), - KeyCode::PageDown => Some(0xff56), - KeyCode::Left => Some(XK_LEFT), - KeyCode::Up => Some(XK_UP), - KeyCode::Right => Some(XK_RIGHT), - KeyCode::Down => Some(XK_DOWN), - KeyCode::Null => Some(0x00), + KeyCode::Esc => native(XK_ESCAPE), + KeyCode::Enter => native(XK_RETURN), + KeyCode::Tab => native(XK_TAB), + KeyCode::Backspace => native(0x7f), + KeyCode::Delete => native(0xffff), + KeyCode::Insert => native(0xff63), + KeyCode::Home => native(0xff50), + KeyCode::End => native(0xff57), + KeyCode::PageUp => native(0xff55), + KeyCode::PageDown => native(0xff56), + KeyCode::Left => native(XK_LEFT), + KeyCode::Up => native(XK_UP), + KeyCode::Right => native(XK_RIGHT), + KeyCode::Down => native(XK_DOWN), + KeyCode::Null => native(0x00), KeyCode::CapsLock | KeyCode::ScrollLock | KeyCode::NumLock @@ -133,13 +134,13 @@ fn map_key_event(event: KeyEvent) -> Option { | KeyCode::KeypadBegin | KeyCode::Media(_) | KeyCode::Modifier(_) => None, // suppress bare modifier/media keys - KeyCode::BackTab => Some(0xff09), // same as Tab, but with shift modifier + KeyCode::BackTab => native(0xff09), // same as Tab, but with shift modifier }; - let keysym = keysym?; + let key = key?; Some(InputEvent::Key { - keysym, + key, modifiers, pressed: event.kind == KeyEventKind::Press, emacs_frame_id: 0, diff --git a/crates/neomacs-display-runtime/src/tty_input/tests/tty_input_test.rs b/crates/neomacs-display-runtime/src/tty_input/tests/tty_input_test.rs index 5edd89aef8..a50bd7ece5 100644 --- a/crates/neomacs-display-runtime/src/tty_input/tests/tty_input_test.rs +++ b/crates/neomacs-display-runtime/src/tty_input/tests/tty_input_test.rs @@ -1,5 +1,6 @@ use super::*; use crossterm::event::{KeyEventKind, KeyEventState}; +use neovm_core::keyboard::FrontendKey; #[cfg(unix)] #[test] @@ -24,11 +25,9 @@ fn key_event(code: KeyCode, modifiers: KeyModifiers) -> KeyEvent { } } -fn key_parts(code: KeyCode, modifiers: KeyModifiers) -> (u32, u32) { +fn key_parts(code: KeyCode, modifiers: KeyModifiers) -> (FrontendKey, u32) { match map_key_event(key_event(code, modifiers)).expect("key event") { - InputEvent::Key { - keysym, modifiers, .. - } => (keysym, modifiers), + InputEvent::Key { key, modifiers, .. } => (key, modifiers), _ => panic!("expected key event"), } } @@ -48,7 +47,7 @@ fn tty_control_digit_aliases_are_raw_control_bytes() { for (input, expected) in cases { let (keysym, modifiers) = key_parts(KeyCode::Char(input), KeyModifiers::CONTROL); - assert_eq!(keysym, expected, "input C-{input}"); + assert_eq!(keysym, FrontendKey::Keysym(expected), "input C-{input}"); assert_eq!(modifiers & NEOMACS_CTRL_MASK, 0, "input C-{input}"); } } @@ -60,7 +59,7 @@ fn tty_meta_control_alias_preserves_meta_only() { KeyModifiers::CONTROL | KeyModifiers::ALT, ); - assert_eq!(keysym, 0x1c); + assert_eq!(keysym, FrontendKey::Keysym(0x1c)); assert_eq!(modifiers & NEOMACS_CTRL_MASK, 0); assert_ne!(modifiers & NEOMACS_META_MASK, 0); } @@ -69,6 +68,20 @@ fn tty_meta_control_alias_preserves_meta_only() { fn tty_backspace_is_raw_del_byte() { let (keysym, modifiers) = key_parts(KeyCode::Backspace, KeyModifiers::ALT); - assert_eq!(keysym, 0x7f); + assert_eq!(keysym, FrontendKey::Keysym(0x7f)); assert_ne!(modifiers & NEOMACS_META_MASK, 0); } + +#[test] +fn crossterm_fullwidth_characters_reach_core_as_text() { + for character in ",();-ヲᅧ".chars() { + let (key, modifiers) = key_parts(KeyCode::Char(character), KeyModifiers::NONE); + let event = + neovm_core::keyboard::render_key_transport_to_input_event(key, modifiers, true, 0) + .unwrap(); + assert!( + matches!(event, neovm_core::keyboard::InputEvent::KeyPress { key, .. } + if key.key == neovm_core::keyboard::Key::Char(character)) + ); + } +} diff --git a/crates/neomacs-gui-tests/fixtures/issue-458-fcitx-profile b/crates/neomacs-gui-tests/fixtures/issue-458-fcitx-profile new file mode 100644 index 0000000000..8359458d19 --- /dev/null +++ b/crates/neomacs-gui-tests/fixtures/issue-458-fcitx-profile @@ -0,0 +1,13 @@ +[Groups/0] +Name=Default +Default Layout=us +DefaultIM=pinyin + +[Groups/0/Items/0] +Name=keyboard-us + +[Groups/0/Items/1] +Name=pinyin + +[GroupOrder] +0=Default diff --git a/crates/neomacs-gui-tests/fixtures/issue-458-ime.el b/crates/neomacs-gui-tests/fixtures/issue-458-ime.el new file mode 100644 index 0000000000..a12183f016 --- /dev/null +++ b/crates/neomacs-gui-tests/fixtures/issue-458-ime.el @@ -0,0 +1,13 @@ +;;; issue-458-ime.el --- Native XIM regression -*- lexical-binding: t -*- +(load (expand-file-name "native-frame-focus.el" + (file-name-directory load-file-name)) nil t) +(global-set-key [f13] + (lambda () (interactive) + (with-current-buffer "*focus-secondary*" (insert "F13")))) +(advice-add 'neomacs-focus-tick :before + (lambda () + (when (and (file-exists-p + (expand-file-name "stop" neomacs-focus-directory)) + (fboundp 'neomacs--write-frame-snapshot)) + (neomacs--write-frame-snapshot + (getenv "NEOMACS_GUI_FRAME_SNAPSHOT_TXT") t 'text)))) diff --git a/crates/neomacs-gui-tests/tests/harness_contract.rs b/crates/neomacs-gui-tests/tests/harness_contract.rs index f1df87d8da..215ce6073d 100644 --- a/crates/neomacs-gui-tests/tests/harness_contract.rs +++ b/crates/neomacs-gui-tests/tests/harness_contract.rs @@ -297,6 +297,24 @@ fn x11_session_owns_authenticated_tcp_display_below_artifact_root() { assert!(display.starts_with("127.0.0.1:"), "DISPLAY was {display}"); assert!(authority.starts_with(&root)); assert!(authority.is_file()); + for name in ["WAYLAND_DISPLAY", "WAYLAND_SOCKET"] { + assert_eq!( + session + .env() + .iter() + .find_map(|(key, value)| (key == name).then_some(value.as_str())), + Some(""), + "Xvfb clients must not inherit the user's Wayland connection" + ); + } + assert_eq!( + session + .env() + .iter() + .find_map(|(key, value)| (key == "GDK_BACKEND").then_some(value.as_str())), + Some("x11"), + "GTK clients must use the owned X11 display" + ); let owned_session_root = authority .parent() .expect("Xauthority is below an owned session root") diff --git a/crates/neomacs-gui-tests/tests/native_frame_focus.rs b/crates/neomacs-gui-tests/tests/native_frame_focus.rs index 79c5d3d609..5567d50c54 100644 --- a/crates/neomacs-gui-tests/tests/native_frame_focus.rs +++ b/crates/neomacs-gui-tests/tests/native_frame_focus.rs @@ -243,3 +243,6 @@ fn xdotool(env: &[(String, String)], args: &[&str]) -> Result { } Ok(String::from_utf8_lossy(&output.stdout).into_owned()) } + +#[path = "native_frame_focus/ime.rs"] +mod ime; diff --git a/crates/neomacs-gui-tests/tests/native_frame_focus/ime.rs b/crates/neomacs-gui-tests/tests/native_frame_focus/ime.rs new file mode 100644 index 0000000000..725781d09d --- /dev/null +++ b/crates/neomacs-gui-tests/tests/native_frame_focus/ime.rs @@ -0,0 +1,227 @@ +//! Issue #458: real fcitx5 commits must stay text, including keysym collisions. +use super::*; +use std::io::{BufRead, BufReader}; +use std::process::{Child, Stdio}; + +struct ChildGuard(Child); +impl Drop for ChildGuard { + fn drop(&mut self) { + let _ = self.0.kill(); + let _ = self.0.wait(); + } +} + +#[test] +fn fcitx5_fullwidth_punctuation_is_text_and_f13_stays_a_key() { + if std::env::var("NEOMACS_GUI_TEST_BACKEND").ok().as_deref() != Some("x11") { + eprintln!( + "set NEOMACS_GUI_TEST_BACKEND=x11; requires fcitx5 with pinyin and unicode, dbus-daemon and xdotool" + ); + return; + } + let root = neomacs_infra::workspace_root(); + let artifacts = root.join(format!( + "target/neomacs-gui-tests/issue-458-{}", + std::process::id() + )); + fs::create_dir_all(&artifacts).unwrap(); + let session = DisplayHarness::Xvfb.start_session(&artifacts).unwrap(); + wait_for_x11(session.env()).unwrap(); + let config = artifacts.join("config"); + fs::create_dir_all(config.join("fcitx5/conf")).unwrap(); + fs::write( + config.join("fcitx5/profile"), + include_str!("../../fixtures/issue-458-fcitx-profile"), + ) + .unwrap(); + // Pinyin's default semicolon starts quickphrase instead of punctuation. + fs::write( + config.join("fcitx5/conf/pinyin.conf"), + "QuickPhraseKey=\nCloudPinyinEnabled=False\n", + ) + .unwrap(); + fs::write( + config.join("fcitx5/conf/unicode.conf"), + "[DirectUnicodeMode]\n0=Control+Shift+U\n", + ) + .unwrap(); + let runtime = artifacts.join("runtime"); + fs::create_dir(&runtime).unwrap(); + use std::os::unix::fs::PermissionsExt; + fs::set_permissions(&runtime, fs::Permissions::from_mode(0o700)).unwrap(); + let mut env = session.env().to_vec(); + env.extend([ + ("XDG_CONFIG_HOME".into(), config.display().to_string()), + ("XDG_RUNTIME_DIR".into(), runtime.display().to_string()), + ("XMODIFIERS".into(), "@im=fcitx".into()), + ("GTK_IM_MODULE".into(), "fcitx".into()), + ]); + let mut bus = ChildGuard( + Command::new("dbus-daemon") + .args(["--session", "--nofork", "--print-address=1"]) + .envs(env.iter().map(|(k, v)| (k, v))) + .stdout(Stdio::piped()) + .spawn() + .expect("private D-Bus daemon"), + ); + let mut address = String::new(); + BufReader::new(bus.0.stdout.take().unwrap()) + .read_line(&mut address) + .unwrap(); + env.push(("DBUS_SESSION_BUS_ADDRESS".into(), address.trim().into())); + let log = fs::File::create(artifacts.join("fcitx.log")).unwrap(); + let _fcitx = ChildGuard( + Command::new("fcitx5") + .args([ + "-D", + "--disable=wayland,notificationitem,notifications,kimpanel", + ]) + .envs(env.iter().map(|(k, v)| (k, v))) + .stdout(Stdio::null()) + .stderr(log) + .spawn() + .expect("fcitx5"), + ); + let deadline = Instant::now() + Duration::from_secs(8); + loop { + let ready = Command::new("fcitx5-remote") + .arg("--check") + .envs(env.iter().map(|(k, v)| (k, v))) + .output() + .unwrap() + .status + .success(); + if ready { + break; + } + assert!( + Instant::now() < deadline, + "fcitx5 did not start; see {}", + artifacts.display() + ); + thread::sleep(Duration::from_millis(20)); + } + let binary = std::env::var_os("NEOMACS_GUI_TEST_BINARY") + .map(PathBuf::from) + .unwrap_or_else(|| root.join("target/release/neomacs")); + let is_neomacs = binary.file_name().is_some_and(|name| name == "neomacs"); + let mut plan = GuiTestPlan::new( + GuiBackend::LinuxX11, + &root, + &artifacts, + GuiScenario::new( + "issue-458", + root.join("crates/neomacs-gui-tests/fixtures/issue-458-ime.el"), + ), + ) + .with_program(binary) + .with_env("NEOMACS_GUI_FOCUS_CONTROL", artifacts.display().to_string()) + // Capture later input/redisplay frames, not just the empty startup frame. + .with_env("NEOMACS_DEBUG_SURFACE_READBACK", "512"); + for (key, value) in &env { + plan = plan.with_env(key, value); + } + let (run, observation) = thread::scope(|scope| { + let run = scope.spawn(|| { + plan.run_with( + &mut ProcessGuiCommandRunner, + GuiRunOptions::with_timeout(Duration::from_secs(45)), + ) + }); + let observation = (|| { + let ready = wait_for_state(&artifacts, "ready", 0, &|| run.is_finished())?; + let window = + wait_for_window(&env, &ready["pid"].to_string(), None, &|| run.is_finished())?; + focus_window(&env, &window)?; + for args in [["-s", "pinyin"].as_slice(), ["-o"].as_slice()] { + let output = Command::new("fcitx5-remote") + .args(args) + .envs(env.iter().map(|(k, v)| (k, v))) + .output() + .map_err(|e| e.to_string())?; + if !output.status.success() { + return Err(format!("fcitx5-remote {args:?}: {output:?}")); + } + } + // Confirm the active IM instead of treating absent IME as a pass. + let active = Command::new("fcitx5-remote") + .arg("-n") + .envs(env.iter().map(|(k, v)| (k, v))) + .output() + .map_err(|e| e.to_string())?; + if String::from_utf8_lossy(&active.stdout).trim() != "pinyin" { + return Err(format!("Pinyin not active: {active:?}")); + } + // ASCII comma is transformed by fcitx5 into an actual U+FF0C commit. + xdotool(&env, &["key", "comma"])?; + expect_buffers(&artifacts, "comma", ",", "", &|| run.is_finished())?; + // '(' and ')' are committed by Pinyin, exercising real named-key collisions. + xdotool(&env, &["key", "shift+9", "shift+0", "semicolon"])?; + expect_buffers(&artifacts, "punctuation", ",();", "", &|| { + run.is_finished() + })?; + // Commit colliding and non-Latin scalars through fcitx itself. + // This avoids synthetic Unicode keysyms: text and key identity + // are the distinction under test, and GNU GTK can classify an + // injected keysym differently from an input-method commit. + let mut expected = String::from(",();"); + for (hex, character) in [ + ("ffca", 'ᅧ'), + ("ff0d", '-'), + ("ff66", 'ヲ'), + ("4e2d", '中'), + ("3042", 'あ'), + ("d55c", '한'), + ] { + xdotool(&env, &["key", "ctrl+shift+u"])?; + type_text(&env, hex)?; + xdotool(&env, &["key", "Return"])?; + expected.push(character); + expect_buffers(&artifacts, &format!("ime-{hex}"), &expected, "", &|| { + run.is_finished() + })?; + } + let inactive = Command::new("fcitx5-remote") + .arg("-c") + .envs(env.iter().map(|(k, v)| (k, v))) + .output() + .map_err(|e| e.to_string())?; + if !inactive.status.success() { + return Err(format!("fcitx5 deactivate: {inactive:?}")); + } + xdotool(&env, &["key", "F13"])?; + expect_buffers( + &artifacts, + "f13", + ",();ᅧ-ヲ中あ한", + "F13", + &|| run.is_finished(), + )?; + Ok::<_, String>(()) + })(); + fs::write(artifacts.join("stop"), "stop").unwrap(); + (run.join().unwrap(), observation) + }); + let run = run.unwrap(); + assert!( + observation.is_ok(), + "{observation:?}; artifacts: {}", + artifacts.display() + ); + assert!(!run.timed_out, "{run:#?}"); + assert_eq!(run.exit_code, Some(0), "{run:#?}"); + if is_neomacs { + let snapshot = + fs::read_to_string(&run.artifacts.frame_snapshot_json).expect("GUI snapshot"); + let text = + fs::read_to_string(&run.artifacts.frame_snapshot_txt).expect("GUI text snapshot"); + assert!( + text.contains(",();ᅧ-ヲ中あ한"), + "committed text must be visible: {text}" + ); + assert!( + snapshot.contains(','), + "committed text must reach redisplay" + ); + } +} diff --git a/crates/neomacs-infra/src/display.rs b/crates/neomacs-infra/src/display.rs index 55140b69a0..56dc103dd1 100644 --- a/crates/neomacs-infra/src/display.rs +++ b/crates/neomacs-infra/src/display.rs @@ -547,6 +547,12 @@ fn start_xvfb_on( return Ok(pending.into_session(vec![ ("DISPLAY".to_string(), display), ("XAUTHORITY".to_string(), path_to_string(&authority_path)), + // Winit 0.31 chooses Wayland before X11 and no longer reads + // WINIT_UNIX_BACKEND. An inherited desktop socket must never + // route private Xvfb tests onto the user's display. + ("WAYLAND_DISPLAY".to_string(), String::new()), + ("WAYLAND_SOCKET".to_string(), String::new()), + ("GDK_BACKEND".to_string(), "x11".to_string()), locale_pin(), ])); } diff --git a/crates/neomacs-tui-tests/tests/editing.rs b/crates/neomacs-tui-tests/tests/editing.rs index 238044f17f..fc43f6e27c 100644 --- a/crates/neomacs-tui-tests/tests/editing.rs +++ b/crates/neomacs-tui-tests/tests/editing.rs @@ -215,3 +215,25 @@ fn delete_char_via_cd_removes_character_after_point() { &neo, ); } + +/// Issue #458: UTF-8 terminal input already carries character identity. +/// Protect it while changing the GUI keyboard transport. +#[test] +fn fullwidth_punctuation_self_inserts_like_gnu() { + let (mut gnu, mut neo) = boot_pair(""); + open_home_file(&mut gnu, &mut neo, "fullwidth.txt", "", "C-x C-f"); + let expected = ",();-ヲᅧ中あ한\n"; + gnu.send(expected.as_bytes()); + neo.send(expected.as_bytes()); + wait_for_both(&mut gnu, &mut neo, Duration::from_secs(8), |grid| { + grid.iter().any(|row| row.contains(expected.trim_end())) + }); + save_current_file_and_assert_contents( + "fullwidth_punctuation_self_inserts_like_gnu", + &mut gnu, + &mut neo, + "fullwidth.txt", + expected, + ); + assert_pair_exact_display("fullwidth_punctuation_self_inserts_like_gnu", &gnu, &neo); +} diff --git a/crates/neomacs/src/bin/mock-display.rs b/crates/neomacs/src/bin/mock-display.rs index 17bf4100be..f2150e13d9 100644 --- a/crates/neomacs/src/bin/mock-display.rs +++ b/crates/neomacs/src/bin/mock-display.rs @@ -229,8 +229,12 @@ fn run_gui(demo: &str) { loop { std::thread::sleep(Duration::from_millis(100)); while let Ok(event) = emacs_comms.input_rx.try_recv() { - if let InputEvent::Key { keysym, .. } = event - && (keysym == b'q' as u32 || keysym == 0xff1b) + if let InputEvent::Key { key, .. } = event + && matches!( + key, + neovm_core::keyboard::FrontendKey::Character('q') + | neovm_core::keyboard::FrontendKey::Keysym(0xff1b) + ) { let _ = emacs_comms .cmd_tx diff --git a/crates/neomacs/src/input_bridge.rs b/crates/neomacs/src/input_bridge.rs index 772e2dd53e..6c1fc5579a 100644 --- a/crates/neomacs/src/input_bridge.rs +++ b/crates/neomacs/src/input_bridge.rs @@ -351,19 +351,19 @@ fn convert_single_display_event(event: &DisplayEvent) -> Option { KbInputEvent::raw_tty_bytes(bytes.clone(), *emacs_frame_id) }), DisplayEvent::Key { - keysym, + key, modifiers, pressed, emacs_frame_id, } => { tracing::debug!( - "input_bridge: key keysym=0x{:04x} mods=0x{:x} pressed={}", - *keysym, + "input_bridge: key={:?} mods=0x{:x} pressed={}", + *key, *modifiers, *pressed ); let event = keyboard::render_key_transport_to_input_event( - *keysym, + *key, *modifiers, *pressed, *emacs_frame_id, diff --git a/crates/neomacs/src/input_bridge/tests/mod.rs b/crates/neomacs/src/input_bridge/tests/mod.rs index 5882d8b348..d82545ea2f 100644 --- a/crates/neomacs/src/input_bridge/tests/mod.rs +++ b/crates/neomacs/src/input_bridge/tests/mod.rs @@ -204,7 +204,7 @@ fn presentation_lifecycle_events_reach_the_evaluator_losslessly() { #[test] fn key_release_is_dropped_by_core_transport_owner() { let display_event = DisplayEvent::Key { - keysym: keyboard::XK_RETURN, + key: keyboard::FrontendKey::Keysym(keyboard::XK_RETURN), modifiers: 0, pressed: false, emacs_frame_id: 0, @@ -254,7 +254,7 @@ fn raw_tty_bytes_cross_the_bridge_without_interpretation() { #[test] fn key_transport_preserves_source_frame_identity() { let display_event = DisplayEvent::Key { - keysym: 'a' as u32, + key: keyboard::FrontendKey::Character('a'), modifiers: keyboard::RENDER_CTRL_MASK, pressed: true, emacs_frame_id: 42, diff --git a/crates/neovm-core/src/keyboard.rs b/crates/neovm-core/src/keyboard.rs index 97c645801e..db25139513 100644 --- a/crates/neovm-core/src/keyboard.rs +++ b/crates/neovm-core/src/keyboard.rs @@ -1112,12 +1112,20 @@ pub fn render_modifiers_to_modifiers(bits: u32) -> Modifiers { } } -/// Convert frontend key transport facts into the core input event model. -/// -/// Key releases are ignored here so the command loop only sees the GNU-like -/// cooked keypress stream. +/// Toolkit input identity. Text and keysyms overlap numerically, so their +/// provenance must survive transport (U+FF0D is text; XK_Return is a key). +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum FrontendKey { + /// A Unicode scalar supplied as text by the toolkit or input method. + Character(char), + /// A key identity in the X11/native keysym domain, not a code point. + Keysym(u32), +} + +/// Cook frontend input without interpreting text as a keysym. +/// Key releases are ignored; frame identity and command modifiers are retained. pub fn render_key_transport_to_input_event( - keysym: u32, + key: FrontendKey, modifiers: u32, pressed: bool, emacs_frame_id: u64, @@ -1126,7 +1134,13 @@ pub fn render_key_transport_to_input_event( return None; } - let key_event = keysym_to_key_event(keysym, modifiers)?; + let key_event = match key { + FrontendKey::Character(character) => { + FrontendCharacterInput::classify(character, render_modifiers_to_modifiers(modifiers)) + .into_key_event() + } + FrontendKey::Keysym(keysym) => keysym_to_key_event(keysym, modifiers)?, + }; Some(InputEvent::key_press_in_frame(key_event, emacs_frame_id)) } diff --git a/crates/neovm-core/src/keyboard/keysym.rs b/crates/neovm-core/src/keyboard/keysym.rs index dc0811deee..31be6d1fb3 100644 --- a/crates/neovm-core/src/keyboard/keysym.rs +++ b/crates/neovm-core/src/keyboard/keysym.rs @@ -2,7 +2,8 @@ //! port gives a native key that has no keysym, and the name lookup that keeps //! such a key bindable. //! -//! GNU decides what a keystroke is by *range* before anything else. Its +//! For input supplied as a keysym (rather than committed text), GNU decides +//! what a keystroke is by *range*. Its //! backends classify with the X protocol's own macros (`IsCursorKey`, //! `IsMiscFunctionKey`, `IsKeypadKey`, `IsFunctionKey`, //! `src/pgtkterm.c:5218-5221`), and `keyboard.c`'s `modify_event_symbol` diff --git a/crates/neovm-core/src/keyboard/tests/mod.rs b/crates/neovm-core/src/keyboard/tests/mod.rs index e75d6693a5..4be1aa30ae 100644 --- a/crates/neovm-core/src/keyboard/tests/mod.rs +++ b/crates/neovm-core/src/keyboard/tests/mod.rs @@ -1777,7 +1777,9 @@ fn render_modifiers_helper_matches_transport_bit_layout() { #[test] fn render_key_transport_drops_key_releases() { crate::test_utils::init_test_tracing(); - assert!(render_key_transport_to_input_event(XK_RETURN, 0, false, 0).is_none()); + assert!( + render_key_transport_to_input_event(FrontendKey::Keysym(XK_RETURN), 0, false, 0).is_none() + ); } #[test] @@ -2308,3 +2310,42 @@ fn display_idle_maintenance_yields_to_input_and_avoids_nested_or_timed_reads() { assert_eq!(calls.get(), 1); assert!(eval.display_idle_maintenance_fn.is_some()); } + +/// Issue #458: text retains its domain even where its number names a key. +#[test] +fn character_transport_preserves_fullwidth_and_halfwidth_text() { + for character in ",();-ヲᅧ\u{fd0e}中あ한😀".chars() { + let event = + render_key_transport_to_input_event(FrontendKey::Character(character), 0, true, 42) + .unwrap(); + assert!( + matches!(event, InputEvent::KeyPress { key, emacs_frame_id: 42 } + if key == KeyEvent::char(character)), + "{character:?}" + ); + } + assert!( + render_key_transport_to_input_event(FrontendKey::Character(','), 0, false, 42,).is_none() + ); +} + +#[test] +fn character_and_keysym_transport_keep_colliding_values_distinct() { + for (character, keysym, expected) in [ + ('(', XK_BACKSPACE, Key::Named(NamedKey::Backspace)), + (')', XK_TAB, Key::Named(NamedKey::Tab)), + ('-', XK_RETURN, Key::Named(NamedKey::Return)), + (';', XK_ESCAPE, Key::Named(NamedKey::Escape)), + ('ᅧ', 0xffca, Key::Named(NamedKey::F(13))), + ('ヲ', 0xff66, Key::Function("redo".into())), + ('\u{fd0e}', 0xfd0e, Key::Function("3270_Attn".into())), + ] { + assert!(matches!(render_key_transport_to_input_event( + FrontendKey::Keysym(keysym), 0, true, 7, + ), Some(InputEvent::KeyPress { key, emacs_frame_id: 7 }) if key.key == expected)); + assert!(matches!(render_key_transport_to_input_event( + FrontendKey::Character(character), RENDER_META_MASK, true, 7, + ), Some(InputEvent::KeyPress { key, emacs_frame_id: 7 }) + if key == KeyEvent::char_with_mods(character, Modifiers::meta()))); + } +} diff --git a/crates/neovm-oracle-tests/src/kbd_event_advanced.rs b/crates/neovm-oracle-tests/src/kbd_event_advanced.rs index ff2162d334..8d120a01fc 100644 --- a/crates/neovm-oracle-tests/src/kbd_event_advanced.rs +++ b/crates/neovm-oracle-tests/src/kbd_event_advanced.rs @@ -346,3 +346,20 @@ fn oracle_prop_kbd_event_modifiers_and_basic_type() { ]]; crate::common::assert_oracle_parity_expect(form, expect); } + +/// The Lisp-visible contract for issue #458: these are self-inserting +/// character events, distinct from the function keys with the same number. +#[test] +fn oracle_prop_kbd_event_fullwidth_punctuation_character_identity() { + return_if_neovm_enable_oracle_proptest_not_set!(); + assert_oracle_parity( + r#"(let ((text ",();-ヲᅧ中あ한")) + (list (string-to-list (kbd text)) + (mapcar (lambda (c) (list (event-basic-type c) + (event-modifiers c) + (key-binding (vector c)))) + (string-to-list text)) + (equal (kbd "ᅧ") (kbd "")) + (key-description (vector 'backspace 'tab 'return 'escape 'f13))))"#, + ); +} diff --git a/docs/design/input-keysyms.md b/docs/design/input-keysyms.md index 95e5a23625..7d791c8e46 100644 --- a/docs/design/input-keysyms.md +++ b/docs/design/input-keysyms.md @@ -19,6 +19,16 @@ every `NamedKey` winit's xkb keymap can produce. ## Keysym is not character +The frontend preserves this distinction with `FrontendKey::Character(char)` +and `FrontendKey::Keysym(u32)` inside `InputEvent::Key`. IME commits, +keyboard committed text, logical character keys, and crossterm characters +carry the character variant. Only actual key identities enter the keysym +classifier. This is GNU GTK's `xg_im_context_commit` separation between +decoded character events and function-key events, and prevents issue #458: +fullwidth punctuation must not become Backspace, Return, F13, or `key-N` +merely because its Unicode scalar has the same number as a keysym. See the +[investigation and test commands](../research/issue-458-text-and-keysym-identity.md). + The X11 keysym space is not the Unicode space, and the two overlap by number: `XK_F13` is 0xffca and U+FFCA is a halfwidth hangul letter; `XK_Redo` is 0xff66 and U+FF66 is a halfwidth katakana letter. A pipeline that decides "is diff --git a/docs/research/issue-458-text-and-keysym-identity.md b/docs/research/issue-458-text-and-keysym-identity.md new file mode 100644 index 0000000000..6cb29b9695 --- /dev/null +++ b/docs/research/issue-458-text-and-keysym-identity.md @@ -0,0 +1,109 @@ +# Issue #458: preserve text identity across frontend transport + +## Reproduction and cause + +Use only the reporter's [issue body](https://github.com/eval-exec/neomacs/issues/458). +A failing regression sent `translate_committed_text(",", 0)` through the core +keyboard boundary: expected `Key::Char(',')`, observed +`Key::Function("key-65292")`. This ran before production edits. A real Pinyin +commit under private Xvfb and fcitx5 also failed against the existing release +binary: the comma command reached Neomacs but inserted nothing. + +The input method supplies **text**, not an untyped X11 keysym. Both +`Ime::Commit` and keyboard committed-text translation cast each character to +`u32`, then transported it through the keysym classifier. The logical-key +fallback and the non-Unix crossterm mapper also discarded character identity. +This makes U+FF08 become Backspace, U+FF09 Tab, U+FF0D Return, U+FF1B Escape, +and U+FFCA F13. Recognizing unnamed printable registry holes would fix comma +but leave these collisions and modifier collisions unresolved. + +## GNU Emacs and primary sources + +Study the requested mirror at `/home/exec/Projects/github.com/emacs-mirror/emacs`: + +- `src/gtkutil.c:xg_im_context_commit` decodes UTF-8 into a + `MULTIBYTE_CHAR_KEYSTROKE_EVENT` with decoded text in `arg`; it does not feed + the character values into the function-key classifier. This is the + reporter's GNU GTK comparison path. +- `src/xterm.c`, KeyPress handling: `XLookupChars` clears the keysym and + modifiers; the Unicode keysym block is explicitly decoded before the + non-ASCII function-key classifier. Keysyms and committed strings remain + distinct inputs. A genuine bare `0xff0c` is therefore not proof that text + should be inferred from the registry; it is the transport provenance that + was lost in Neomacs. +- `src/keyboard.c:make_lispy_event` and `modify_event_symbol` cook character + events and name function-key events separately. + +The [X.Org XIM protocol, section 4.18](https://xorg.freedesktop.org/archive/X11R7.5/doc/libX11/xim.html) +separates `XLookupChars`, `XLookupKeySym`, and `XLookupBoth`: a commit can +carry a string, keysyms, or both. The +[winit IME contract](https://docs.rs/winit/latest/winit/event/enum.Ime.html) +defines `Commit` as text for insertion. These support preserving the toolkit's +text facts rather than reconstructing them from numeric ranges. +The [fcitx5 Unicode addon configuration](https://github.com/fcitx/fcitx5/blob/master/src/modules/unicode/unicode.h) +defines its direct Unicode entry shortcut; the native regression pins +`Control+Shift+U` in its private configuration to obtain genuine IME commits. + +## Design + +`FrontendKey` is a closed sum type: `Character(char)` or `Keysym(u32)`. +`InputEvent::Key` carries it end to end, alongside modifiers, press state, and +source frame. Rust's `char` excludes invalid Unicode scalars; exhaustive +matches force consumers to choose between text and key identity. No implicit +integer conversion erases this choice. + +The core cooks `Character` using the existing `FrontendCharacterInput` +modifier policy. `Keysym` retains existing GNU key naming, including F13–F35, +3270, ISO, modifier suppression, and unknown vendor keys. IME commits have +zero modifiers; ordinary keyboard text and logical command chords retain +their existing modifier policy. Modifier cooking selects ordinary policy +for characters even when their numeric values overlap function-key bands. +Unix TTY input remains raw bytes decoded by `keyboard-coding-system`; +non-Unix crossterm characters now use the same typed character transport. + +## Isolation and verification + +`neomacs-infra::display::start_xvfb` provides an authenticated loopback X11 +server. It now exports empty `WAYLAND_DISPLAY` and `WAYLAND_SOCKET`, because +winit 0.31 prefers inherited Wayland connections and no longer reads +`WINIT_UNIX_BACKEND`. It also pins `GDK_BACKEND=x11` for GTK clients. Failing +harness assertions first demonstrated that these isolation facts were absent. +The fcitx scenario owns its D-Bus session, +configuration, and runtime directory. It verifies the editor window's PID +on the private X11 server before sending XTEST input. + +Tests cover core character/key collisions and modifiers, frontend text +translation, GNU Lisp event parity, native fcitx Pinyin punctuation plus +fcitx Unicode commits and an F13 binding, final GUI redisplay snapshots, +and real UTF-8 TUI input compared to GNU display and saved text. + +Commands (the debug binary is rebuilt from this checkout): + +```sh +cargo build -p neomacs --bins +cargo test -p neovm-core --lib keyboard:: +cargo test -p neomacs-display-runtime --lib +cargo check -p neomacs-display-runtime --features webview +cargo test -p neomacs --lib input_bridge::tests:: +cargo test -p neomacs-gui-tests --test harness_contract +NEOVM_ENABLE_ORACLE_PROPTEST=1 NEOVM_BINARY_PATH="$PWD/target/debug/neomacs" \ + cargo test -p neovm-oracle-tests --lib oracle_prop_kbd_event_fullwidth_punctuation_character_identity +NEOMACS_TUI_NEOMACS_BIN="$PWD/target/debug/neomacs" \ + cargo test -p neomacs-tui-tests --test tui fullwidth_punctuation_self_inserts_like_gnu +NEOMACS_GUI_TEST_BACKEND=x11 NEOMACS_GUI_TEST_BINARY="$PWD/target/debug/neomacs" \ + cargo test -p neomacs-gui-tests --test native_frame_focus +``` + +The fcitx scenario requires fcitx5 with its Pinyin and Unicode addons, dbus-daemon, +xdotool, Xvfb, and xauth. It is opt-in with `NEOMACS_GUI_TEST_BACKEND=x11`. +Its fixture also works with `NEOMACS_GUI_TEST_BINARY=emacs` to check the same +native input against GNU Emacs. + +Verified on Linux/X11: 83 core keyboard tests, 1,073 runtime unit tests +(five existing opt-in tests ignored), 24 input-bridge tests, and 18 harness +contract tests passed. The enabled GNU oracle regression, paired GNU/Neomacs +TUI regression, and both native GUI tests passed. The fcitx GUI scenario also +passed against GNU Emacs 31.1. The crossterm mapper is exercised by unit tests; +native macOS and Windows were not run. The optional WebView feature compiles; +its numeric API conversion stays at the WebView boundary. Formatting and +whitespace checks passed.