Skip to content

Commit

Permalink
Revert "[web] Ensure handled key event is not propagated to IME" (flu…
Browse files Browse the repository at this point in the history
…tter#47086)

Reverts flutter#46829

Fixes flutter/flutter#136857

Speculative fix for The builds breaking on the web text tests as seen
here: https://github.com/flutter/flutter/runs/17840697842
flar authored Oct 19, 2023
1 parent 97c5022 commit d0d7b4d
Showing 6 changed files with 20 additions and 74 deletions.
9 changes: 2 additions & 7 deletions lib/web_ui/lib/src/engine/keyboard_binding.dart
Original file line number Diff line number Diff line change
@@ -13,7 +13,6 @@ import 'browser_detection.dart';
import 'dom.dart';
import 'key_map.g.dart';
import 'platform_dispatcher.dart';
import 'raw_keyboard.dart';
import 'semantics.dart';

typedef _VoidCallback = void Function();
@@ -105,12 +104,9 @@ class KeyboardBinding {
_addEventListener('keydown', (DomEvent domEvent) {
final FlutterHtmlKeyboardEvent event = FlutterHtmlKeyboardEvent(domEvent as DomKeyboardEvent);
_converter.handleEvent(event);
RawKeyboard.instance?.handleHtmlEvent(domEvent);
});
_addEventListener('keyup', (DomEvent domEvent) {
final FlutterHtmlKeyboardEvent event = FlutterHtmlKeyboardEvent(domEvent as DomKeyboardEvent);
_converter.handleEvent(event);
RawKeyboard.instance?.handleHtmlEvent(domEvent);
_addEventListener('keyup', (DomEvent event) {
_converter.handleEvent(FlutterHtmlKeyboardEvent(event as DomKeyboardEvent));
});
}

@@ -213,7 +209,6 @@ class FlutterHtmlKeyboardEvent {

bool getModifierState(String key) => _event.getModifierState(key);
void preventDefault() => _event.preventDefault();
void stopPropagation() => _event.stopPropagation();
bool get defaultPrevented => _event.defaultPrevented;
}

23 changes: 18 additions & 5 deletions lib/web_ui/lib/src/engine/raw_keyboard.dart
Original file line number Diff line number Diff line change
@@ -15,6 +15,15 @@ import 'services.dart';
/// Provides keyboard bindings, such as the `flutter/keyevent` channel.
class RawKeyboard {
RawKeyboard._(this._onMacOs) {
_keydownListener = createDomEventListener((DomEvent event) {
_handleHtmlEvent(event);
});
domWindow.addEventListener('keydown', _keydownListener);

_keyupListener = createDomEventListener((DomEvent event) {
_handleHtmlEvent(event);
});
domWindow.addEventListener('keyup', _keyupListener);
registerHotRestartListener(() {
dispose();
});
@@ -25,9 +34,6 @@ class RawKeyboard {
/// Use the [instance] getter to get the singleton after calling this method.
static void initialize({bool onMacOs = false}) {
_instance ??= RawKeyboard._(onMacOs);
// KeyboardBinding is responsible for forwarding the keyboard
// events to the RawKeyboard handler.
KeyboardBinding.initInstance();
}

/// The [RawKeyboard] singleton.
@@ -40,16 +46,24 @@ class RawKeyboard {
/// if no repeat events were received.
final Map<String, Timer> _keydownTimers = <String, Timer>{};

DomEventListener? _keydownListener;
DomEventListener? _keyupListener;

/// Uninitializes the [RawKeyboard] singleton.
///
/// After calling this method this object becomes unusable and [instance]
/// becomes `null`. Call [initialize] again to initialize a new singleton.
void dispose() {
domWindow.removeEventListener('keydown', _keydownListener);
domWindow.removeEventListener('keyup', _keyupListener);

for (final String key in _keydownTimers.keys) {
_keydownTimers[key]!.cancel();
}
_keydownTimers.clear();

_keydownListener = null;
_keyupListener = null;
_instance = null;
}

@@ -82,7 +96,7 @@ class RawKeyboard {
return event.type == 'keydown' && event.key == 'Tab' && event.isComposing;
}

void handleHtmlEvent(DomEvent domEvent) {
void _handleHtmlEvent(DomEvent domEvent) {
if (!domInstanceOfString(domEvent, 'KeyboardEvent')) {
return;
}
@@ -144,7 +158,6 @@ class RawKeyboard {
if (jsonResponse['handled'] as bool) {
// If the framework handled it, then don't propagate it any further.
event.preventDefault();
event.stopPropagation();
}
},
);
7 changes: 0 additions & 7 deletions lib/web_ui/test/common/keyboard_test_common.dart
Original file line number Diff line number Diff line change
@@ -22,7 +22,6 @@ class MockKeyboardEvent implements FlutterHtmlKeyboardEvent {
bool altGrKey = false,
this.location = 0,
this.onPreventDefault,
this.onStopPropagation,
}) : modifierState =
<String>{
if (altKey) 'Alt',
@@ -85,12 +84,6 @@ class MockKeyboardEvent implements FlutterHtmlKeyboardEvent {
bool get defaultPrevented => _defaultPrevented;
bool _defaultPrevented = false;

@override
void stopPropagation() {
onStopPropagation?.call();
}
VoidCallback? onStopPropagation;

static bool get lastDefaultPrevented => _lastEvent?.defaultPrevented ?? false;
static MockKeyboardEvent? _lastEvent;
}
4 changes: 0 additions & 4 deletions lib/web_ui/test/engine/raw_keyboard_test.dart
Original file line number Diff line number Diff line change
@@ -52,10 +52,6 @@ void testMain() {

DomKeyboardEvent event;

// Dispatch a keydown event first so that KeyboardBinding will recognize the keyup event.
// and will not set preventDefault on it.
event = dispatchKeyboardEvent('keydown', key: 'SomeKey', code: 'SomeCode', keyCode: 1);

event = dispatchKeyboardEvent('keyup', key: 'SomeKey', code: 'SomeCode', keyCode: 1);

expect(event.defaultPrevented, isFalse);
48 changes: 0 additions & 48 deletions lib/web_ui/test/engine/text_editing_test.dart
Original file line number Diff line number Diff line change
@@ -12,14 +12,12 @@ import 'package:test/test.dart';
import 'package:ui/src/engine.dart' show flutterViewEmbedder;
import 'package:ui/src/engine/browser_detection.dart';
import 'package:ui/src/engine/dom.dart';
import 'package:ui/src/engine/raw_keyboard.dart';
import 'package:ui/src/engine/services.dart';
import 'package:ui/src/engine/text_editing/autofill_hint.dart';
import 'package:ui/src/engine/text_editing/input_type.dart';
import 'package:ui/src/engine/text_editing/text_editing.dart';
import 'package:ui/src/engine/util.dart';
import 'package:ui/src/engine/vector_math.dart';
import 'package:ui/ui.dart' as ui;

import '../common/spy.dart';
import '../common/test_initialization.dart';
@@ -372,52 +370,6 @@ Future<void> testMain() async {
expect(lastInputAction, 'TextInputAction.done');
});

test('handling keyboard event prevents triggering input action', () {
final ui.PlatformMessageCallback? savedCallback = ui.window.onPlatformMessage;

bool markTextEventHandled = false;
ui.window.onPlatformMessage = (String channel, ByteData? data,
ui.PlatformMessageResponseCallback? callback) {
final ByteData response = const JSONMessageCodec()
.encodeMessage(<String, dynamic>{'handled': markTextEventHandled})!;
callback!(response);
};
RawKeyboard.initialize();

final InputConfiguration config = InputConfiguration();
editingStrategy!.enable(
config,
onChange: trackEditingState,
onAction: trackInputAction,
);

// No input action so far.
expect(lastInputAction, isNull);

markTextEventHandled = true;
dispatchKeyboardEvent(
editingStrategy!.domElement!,
'keydown',
keyCode: _kReturnKeyCode,
);

// Input action prevented by platform message callback.
expect(lastInputAction, isNull);

markTextEventHandled = false;
dispatchKeyboardEvent(
editingStrategy!.domElement!,
'keydown',
keyCode: _kReturnKeyCode,
);

// Input action received.
expect(lastInputAction, 'TextInputAction.done');

ui.window.onPlatformMessage = savedCallback;
RawKeyboard.instance?.dispose();
});

test('Triggers input action in multi-line mode', () {
final InputConfiguration config = InputConfiguration(
inputType: EngineInputType.multiline,
Original file line number Diff line number Diff line change
@@ -41,9 +41,6 @@ class LocaleKeymap {
return eventKeyCode;
}
if (result == null) {
if ((eventCode ?? '').isEmpty && (eventKey ?? '').isEmpty) {
return null;
}
final int? heuristicResult = heuristicMapper(eventCode ?? '', eventKey ?? '');
if (heuristicResult != null) {
return heuristicResult;

0 comments on commit d0d7b4d

Please sign in to comment.