From d4db3db5cb8aeeff2dc56484bc3fa4470a90b43c Mon Sep 17 00:00:00 2001 From: Eval Exec Date: Sun, 4 Oct 2026 13:16:30 +0800 Subject: [PATCH] fix(input): preserve fullwidth IME text identity (#458) Fullwidth punctuation committed by fcitx5 was transported as an untyped integer and then interpreted as an X11 keysym. U+FF0C consequently became the undefined key-65292 event. Other Unicode scalars overlap named keysyms, including Backspace, Tab, Return, Escape, F13, and Redo, so recognizing unnamed registry holes alone would leave those collisions unresolved. Introduce FrontendKey::Character(char) and FrontendKey::Keysym(u32) and preserve that distinction from frontend input through the core keyboard boundary. IME commits, keyboard committed text, logical character keys, and crossterm characters use the character variant. Genuine key identities retain existing keysym naming and function-key modifier handling. Keep raw control aliases and Unix TTY byte decoding intact, restrict scroll observation to keysyms, and convert numeric WebView input only at its API boundary. Rename the raw toolkit helper to translate_keysym. Follow GNU Emacs's separation of decoded IME text from function-key events. Document the GNU source investigation, primary-source research, typed transport design, and reproduction commands. Fix private Xvfb isolation in neomacs-infra by overriding inherited Wayland connection settings and pinning GTK to X11. The native fcitx regression owns its configuration, runtime directory, and D-Bus session and verifies the editor's PID and focus on the private display before sending input. Add regression coverage for the original comma failure, character/keysym collisions, logical and crossterm input, GNU Lisp event parity, paired GNU and Neomacs TUI insertion, and real fcitx commits alongside a physical F13 binding. Preserve final redisplay snapshots and capture later GUI frames. Fixes #458 --- .../src/render_thread/input.rs | 35 ++- .../src/render_thread/input/tests/mod.rs | 116 +++++---- .../src/render_thread/tests.rs | 65 +++-- .../src/render_thread/window_events.rs | 64 +++-- .../src/thread_comm.rs | 6 +- .../src/thread_comm/tests/mod.rs | 38 +-- .../neomacs-display-runtime/src/tty_input.rs | 45 ++-- .../src/tty_input/tests/tty_input_test.rs | 27 ++- .../fixtures/issue-458-fcitx-profile | 13 + .../fixtures/issue-458-ime.el | 13 + .../tests/harness_contract.rs | 18 ++ .../tests/native_frame_focus.rs | 3 + .../tests/native_frame_focus/ime.rs | 227 ++++++++++++++++++ crates/neomacs-infra/src/display.rs | 6 + crates/neomacs-tui-tests/tests/editing.rs | 22 ++ crates/neomacs/src/bin/mock-display.rs | 8 +- crates/neomacs/src/input_bridge.rs | 8 +- crates/neomacs/src/input_bridge/tests/mod.rs | 4 +- crates/neovm-core/src/keyboard.rs | 26 +- crates/neovm-core/src/keyboard/keysym.rs | 3 +- crates/neovm-core/src/keyboard/tests/mod.rs | 43 +++- .../src/kbd_event_advanced.rs | 17 ++ docs/design/input-keysyms.md | 10 + .../issue-458-text-and-keysym-identity.md | 109 +++++++++ 24 files changed, 740 insertions(+), 186 deletions(-) create mode 100644 crates/neomacs-gui-tests/fixtures/issue-458-fcitx-profile create mode 100644 crates/neomacs-gui-tests/fixtures/issue-458-ime.el create mode 100644 crates/neomacs-gui-tests/tests/native_frame_focus/ime.rs create mode 100644 docs/research/issue-458-text-and-keysym-identity.md 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.