From c133948a913d754d290e6d2ddac6e0710c77ae2c Mon Sep 17 00:00:00 2001 From: rustdesk Date: Wed, 23 Sep 2026 13:50:13 +0800 Subject: [PATCH] fix(keyboard): shortcuts, record the physical key The recording dialog and the Web Flutter matcher named keys by LogicalKeyboardKey, while the native matcher sees the physical key through its USB HID usage and the Web JS matcher through KeyboardEvent.code. On AZERTY, QWERTZ, Dvorak and similar layouts a recorded binding then did not fire, or fired from another key. Name keys by their physical position everywhere: physicalKeyName maps the USB HID usage to the stored name. A new fixture pins every name to its USB HID usage, checked on the Dart side and against rdev::usb_hid_key_from_code on the Rust side, and a test covers AZERTY and QWERTZ events. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Ra4my61t8q1FN5n59wB16D --- .../keyboard_shortcuts/recording_dialog.dart | 2 +- .../keyboard_shortcuts/shortcut_utils.dart | 115 ++++++------------ flutter/lib/models/input_model.dart | 2 +- .../test/fixtures/shortcut_key_usb_hid.json | 65 ++++++++++ flutter/test/keyboard_shortcuts_test.dart | 110 +++++++---------- src/keyboard/shortcuts.rs | 27 +++- 6 files changed, 174 insertions(+), 147 deletions(-) create mode 100644 flutter/test/fixtures/shortcut_key_usb_hid.json diff --git a/flutter/lib/common/widgets/keyboard_shortcuts/recording_dialog.dart b/flutter/lib/common/widgets/keyboard_shortcuts/recording_dialog.dart index e4c10eab3..9241113ab 100644 --- a/flutter/lib/common/widgets/keyboard_shortcuts/recording_dialog.dart +++ b/flutter/lib/common/widgets/keyboard_shortcuts/recording_dialog.dart @@ -167,7 +167,7 @@ class _RecordingDialogState extends State<_RecordingDialog> { // Ignore modifier-only KeyDowns: don't lock in a partial combo. final logical = event.logicalKey; - final keyName = logicalKeyName(logical); + final keyName = shortcutKeyNameForEvent(event); // Mirror of `normalize_modifiers` in src/keyboard/shortcuts.rs: // * macOS: Cmd → primary, Ctrl → ctrl (distinct). diff --git a/flutter/lib/common/widgets/keyboard_shortcuts/shortcut_utils.dart b/flutter/lib/common/widgets/keyboard_shortcuts/shortcut_utils.dart index 775bae0be..ba0481a69 100644 --- a/flutter/lib/common/widgets/keyboard_shortcuts/shortcut_utils.dart +++ b/flutter/lib/common/widgets/keyboard_shortcuts/shortcut_utils.dart @@ -40,88 +40,47 @@ bool isSwitchTabShortcutAction(String? actionId) { actionId == kShortcutActionSwitchTabPrev; } -/// Map a [LogicalKeyboardKey] to the canonical key name used in saved +/// USB HID usage (keyboard page 0x07) of every key accepted as a shortcut, +/// mapped to the canonical key name used in saved bindings. +const Map _kUsbHidKeyNames = { + 0x04: 'a', 0x05: 'b', 0x06: 'c', 0x07: 'd', 0x08: 'e', 0x09: 'f', + 0x0A: 'g', 0x0B: 'h', 0x0C: 'i', 0x0D: 'j', 0x0E: 'k', 0x0F: 'l', + 0x10: 'm', 0x11: 'n', 0x12: 'o', 0x13: 'p', 0x14: 'q', 0x15: 'r', + 0x16: 's', 0x17: 't', 0x18: 'u', 0x19: 'v', 0x1A: 'w', 0x1B: 'x', + 0x1C: 'y', 0x1D: 'z', + 0x1E: 'digit1', 0x1F: 'digit2', 0x20: 'digit3', 0x21: 'digit4', + 0x22: 'digit5', 0x23: 'digit6', 0x24: 'digit7', 0x25: 'digit8', + 0x26: 'digit9', 0x27: 'digit0', + 0x28: 'enter', 0x2A: 'backspace', 0x2B: 'tab', 0x2C: 'space', + 0x3A: 'f1', 0x3B: 'f2', 0x3C: 'f3', 0x3D: 'f4', 0x3E: 'f5', 0x3F: 'f6', + 0x40: 'f7', 0x41: 'f8', 0x42: 'f9', 0x43: 'f10', 0x44: 'f11', 0x45: 'f12', + 0x49: 'insert', 0x4A: 'home', 0x4B: 'page_up', 0x4C: 'delete', + 0x4D: 'end', 0x4E: 'page_down', 0x4F: 'arrow_right', 0x50: 'arrow_left', + 0x51: 'arrow_down', 0x52: 'arrow_up', + // Numpad Enter shares the "enter" name with the main Return key, as in the + // Rust matcher (`Return | KpReturn`) and the Web matcher (`NumpadEnter`). + 0x58: 'enter', +}; + +/// Map a [PhysicalKeyboardKey] to the canonical key name used in saved /// bindings, or `null` for keys we don't accept as shortcuts. /// -/// Mirror of `event_to_key_name` in `src/keyboard/shortcuts.rs` and -/// `logicalToKeyName` in `flutter/web/js/src/shortcut_matcher.ts` — keep -/// the three in lockstep. Cross-language parity is enforced by: -/// * `flutter/test/fixtures/supported_shortcut_keys.json` — the -/// authoritative list of names this function must produce. -/// * Dart `supported keys` test in `keyboard_shortcuts_test.dart` — -/// asserts the (LogicalKeyboardKey → name) mapping covers the fixture. -/// * Rust `supported_keys_match_fixture` test in `shortcuts.rs` — the -/// Rust-side mirror against the same fixture. -/// A drift in any of the three breaks one of the two tests. -String? logicalKeyName(LogicalKeyboardKey k) { - // Singletons that map 1:1. - if (k == LogicalKeyboardKey.delete) return 'delete'; - if (k == LogicalKeyboardKey.backspace) return 'backspace'; - // Numpad Enter shares the "enter" name with the main Return key — matches - // the Rust matcher (`Return | KpReturn` → "enter") and matches user - // expectation that the two physical Enters are interchangeable. - if (k == LogicalKeyboardKey.enter || k == LogicalKeyboardKey.numpadEnter) { - return 'enter'; - } - if (k == LogicalKeyboardKey.tab) return 'tab'; - if (k == LogicalKeyboardKey.space) return 'space'; - if (k == LogicalKeyboardKey.arrowLeft) return 'arrow_left'; - if (k == LogicalKeyboardKey.arrowRight) return 'arrow_right'; - if (k == LogicalKeyboardKey.arrowUp) return 'arrow_up'; - if (k == LogicalKeyboardKey.arrowDown) return 'arrow_down'; - if (k == LogicalKeyboardKey.home) return 'home'; - if (k == LogicalKeyboardKey.end) return 'end'; - if (k == LogicalKeyboardKey.pageUp) return 'page_up'; - if (k == LogicalKeyboardKey.pageDown) return 'page_down'; - if (k == LogicalKeyboardKey.insert) return 'insert'; - - // Letter / digit / F-key tables. `LogicalKeyboardKey` constants are - // `static final` (not `const`), so the maps can't be `const` — but they - // initialize once per process and the lookup is O(1). - final letters = { - LogicalKeyboardKey.keyA: 'a', LogicalKeyboardKey.keyB: 'b', - LogicalKeyboardKey.keyC: 'c', LogicalKeyboardKey.keyD: 'd', - LogicalKeyboardKey.keyE: 'e', LogicalKeyboardKey.keyF: 'f', - LogicalKeyboardKey.keyG: 'g', LogicalKeyboardKey.keyH: 'h', - LogicalKeyboardKey.keyI: 'i', LogicalKeyboardKey.keyJ: 'j', - LogicalKeyboardKey.keyK: 'k', LogicalKeyboardKey.keyL: 'l', - LogicalKeyboardKey.keyM: 'm', LogicalKeyboardKey.keyN: 'n', - LogicalKeyboardKey.keyO: 'o', LogicalKeyboardKey.keyP: 'p', - LogicalKeyboardKey.keyQ: 'q', LogicalKeyboardKey.keyR: 'r', - LogicalKeyboardKey.keyS: 's', LogicalKeyboardKey.keyT: 't', - LogicalKeyboardKey.keyU: 'u', LogicalKeyboardKey.keyV: 'v', - LogicalKeyboardKey.keyW: 'w', LogicalKeyboardKey.keyX: 'x', - LogicalKeyboardKey.keyY: 'y', LogicalKeyboardKey.keyZ: 'z', - }; - final letter = letters[k]; - if (letter != null) return letter; - - final digits = { - LogicalKeyboardKey.digit0: 'digit0', - LogicalKeyboardKey.digit1: 'digit1', - LogicalKeyboardKey.digit2: 'digit2', - LogicalKeyboardKey.digit3: 'digit3', - LogicalKeyboardKey.digit4: 'digit4', - LogicalKeyboardKey.digit5: 'digit5', - LogicalKeyboardKey.digit6: 'digit6', - LogicalKeyboardKey.digit7: 'digit7', - LogicalKeyboardKey.digit8: 'digit8', - LogicalKeyboardKey.digit9: 'digit9', - }; - final digit = digits[k]; - if (digit != null) return digit; - - final fkeys = { - LogicalKeyboardKey.f1: 'f1', LogicalKeyboardKey.f2: 'f2', - LogicalKeyboardKey.f3: 'f3', LogicalKeyboardKey.f4: 'f4', - LogicalKeyboardKey.f5: 'f5', LogicalKeyboardKey.f6: 'f6', - LogicalKeyboardKey.f7: 'f7', LogicalKeyboardKey.f8: 'f8', - LogicalKeyboardKey.f9: 'f9', LogicalKeyboardKey.f10: 'f10', - LogicalKeyboardKey.f11: 'f11', LogicalKeyboardKey.f12: 'f12', - }; - return fkeys[k]; +/// Bindings name physical key positions (US layout names), whatever the +/// active keyboard layout: the native matcher sees the key through its USB +/// HID usage (`rdev::usb_hid_key_from_code` -> `event_to_key_name` in +/// `src/keyboard/shortcuts.rs`) and the Web matcher through +/// `KeyboardEvent.code` (`flutter/web/js/src/shortcut_matcher.ts`). Parity +/// is enforced against `flutter/test/fixtures/shortcut_key_usb_hid.json` by +/// a Dart test and the Rust `usb_hid_keys_match_fixture` test. +String? physicalKeyName(PhysicalKeyboardKey k) { + final usage = k.usbHidUsage; + if (usage >> 16 != 0x07) return null; + return _kUsbHidKeyNames[usage & 0xFFFF]; } +/// The key name a [KeyEvent] records or matches as. +String? shortcutKeyNameForEvent(KeyEvent e) => physicalKeyName(e.physicalKey); + /// Bundle of "is this shortcut available on the current platform" flags. /// /// Production code reaches a single source of truth via diff --git a/flutter/lib/models/input_model.dart b/flutter/lib/models/input_model.dart index e7f2ea0d6..dff34d9c1 100644 --- a/flutter/lib/models/input_model.dart +++ b/flutter/lib/models/input_model.dart @@ -935,7 +935,7 @@ class InputModel { if (!ShortcutModel.isEnabled() || ShortcutModel.isPassThrough()) { return false; } - final keyName = logicalKeyName(e.logicalKey); + final keyName = shortcutKeyNameForEvent(e); if (keyName == null) return false; final mods = canonicalShortcutModsForSave(_webFlutterShortcutMods()); final action = _matchWebFlutterShortcut(keyName, mods); diff --git a/flutter/test/fixtures/shortcut_key_usb_hid.json b/flutter/test/fixtures/shortcut_key_usb_hid.json new file mode 100644 index 000000000..f85d99fe9 --- /dev/null +++ b/flutter/test/fixtures/shortcut_key_usb_hid.json @@ -0,0 +1,65 @@ +[ + ["a", 4], + ["b", 5], + ["c", 6], + ["d", 7], + ["e", 8], + ["f", 9], + ["g", 10], + ["h", 11], + ["i", 12], + ["j", 13], + ["k", 14], + ["l", 15], + ["m", 16], + ["n", 17], + ["o", 18], + ["p", 19], + ["q", 20], + ["r", 21], + ["s", 22], + ["t", 23], + ["u", 24], + ["v", 25], + ["w", 26], + ["x", 27], + ["y", 28], + ["z", 29], + ["digit1", 30], + ["digit2", 31], + ["digit3", 32], + ["digit4", 33], + ["digit5", 34], + ["digit6", 35], + ["digit7", 36], + ["digit8", 37], + ["digit9", 38], + ["digit0", 39], + ["f1", 58], + ["f2", 59], + ["f3", 60], + ["f4", 61], + ["f5", 62], + ["f6", 63], + ["f7", 64], + ["f8", 65], + ["f9", 66], + ["f10", 67], + ["f11", 68], + ["f12", 69], + ["delete", 76], + ["backspace", 42], + ["tab", 43], + ["space", 44], + ["enter", 40], + ["enter", 88], + ["arrow_left", 80], + ["arrow_right", 79], + ["arrow_up", 82], + ["arrow_down", 81], + ["home", 74], + ["end", 77], + ["page_up", 75], + ["page_down", 78], + ["insert", 73] +] diff --git a/flutter/test/keyboard_shortcuts_test.dart b/flutter/test/keyboard_shortcuts_test.dart index a1ac38eac..e5c15154b 100644 --- a/flutter/test/keyboard_shortcuts_test.dart +++ b/flutter/test/keyboard_shortcuts_test.dart @@ -367,74 +367,56 @@ void main() { } }); - test('logicalKeyName covers the supported-keys fixture', () { - // The fixture is the cross-language source of truth for the full set of - // shortcut-bindable key names. Rust has a mirror test against the same - // file (`supported_keys_match_fixture` in src/keyboard/shortcuts.rs). - // Drift on either side breaks one of the two tests. - final fixturePath = 'test/fixtures/supported_shortcut_keys.json'; - final fixture = - (jsonDecode(File(fixturePath).readAsStringSync()) as List) - .cast() - .toSet(); + test('physicalKeyName covers the supported-keys fixture', () { + // `shortcut_key_usb_hid.json` pins every key name to the USB HID usage + // it is recorded and matched from. Rust has a mirror test against the + // same file (`usb_hid_keys_match_fixture` in src/keyboard/shortcuts.rs), + // and the set of names must equal `supported_shortcut_keys.json`. + final supported = (jsonDecode( + File('test/fixtures/supported_shortcut_keys.json') + .readAsStringSync()) as List) + .cast() + .toSet(); + final pairs = (jsonDecode(File('test/fixtures/shortcut_key_usb_hid.json') + .readAsStringSync()) as List) + .cast>(); - // Hand-rolled (LogicalKeyboardKey, name) round-trip table. Adding a key - // requires updates in three places: the fixture, this table, and Rust's - // matching table — that's the price of the parity guarantee. - final mappings = <(LogicalKeyboardKey, String)>[ - for (var c = 0; c < 26; c++) - ( - LogicalKeyboardKey(0x00000000061 + c), - String.fromCharCode(0x61 + c), - ), - for (var d = 0; d < 10; d++) - (LogicalKeyboardKey(0x00000000030 + d), 'digit$d'), - (LogicalKeyboardKey.f1, 'f1'), - (LogicalKeyboardKey.f2, 'f2'), - (LogicalKeyboardKey.f3, 'f3'), - (LogicalKeyboardKey.f4, 'f4'), - (LogicalKeyboardKey.f5, 'f5'), - (LogicalKeyboardKey.f6, 'f6'), - (LogicalKeyboardKey.f7, 'f7'), - (LogicalKeyboardKey.f8, 'f8'), - (LogicalKeyboardKey.f9, 'f9'), - (LogicalKeyboardKey.f10, 'f10'), - (LogicalKeyboardKey.f11, 'f11'), - (LogicalKeyboardKey.f12, 'f12'), - (LogicalKeyboardKey.delete, 'delete'), - (LogicalKeyboardKey.backspace, 'backspace'), - (LogicalKeyboardKey.tab, 'tab'), - (LogicalKeyboardKey.space, 'space'), - (LogicalKeyboardKey.enter, 'enter'), - (LogicalKeyboardKey.numpadEnter, 'enter'), - (LogicalKeyboardKey.arrowLeft, 'arrow_left'), - (LogicalKeyboardKey.arrowRight, 'arrow_right'), - (LogicalKeyboardKey.arrowUp, 'arrow_up'), - (LogicalKeyboardKey.arrowDown, 'arrow_down'), - (LogicalKeyboardKey.home, 'home'), - (LogicalKeyboardKey.end, 'end'), - (LogicalKeyboardKey.pageUp, 'page_up'), - (LogicalKeyboardKey.pageDown, 'page_down'), - (LogicalKeyboardKey.insert, 'insert'), - ]; - - // Round-trip: every (key, name) pair must agree with logicalKeyName. - for (final (key, name) in mappings) { - expect(logicalKeyName(key), equals(name), - reason: 'logicalKeyName($key) should be "$name"'); + for (final pair in pairs) { + final name = pair[0] as String; + final usage = pair[1] as int; + expect(physicalKeyName(PhysicalKeyboardKey(0x00070000 | usage)), + equals(name), + reason: 'USB HID 0x${usage.toRadixString(16)} should be "$name"'); } - - // The set of names produced by the table must equal the fixture. - final namesFromTable = mappings.map((e) => e.$2).toSet(); - expect(namesFromTable, equals(fixture), - reason: 'logicalKeyName vocabulary drifted from $fixturePath — update ' - 'shortcut_utils.dart::logicalKeyName, the fixture, and Rust ' - 'event_to_key_name together'); + expect(pairs.map((p) => p[0] as String).toSet(), equals(supported)); // Modifier-only / unsupported keys must return null. - expect(logicalKeyName(LogicalKeyboardKey.shift), isNull); - expect(logicalKeyName(LogicalKeyboardKey.escape), isNull); - expect(logicalKeyName(LogicalKeyboardKey.f13), isNull); + expect(physicalKeyName(PhysicalKeyboardKey.shiftLeft), isNull); + expect(physicalKeyName(PhysicalKeyboardKey.escape), isNull); + expect(physicalKeyName(PhysicalKeyboardKey.f13), isNull); + expect(physicalKeyName(PhysicalKeyboardKey.numpad1), isNull); + }); + + test('non-US layouts record and match the physical key', () { + // AZERTY: the key labelled "A" sits where US QWERTY has Q. The native + // matcher only sees the physical position (USB HID usage), so the + // binding must name that position too. + const azertyA = KeyDownEvent( + physicalKey: PhysicalKeyboardKey.keyQ, + logicalKey: LogicalKeyboardKey.keyA, + character: 'a', + timeStamp: Duration.zero, + ); + expect(shortcutKeyNameForEvent(azertyA), 'q'); + + // QWERTZ: the key labelled "Z" sits where US QWERTY has Y. + const qwertzZ = KeyDownEvent( + physicalKey: PhysicalKeyboardKey.keyY, + logicalKey: LogicalKeyboardKey.keyZ, + character: 'z', + timeStamp: Duration.zero, + ); + expect(shortcutKeyNameForEvent(qwertzZ), 'y'); }); test('configurable shortcut list does not include known-removed action IDs', diff --git a/src/keyboard/shortcuts.rs b/src/keyboard/shortcuts.rs index cf40b9f09..db5bdcc72 100644 --- a/src/keyboard/shortcuts.rs +++ b/src/keyboard/shortcuts.rs @@ -676,11 +676,11 @@ mod tests { /// names (not just the defaults). The fixture lists every name the /// matcher accepts; this test verifies the (rdev::Key → name) round-trip /// covers exactly that set. Dart has a mirror test against the same - /// fixture (`logicalKeyName covers the supported-keys fixture` in + /// fixture (`physicalKeyName covers the supported-keys fixture` in /// `flutter/test/keyboard_shortcuts_test.dart`). /// /// Adding a key requires updates in three places: the fixture, this - /// table, and the Dart `logicalKeyName` — that's the price of the + /// table, and the Dart `physicalKeyName` — that's the price of the /// parity guarantee. Drift on any side breaks one of the two tests. #[test] fn supported_keys_match_fixture() { @@ -746,10 +746,31 @@ mod tests { actual, expected, "event_to_key_name vocabulary drifted from \ flutter/test/fixtures/supported_shortcut_keys.json — update \ - shortcuts.rs, the fixture, and Dart logicalKeyName together" + shortcuts.rs, the fixture, and Dart physicalKeyName together" ); } + /// The native Flutter path turns a key into a name through its USB HID + /// usage (`rdev::usb_hid_key_from_code` -> `event_to_key_name`); the Dart + /// recorder does the same through `physicalKeyName`. Both are checked + /// against the same fixture, so a binding recorded on any keyboard layout + /// matches the key that was pressed. + #[test] + fn usb_hid_keys_match_fixture() { + let pairs: Vec<(String, u32)> = serde_json::from_str(include_str!( + "../../flutter/test/fixtures/shortcut_key_usb_hid.json" + )) + .expect("parse fixture"); + for (name, usage) in pairs { + let key = rdev::usb_hid_key_from_code(usage); + assert_eq!( + event_to_key_name(&make_press(key)).as_deref(), + Some(name.as_str()), + "USB HID {usage:#04x}" + ); + } + } + /// Serializes the tests that write the global `CACHE`. static CACHE_TEST_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());