Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions lib/web_ui/lib/src/engine/keyboard.dart
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ class Keyboard {

final String timerKey = keyboardEvent.code!;

// Don't synthesize a keyup event for modifier keys because the browser always
// sends a keyup event for those.
// Don't handle synthesizing a keyup event for modifier keys
if (!_isModifierKey(event)) {
// When the user enters a browser/system shortcut (e.g. `cmd+alt+i`) the
// browser doesn't send a keyup for it. This puts the framework in a
Expand All @@ -103,7 +102,10 @@ class Keyboard {
// event within a specific duration ([_keydownCancelDuration]) we assume
// the user has released the key and we synthesize a keyup event.
_keydownTimers[timerKey]?.cancel();
if (event.type == 'keydown') {

// Only keys affected by modifiers, require synthesizing
// because the browser always sends a keyup event otherwise
if (event.type == 'keydown' && _isAffectedByModifiers(event)) {
_keydownTimers[timerKey] = Timer(_keydownCancelDuration, () {
_keydownTimers.remove(timerKey);
_synthesizeKeyup(event);
Expand Down Expand Up @@ -185,4 +187,11 @@ bool _isModifierKey(html.KeyboardEvent event) {
return key == 'Meta' || key == 'Shift' || key == 'Alt' || key == 'Control';
}

/// Returns true if the [event] is been affects by any of the modifiers key
///
/// This is a strong indication that this key is been used for a shortcut
bool _isAffectedByModifiers(html.KeyboardEvent event) {
return event.ctrlKey || event.shiftKey || event.altKey || event.metaKey;
}

void _noopCallback(ByteData? data) {}
41 changes: 30 additions & 11 deletions lib/web_ui/test/keyboard_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -473,18 +473,37 @@ void testMain() {
}
messages.clear();

// When repeat events stop for a long-enough period of time, a keyup
// should be synthesized.
Keyboard.instance.dispose();
},
);

testFakeAsync(
'do not synthesize keyup when keys are not affected by meta modifiers',
(FakeAsync async) {
Keyboard.initialize();

List<Map<String, dynamic>> messages = <Map<String, dynamic>>[];
ui.window.onPlatformMessage = (String channel, ByteData data,
ui.PlatformMessageResponseCallback callback) {
messages.add(const JSONMessageCodec().decodeMessage(data));
};

dispatchKeyboardEvent(
'keydown',
key: 'i',
code: 'KeyI',
);
dispatchKeyboardEvent(
'keydown',
key: 'o',
code: 'KeyO',
);
messages.clear();

// Wait for a long-enough period of time and no events
// should be synthesized
async.elapse(Duration(seconds: 3));
expect(messages, <Map<String, dynamic>>[
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mdebbar Your suggestion worked, and moving the condition deeper inside the if worked fine to solve the original issue.

To fix the test, I needed to remove this last assertion, which I realize, is not needed anymore, since we only synthesise key ups for meta affected keys, and up in the test, we trigger a bunch of repeat events, those would have cleared the synthesising timer, and therefore, no keyup should be synthesise. Let me know if this makes sense.

<String, dynamic>{
'type': 'keyup',
'keymap': 'web',
'key': 'i',
'code': 'KeyI',
'metaState': 0x0,
}
]);
expect(messages, hasLength(0));

Keyboard.instance.dispose();
},
Expand Down