From 1fbf3f5f7fa51eee9fe8101446d1f849a85f2603 Mon Sep 17 00:00:00 2001 From: rustdesk Date: Wed, 23 Sep 2026 20:24:15 +0800 Subject: [PATCH] fix(keyboard): shortcuts, run focus-changing actions on key release Close tab and tab switching move keyboard focus to another session before the key that fired them is released. On Linux the Flutter input source routes map-mode sessions through the Rust matcher and legacy-mode sessions through the Dart matcher, and neither knows the other's fired keys, so after such an action the repeats and the release of the held key reached the next session's remote, and the key stayed recorded as fired in the matcher that saw the press. Routing Linux legacy mode through the Rust FFI path instead is not a drop-in change: the raw key handler builds its event with no unicode, so Rust's legacy mode would fall back to US key names and lose the layout-aware characters the Dart path sends today, and it would take modifiers from the X11 key state instead of Flutter's. Run these three actions when the fired key is released. The matcher that consumed the press then also sees the repeats and the release, the remote modifiers are still released on the press, and a session that closes drops the action it owed. Both matchers keep the same list of actions. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Ra4my61t8q1FN5n59wB16D --- .../shortcut_constants.dart | 12 +++ .../lib/models/local_flutter_shortcuts.dart | 38 ++++++-- .../test/local_flutter_shortcuts_test.dart | 42 +++++++++ src/keyboard/shortcuts.rs | 87 ++++++++++++++++++- 4 files changed, 170 insertions(+), 9 deletions(-) diff --git a/flutter/lib/common/widgets/keyboard_shortcuts/shortcut_constants.dart b/flutter/lib/common/widgets/keyboard_shortcuts/shortcut_constants.dart index 3d265199d..a652dfc20 100644 --- a/flutter/lib/common/widgets/keyboard_shortcuts/shortcut_constants.dart +++ b/flutter/lib/common/widgets/keyboard_shortcuts/shortcut_constants.dart @@ -17,6 +17,18 @@ const kShortcutActionRestartRemote = 'restart_remote'; const kShortcutActionResetCanvas = 'reset_canvas'; const kShortcutActionSwitchTabNext = 'switch_tab_next'; const kShortcutActionSwitchTabPrev = 'switch_tab_prev'; + +/// Actions that move keyboard focus to another session. They run when the +/// key that fired them is released, not when it is pressed, so the matcher +/// that consumed the press also sees its repeats and its release. On Linux a +/// legacy-mode session matches in Dart while a map-mode session matches in +/// Rust, and neither knows the other's fired keys. Mirrors `runs_on_release` +/// in `src/keyboard/shortcuts.rs`. +const kShortcutActionsRunOnKeyUp = { + kShortcutActionCloseTab, + kShortcutActionSwitchTabNext, + kShortcutActionSwitchTabPrev, +}; const kShortcutActionToggleMute = 'toggle_mute'; const kShortcutActionPinToolbar = 'pin_toolbar'; const kShortcutActionViewModeOriginal = 'view_mode_original'; diff --git a/flutter/lib/models/local_flutter_shortcuts.dart b/flutter/lib/models/local_flutter_shortcuts.dart index 2733f4058..38840eb4c 100644 --- a/flutter/lib/models/local_flutter_shortcuts.dart +++ b/flutter/lib/models/local_flutter_shortcuts.dart @@ -3,6 +3,8 @@ import 'dart:async'; import 'package:flutter/foundation.dart'; import 'package:flutter/services.dart'; +import '../common/widgets/keyboard_shortcuts/shortcut_constants.dart'; + class LocalFlutterShortcutDispatcher { final void Function(String) onTriggered; // Keys whose press fired a shortcut and whose release has not arrived yet. @@ -11,22 +13,30 @@ class LocalFlutterShortcutDispatcher { // it is never dropped with a session. A key whose release is missed heals // itself: its next press is consumed as a repeat and that release removes it. static final _firedKeys = {}; + // Actions waiting for the release of the key that fired them, with the + // session that fired them. See [kShortcutActionsRunOnKeyUp]. + static final _keyUpActions = + {}; final _releasedModifiers = {}; bool _viewOnlyShortcutPending = false; int _generation = 0; LocalFlutterShortcutDispatcher({required this.onTriggered}); - /// Drops this session's pending action and modifier replay. Fired keys + /// Drops this session's pending actions and modifier replay. Fired keys /// stay owned until their release, whichever session receives it. void clear() { + _keyUpActions.removeWhere((_, pending) => identical(pending.$1, this)); _releasedModifiers.clear(); _viewOnlyShortcutPending = false; _generation++; } @visibleForTesting - static void resetFiredKeys() => _firedKeys.clear(); + static void resetFiredKeys() { + _firedKeys.clear(); + _keyUpActions.clear(); + } void recordReleasedModifiers(Iterable keys) { _releasedModifiers.addAll(keys); @@ -106,23 +116,39 @@ class LocalFlutterShortcutDispatcher { String? Function()? match, Future Function()? releaseModifiers, }) { - if (up) return _firedKeys.remove(key); + if (up) { + if (!_firedKeys.remove(key)) return false; + final pending = _keyUpActions.remove(key); + if (pending != null) { + final (owner, action) = pending; + unawaited(owner._trigger(action, null)); + } + return true; + } if (_firedKeys.contains(key)) return true; if (!down) return false; final action = match?.call(); if (action == null) return false; _firedKeys.add(key); if (viewOnly) _viewOnlyShortcutPending = true; - unawaited(_trigger(action, releaseModifiers)); + final runOnKeyUp = kShortcutActionsRunOnKeyUp.contains(action); + if (runOnKeyUp) { + _keyUpActions[key] = (this, action); + } else { + _keyUpActions.remove(key); + } + unawaited(_trigger(runOnKeyUp ? null : action, releaseModifiers)); return true; } + /// Releases the remote modifiers, then runs [action] unless it is null or + /// this session was cleared meanwhile. Future _trigger( - String action, Future Function()? releaseModifiers) async { + String? action, Future Function()? releaseModifiers) async { final generation = _generation; try { if (releaseModifiers != null) await releaseModifiers(); - if (generation == _generation) onTriggered(action); + if (action != null && generation == _generation) onTriggered(action); } catch (e, st) { debugPrint('Local shortcut failed for $action: $e\n$st'); } diff --git a/flutter/test/local_flutter_shortcuts_test.dart b/flutter/test/local_flutter_shortcuts_test.dart index cf3ec9691..de46188bc 100644 --- a/flutter/test/local_flutter_shortcuts_test.dart +++ b/flutter/test/local_flutter_shortcuts_test.dart @@ -2,6 +2,7 @@ import 'dart:async'; import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:flutter_hbb/common/widgets/keyboard_shortcuts/shortcut_constants.dart'; import 'package:flutter_hbb/models/local_flutter_shortcuts.dart'; LocalFlutterShortcutDispatcher dispatcherFor( @@ -224,6 +225,47 @@ void main() { expect(actions, isEmpty); }); + test('focus-changing actions run when the key is released', () async { + // Close tab moves focus before the key is released; the release may then + // land in a session whose keys go through the other matcher. + var releases = 0; + final actions = []; + final dispatcher = _ShortcutHarness( + match: (_) => kShortcutActionCloseTab, + releaseModifiers: () async => releases++, + onTriggered: actions.add, + ); + + expect(dispatcher.tryDispatch(down(PhysicalKeyboardKey.keyW)), isTrue); + expect(dispatcher.tryDispatch(repeat(PhysicalKeyboardKey.keyW)), isTrue); + await Future.value(); + expect(releases, 1, reason: 'remote modifiers are released on the press'); + expect(actions, isEmpty, reason: 'the action waits for the release'); + + final other = dispatcherFor(actions.add); + expect(other.tryDispatch(up(PhysicalKeyboardKey.keyW)), isTrue); + await Future.value(); + expect(actions, [kShortcutActionCloseTab]); + expect(other.tryDispatch(up(PhysicalKeyboardKey.keyW)), isFalse); + }); + + test('closing the session drops its action pending on key release', + () async { + final actions = []; + final dispatcher = _ShortcutHarness( + match: (_) => kShortcutActionSwitchTabNext, + releaseModifiers: () async {}, + onTriggered: actions.add, + ); + + expect(dispatcher.tryDispatch(down(PhysicalKeyboardKey.keyW)), isTrue); + dispatcher.clear(); + expect(dispatcher.tryDispatch(up(PhysicalKeyboardKey.keyW)), isTrue, + reason: 'the key stays owned'); + await Future.value(); + expect(actions, isEmpty); + }); + test('raw legacy shortcuts own repeats and release after modifiers change', () async { addTearDown(RawKeyboard.instance.clearKeysPressed); diff --git a/src/keyboard/shortcuts.rs b/src/keyboard/shortcuts.rs index 46919be94..9f23c4df4 100644 --- a/src/keyboard/shortcuts.rs +++ b/src/keyboard/shortcuts.rs @@ -328,15 +328,35 @@ lazy_static::lazy_static! { /// removes it. static ref FIRED_KEYS: std::sync::Mutex> = Default::default(); + /// Actions waiting for the release of the key that fired them, with the + /// session that fired them. See `runs_on_release`. + static ref RELEASE_ACTIONS: std::sync::Mutex< + std::collections::HashMap, + > = Default::default(); static ref RELEASED_MODIFIERS: std::sync::Mutex< std::collections::HashMap>, > = Default::default(); } -/// Forget the modifiers a session released on its remote. Fired keys are -/// physical and stay owned until their release, whichever session gets it. +/// Actions that move keyboard focus to another session. They run when the +/// key that fired them is released, not when it is pressed, so the matcher +/// that consumed the press also sees its repeats and its release. The next +/// session may route its keys through the other matcher (Flutter's legacy +/// path on Linux), which never saw the press. Mirrors +/// `kShortcutActionsRunOnKeyUp` in `shortcut_constants.dart`. +pub fn runs_on_release(action_id: &str) -> bool { + matches!( + action_id, + action_id::CLOSE_TAB | action_id::SWITCH_TAB_NEXT | action_id::SWITCH_TAB_PREV + ) +} + +/// Forget the modifiers a session released on its remote and the actions it +/// still owes on key release. Fired keys are physical and stay owned until +/// their release, whichever session gets it. #[cfg(feature = "flutter")] pub fn clear_session_state(session_id: &hbb_common::SessionID) { + RELEASE_ACTIONS.lock().unwrap().retain(|_, (sid, _)| sid != session_id); let held = RELEASED_MODIFIERS.lock().unwrap().remove(session_id); if let Some(held) = held { { @@ -361,6 +381,7 @@ pub fn enter_view_only(session_id: &hbb_common::SessionID) { // The Flutter input source stops routing key events here, so the release // of a fired key can no longer reach this matcher. FIRED_KEYS.lock().unwrap().clear(); + RELEASE_ACTIONS.lock().unwrap().clear(); clear_session_state(session_id); } @@ -419,7 +440,18 @@ pub fn try_dispatch( let mut fired = FIRED_KEYS.lock().unwrap(); match event.event_type { EventType::KeyPress(k) if fired.contains(&k) => return true, - EventType::KeyRelease(k) if fired.remove(&k) => return true, + EventType::KeyRelease(k) if fired.remove(&k) => { + drop(fired); + let pending = RELEASE_ACTIONS.lock().unwrap().remove(&k); + if let Some((sid, action_id)) = pending { + crate::flutter::push_session_event( + &sid, + "shortcut_triggered", + vec![("action", &action_id)], + ); + } + return true; + } _ => {} } } @@ -430,6 +462,12 @@ pub fn try_dispatch( release_remote_keys(sid, keyboard_mode, &peer(), &send); if let EventType::KeyPress(k) = event.event_type { FIRED_KEYS.lock().unwrap().insert(k); + let mut pending = RELEASE_ACTIONS.lock().unwrap(); + if runs_on_release(&action_id) { + pending.insert(k, (*sid, action_id)); + return true; + } + pending.remove(&k); } crate::flutter::push_session_event(sid, "shortcut_triggered", vec![("action", &action_id)]); true @@ -1199,6 +1237,48 @@ mod tests { release_chord(chord); } + /// Close tab and tab switching move focus away before the key is + /// released; they run on the release so the same matcher sees the whole + /// press. + #[cfg(feature = "flutter")] + #[test] + fn focus_changing_actions_run_on_release() { + use rdev::Key; + + let _guard = CACHE_TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let chord = enable_defaults_and_hold_chord(); + let prefix = || vec![Modifier::Primary, Modifier::Alt, Modifier::Shift]; + *CACHE.write().unwrap() = Arc::new(Bindings { + enabled: true, + pass_through: false, + bindings: vec![ + Binding { action: action_id::CLOSE_TAB.into(), mods: prefix(), key: "w".into() }, + Binding { action: action_id::SCREENSHOT.into(), mods: prefix(), key: "p".into() }, + ], + }); + let dispatch = |sid: &hbb_common::SessionID, e: &rdev::Event| { + try_dispatch(Some(sid), e, "map", || "windows".into(), |_| {}) + }; + let pending_for = |k: Key| RELEASE_ACTIONS.lock().unwrap().get(&k).cloned(); + + assert!(dispatch(&SID_A, &make_press(Key::KeyW))); + assert_eq!(pending_for(Key::KeyW), Some((SID_A, action_id::CLOSE_TAB.to_owned()))); + assert!(dispatch(&SID_A, &make_press(Key::KeyW)), "repeat is consumed"); + assert_eq!(pending_for(Key::KeyW), Some((SID_A, action_id::CLOSE_TAB.to_owned()))); + assert!(dispatch(&SID_B, &make_release(Key::KeyW)), "release is consumed wherever it lands"); + assert_eq!(pending_for(Key::KeyW), None, "the action ran on the release"); + + assert!(dispatch(&SID_A, &make_press(Key::KeyP))); + assert_eq!(pending_for(Key::KeyP), None, "other actions still run on the press"); + assert!(dispatch(&SID_A, &make_release(Key::KeyP))); + + assert!(dispatch(&SID_A, &make_press(Key::KeyW))); + clear_session_state(&SID_A); + assert_eq!(pending_for(Key::KeyW), None, "a closed session owes nothing"); + assert!(dispatch(&SID_A, &make_release(Key::KeyW)), "but its key stays owned"); + release_chord(chord); + } + #[cfg(feature = "flutter")] const SID_A: hbb_common::SessionID = hbb_common::SessionID::from_u128(0xA); #[cfg(feature = "flutter")] @@ -1212,6 +1292,7 @@ mod tests { #[cfg(feature = "flutter")] fn reset_fired_keys() { FIRED_KEYS.lock().unwrap().clear(); + RELEASE_ACTIONS.lock().unwrap().clear(); } #[cfg(feature = "flutter")]