From b0c4be9bd887fdb39e74e2c3eff21301394c682a Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Mon, 20 Apr 2020 15:23:39 -0700 Subject: [PATCH] [web] Synthesize keyup event when the browser doesn't trigger a keyup (#17742) --- lib/web_ui/lib/src/engine/keyboard.dart | 72 ++++++- lib/web_ui/test/keyboard_test.dart | 261 ++++++++++++++++++++++++ 2 files changed, 332 insertions(+), 1 deletion(-) diff --git a/lib/web_ui/lib/src/engine/keyboard.dart b/lib/web_ui/lib/src/engine/keyboard.dart index f9408d1438b42..572d1189d30cb 100644 --- a/lib/web_ui/lib/src/engine/keyboard.dart +++ b/lib/web_ui/lib/src/engine/keyboard.dart @@ -5,6 +5,12 @@ // @dart = 2.6 part of engine; +/// After a keydown is received, this is the duration we wait for a repeat event +/// before we decide to synthesize a keyup event. +/// +/// On Linux and Windows, the typical ranges for keyboard repeat delay go up to +/// 1000ms. On Mac, the range goes up to 2000ms. +const Duration _keydownCancelDuration = Duration(milliseconds: 1000); /// Provides keyboard bindings, such as the `flutter/keyevent` channel. class Keyboard { @@ -19,6 +25,12 @@ class Keyboard { static Keyboard get instance => _instance; static Keyboard _instance; + /// A mapping of [KeyboardEvent.code] to [Timer]. + /// + /// The timer is for when to synthesize a keyup for the [KeyboardEvent.code] + /// if no repeat events were received. + final Map _keydownTimers = {}; + html.EventListener _keydownListener; html.EventListener _keyupListener; @@ -44,6 +56,12 @@ class Keyboard { void dispose() { html.window.removeEventListener('keydown', _keydownListener); html.window.removeEventListener('keyup', _keyupListener); + + for (final String key in _keydownTimers.keys) { + _keydownTimers[key].cancel(); + } + _keydownTimers.clear(); + _keydownListener = null; _keyupListener = null; _instance = null; @@ -51,6 +69,11 @@ class Keyboard { static const JSONMessageCodec _messageCodec = JSONMessageCodec(); + /// Contains meta state from the latest event. + /// + /// Initializing with `0x0` which means no meta keys are pressed. + int _lastMetaState = 0x0; + void _handleHtmlEvent(html.KeyboardEvent event) { if (window._onPlatformMessage == null) { return; @@ -60,12 +83,37 @@ class Keyboard { event.preventDefault(); } + final String timerKey = event.code; + + // Don't synthesize a keyup event for modifier keys because the browser always + // sends a keyup event for those. + 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. + // + // To avoid this, we rely on the fact that browsers send repeat events + // while the key is held down by the user. If we don't receive a repeat + // 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') { + _keydownTimers[timerKey] = Timer(_keydownCancelDuration, () { + _keydownTimers.remove(timerKey); + _synthesizeKeyup(event); + }); + } else { + _keydownTimers.remove(timerKey); + } + } + + _lastMetaState = _getMetaState(event); final Map eventData = { 'type': event.type, 'keymap': 'web', 'code': event.code, 'key': event.key, - 'metaState': _getMetaState(event), + 'metaState': _lastMetaState, }; window.invokeOnPlatformMessage('flutter/keyevent', @@ -81,6 +129,19 @@ class Keyboard { return false; } } + + void _synthesizeKeyup(html.KeyboardEvent event) { + final Map eventData = { + 'type': 'keyup', + 'keymap': 'web', + 'code': event.code, + 'key': event.key, + 'metaState': _lastMetaState, + }; + + window.invokeOnPlatformMessage('flutter/keyevent', + _messageCodec.encodeMessage(eventData), _noopCallback); + } } const int _modifierNone = 0x00; @@ -109,4 +170,13 @@ int _getMetaState(html.KeyboardEvent event) { return metaState; } +/// Returns true if the [event] was caused by a modifier key. +/// +/// Modifier keys are shift, alt, ctrl and meta/cmd/win. These are the keys used +/// to perform keyboard shortcuts (e.g. `cmd+c`, `cmd+l`). +bool _isModifierKey(html.KeyboardEvent event) { + final String key = event.key; + return key == 'Meta' || key == 'Shift' || key == 'Alt' || key == 'Control'; +} + void _noopCallback(ByteData data) {} diff --git a/lib/web_ui/test/keyboard_test.dart b/lib/web_ui/test/keyboard_test.dart index c365f3c69a6b1..3a819d6ffbf90 100644 --- a/lib/web_ui/test/keyboard_test.dart +++ b/lib/web_ui/test/keyboard_test.dart @@ -7,6 +7,7 @@ import 'dart:html' as html; import 'dart:js_util' as js_util; import 'dart:typed_data'; +import 'package:quiver/testing/async.dart'; import 'package:ui/src/engine.dart'; import 'package:ui/ui.dart' as ui; @@ -289,6 +290,258 @@ void main() { Keyboard.instance.dispose(); }); + + testFakeAsync( + 'synthesize keyup when shortcut is handled by the system', + (FakeAsync async) { + // This can happen when the user clicks `cmd+alt+i` to open devtools. Here + // is the sequence we receive from the browser in such case: + // + // keydown(cmd) -> keydown(alt) -> keydown(i) -> keyup(alt) -> keyup(cmd) + // + // There's no `keyup(i)`. The web engine is expected to synthesize a + // `keyup(i)` event. + Keyboard.initialize(); + + List> messages = >[]; + ui.window.onPlatformMessage = (String channel, ByteData data, + ui.PlatformMessageResponseCallback callback) { + messages.add(const JSONMessageCodec().decodeMessage(data)); + }; + + dispatchKeyboardEvent( + 'keydown', + key: 'Meta', + code: 'MetaLeft', + isMetaPressed: true, + ); + dispatchKeyboardEvent( + 'keydown', + key: 'Alt', + code: 'AltLeft', + isMetaPressed: true, + isAltPressed: true, + ); + dispatchKeyboardEvent( + 'keydown', + key: 'i', + code: 'KeyI', + isMetaPressed: true, + isAltPressed: true, + ); + async.elapse(Duration(milliseconds: 10)); + dispatchKeyboardEvent( + 'keyup', + key: 'Meta', + code: 'MetaLeft', + isAltPressed: true, + ); + dispatchKeyboardEvent('keyup', key: 'Alt', code: 'AltLeft'); + // Notice no `keyup` for "i". + + expect(messages, >[ + { + 'type': 'keydown', + 'keymap': 'web', + 'key': 'Meta', + 'code': 'MetaLeft', + // meta + 'metaState': 0x8, + }, + { + 'type': 'keydown', + 'keymap': 'web', + 'key': 'Alt', + 'code': 'AltLeft', + // alt meta + 'metaState': 0x2 | 0x8, + }, + { + 'type': 'keydown', + 'keymap': 'web', + 'key': 'i', + 'code': 'KeyI', + // alt meta + 'metaState': 0x2 | 0x8, + }, + { + 'type': 'keyup', + 'keymap': 'web', + 'key': 'Meta', + 'code': 'MetaLeft', + // alt + 'metaState': 0x2, + }, + { + 'type': 'keyup', + 'keymap': 'web', + 'key': 'Alt', + 'code': 'AltLeft', + 'metaState': 0x0, + }, + ]); + messages.clear(); + + // Still too eary to synthesize a keyup event. + async.elapse(Duration(milliseconds: 50)); + expect(messages, isEmpty); + + async.elapse(Duration(seconds: 3)); + expect(messages, >[ + { + 'type': 'keyup', + 'keymap': 'web', + 'key': 'i', + 'code': 'KeyI', + 'metaState': 0x0, + } + ]); + + Keyboard.instance.dispose(); + }, + ); + + testFakeAsync( + 'do not synthesize keyup when we receive repeat events', + (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: 'Meta', + code: 'MetaLeft', + isMetaPressed: true, + ); + dispatchKeyboardEvent( + 'keydown', + key: 'Alt', + code: 'AltLeft', + isMetaPressed: true, + isAltPressed: true, + ); + dispatchKeyboardEvent( + 'keydown', + key: 'i', + code: 'KeyI', + isMetaPressed: true, + isAltPressed: true, + ); + async.elapse(Duration(milliseconds: 10)); + dispatchKeyboardEvent( + 'keyup', + key: 'Meta', + code: 'MetaLeft', + isAltPressed: true, + ); + dispatchKeyboardEvent('keyup', key: 'Alt', code: 'AltLeft'); + // Notice no `keyup` for "i". + + messages.clear(); + + // Spend more than 2 seconds sending repeat events and make sure no + // keyup was synthesized. + for (int i = 0; i < 20; i++) { + async.elapse(Duration(milliseconds: 100)); + dispatchKeyboardEvent( + 'keydown', + key: 'i', + code: 'KeyI', + repeat: true, + ); + } + + // There should be no synthesized keyup. + expect(messages, hasLength(20)); + for (int i = 0; i < 20; i++) { + expect(messages[i], { + 'type': 'keydown', + 'keymap': 'web', + 'key': 'i', + 'code': 'KeyI', + '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': 0x0, + } + ]); + + Keyboard.instance.dispose(); + }, + ); + + testFakeAsync('do not synthesize keyup for meta keys', (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: 'Meta', + code: 'MetaLeft', + isMetaPressed: true, + ); + dispatchKeyboardEvent( + 'keydown', + key: 'Alt', + code: 'AltLeft', + isMetaPressed: true, + isAltPressed: true, + ); + dispatchKeyboardEvent( + 'keydown', + key: 'i', + code: 'KeyI', + isMetaPressed: true, + isAltPressed: true, + ); + async.elapse(Duration(milliseconds: 10)); + dispatchKeyboardEvent( + 'keyup', + key: 'Meta', + code: 'MetaLeft', + isAltPressed: true, + ); + // Notice no `keyup` for "AltLeft" and "i". + + messages.clear(); + + // There has been no repeat events for "AltLeft" nor "i". Only "i" should + // synthesize a keyup event. + async.elapse(Duration(seconds: 3)); + expect(messages, >[ + { + 'type': 'keyup', + 'keymap': 'web', + 'key': 'i', + 'code': 'KeyI', + // alt + 'metaState': 0x2, + } + ]); + + Keyboard.instance.dispose(); + }); }); } @@ -341,3 +594,11 @@ html.KeyboardEvent dispatchKeyboardEvent( return event; } + +typedef FakeAsyncTest = void Function(FakeAsync); + +void testFakeAsync(String description, FakeAsyncTest fn) { + test(description, () { + FakeAsync().run(fn); + }); +}