From 51e47ffdd643a98885a84b644f03464f4573d9b2 Mon Sep 17 00:00:00 2001 From: Erick Zanardo Date: Thu, 9 Jul 2020 22:53:13 -0300 Subject: [PATCH 1/3] Fixing synthesizing keys for multiple keys pressed down --- lib/web_ui/lib/src/engine/keyboard.dart | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/web_ui/lib/src/engine/keyboard.dart b/lib/web_ui/lib/src/engine/keyboard.dart index 0d7aec59c57cc..9c3aff8c8271f 100644 --- a/lib/web_ui/lib/src/engine/keyboard.dart +++ b/lib/web_ui/lib/src/engine/keyboard.dart @@ -91,9 +91,9 @@ 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. - if (!_isModifierKey(event)) { + // Don't synthesize a keyup event for modifier keys, or keys not affected by modifiers, + // because the browser always sends a keyup event for those. + if (!_isModifierKey(event) && _isAffectedByModifiers(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 // corrupt state because it thinks the key was never released. @@ -185,4 +185,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) {} From 295041d605a685537dff1e843cfeb4b437e6bed9 Mon Sep 17 00:00:00 2001 From: Erick Zanardo Date: Tue, 11 Aug 2020 19:20:27 -0300 Subject: [PATCH 2/3] Fixing breaking test, and adding a new one --- lib/web_ui/test/keyboard_test.dart | 39 +++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/lib/web_ui/test/keyboard_test.dart b/lib/web_ui/test/keyboard_test.dart index 4b68cec6e2e13..7f3ca9d2ca3ca 100644 --- a/lib/web_ui/test/keyboard_test.dart +++ b/lib/web_ui/test/keyboard_test.dart @@ -406,7 +406,7 @@ void testMain() { ); testFakeAsync( - 'do not synthesize keyup when we receive repeat events', + 'do not synthesize keyup when we receive repeat events under meta affect', (FakeAsync async) { Keyboard.initialize(); @@ -457,6 +457,7 @@ void testMain() { key: 'i', code: 'KeyI', repeat: true, + isMetaPressed: true, ); } @@ -468,7 +469,7 @@ void testMain() { 'keymap': 'web', 'key': 'i', 'code': 'KeyI', - 'metaState': 0x0, + 'metaState': 0x08, }); } messages.clear(); @@ -482,7 +483,7 @@ void testMain() { 'keymap': 'web', 'key': 'i', 'code': 'KeyI', - 'metaState': 0x0, + 'metaState': 0x08, } ]); @@ -490,6 +491,38 @@ void testMain() { }, ); + testFakeAsync( + 'do not synthesize keyup when keys are not affected by meta modifiers', + (FakeAsync async) { + Keyboard.initialize(); + + List> messages = >[]; + 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, hasLength(0)); + + Keyboard.instance.dispose(); + }, + ); + testFakeAsync('do not synthesize keyup for meta keys', (FakeAsync async) { Keyboard.initialize(); From e5d92da110e7aecc9d27476be90e71ce758410ff Mon Sep 17 00:00:00 2001 From: Erick Zanardo Date: Sat, 22 Aug 2020 11:42:08 -0300 Subject: [PATCH 3/3] Improving handling the synthesizing keys with PR suggestions --- lib/web_ui/lib/src/engine/keyboard.dart | 10 ++++++---- lib/web_ui/test/keyboard_test.dart | 18 ++---------------- 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/lib/web_ui/lib/src/engine/keyboard.dart b/lib/web_ui/lib/src/engine/keyboard.dart index 9c3aff8c8271f..c003dcfce6045 100644 --- a/lib/web_ui/lib/src/engine/keyboard.dart +++ b/lib/web_ui/lib/src/engine/keyboard.dart @@ -91,9 +91,8 @@ class Keyboard { final String timerKey = keyboardEvent.code!; - // Don't synthesize a keyup event for modifier keys, or keys not affected by modifiers, - // because the browser always sends a keyup event for those. - if (!_isModifierKey(event) && _isAffectedByModifiers(event)) { + // 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 // corrupt state because it thinks the key was never released. @@ -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); diff --git a/lib/web_ui/test/keyboard_test.dart b/lib/web_ui/test/keyboard_test.dart index 7f3ca9d2ca3ca..c16fe89467b03 100644 --- a/lib/web_ui/test/keyboard_test.dart +++ b/lib/web_ui/test/keyboard_test.dart @@ -406,7 +406,7 @@ void testMain() { ); testFakeAsync( - 'do not synthesize keyup when we receive repeat events under meta affect', + 'do not synthesize keyup when we receive repeat events', (FakeAsync async) { Keyboard.initialize(); @@ -457,7 +457,6 @@ void testMain() { key: 'i', code: 'KeyI', repeat: true, - isMetaPressed: true, ); } @@ -469,24 +468,11 @@ void testMain() { 'keymap': 'web', 'key': 'i', 'code': 'KeyI', - 'metaState': 0x08, + 'metaState': 0x0, }); } messages.clear(); - // When repeat events stop for a long-enough period of time, a keyup - // should be synthesized. - async.elapse(Duration(seconds: 3)); - expect(messages, >[ - { - 'type': 'keyup', - 'keymap': 'web', - 'key': 'i', - 'code': 'KeyI', - 'metaState': 0x08, - } - ]); - Keyboard.instance.dispose(); }, );