From c0dfb3386ffce3e73a516c694e2aaaebca709b35 Mon Sep 17 00:00:00 2001 From: Nurhan Turgut <50856934+nturgut@users.noreply.github.com> Date: Tue, 17 Dec 2019 16:18:54 -0800 Subject: [PATCH] Calling onConnectionClosed when the input element is blurred (flutter/engine#14484) * close connection call * closing connection on blur * remove the timer and check the window focus directly. address reviewer comments. * addressing reviewer comments --- .../web_ui/lib/src/engine/dom_renderer.dart | 10 ++ .../src/engine/text_editing/text_editing.dart | 158 +++++++++++++----- .../lib/web_ui/test/text_editing_test.dart | 37 ++++ 3 files changed, 167 insertions(+), 38 deletions(-) diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/dom_renderer.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/dom_renderer.dart index a7f6ac1d4cc..814afba8a9e 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/dom_renderer.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/dom_renderer.dart @@ -62,6 +62,16 @@ class DomRenderer { static const String _staleHotRestartStore = '__flutter_state'; List _staleHotRestartState; + /// Used to decide if the browser tab still has the focus. + /// + /// This information is useful for deciding on the blur behavior. + /// See [DefaultTextEditingStrategy]. + /// + /// This getter calls the `hasFocus` method of the `Document` interface. + /// See for more details: + /// https://developer.mozilla.org/en-US/docs/Web/API/Document/hasFocus + bool get windowHasFocus => js_util.callMethod(html.document, 'hasFocus', []); + void _setupHotRestart() { // This persists across hot restarts to clear stale DOM. _staleHotRestartState = diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/text_editing/text_editing.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/text_editing/text_editing.dart index 9c99621b78d..589bc5da4d6 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/text_editing/text_editing.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/text_editing/text_editing.dart @@ -343,6 +343,33 @@ class DefaultTextEditingStrategy implements TextEditingStrategy { _subscriptions.add(domElement.onKeyDown.listen(_maybeSendAction)); _subscriptions.add(html.document.onSelectionChange.listen(_handleChange)); + + // The behavior for blur in DOM elements changes depending on the reason of + // blur: + // + // (1) If the blur is triggered due to tab change or browser minimize, same + // element receives the focus as soon as the page reopens. Hence, text + // editing connection does not need to be closed. In this case we dot blur + // the DOM element. + // + // (2) On the other hand if the blur is triggered due to interaction with + // another element on the page, the current text connection is obsolete so + // connection close request is send to Flutter. + // + // See [HybridTextEditing.sendTextConnectionClosedToFlutterIfAny]. + // + // In order to detect between these two cases, after a blur event is + // triggered [domRenderer.windowHasFocus] method which checks the window + // focus is called. + _subscriptions.add(domElement.onBlur.listen((_) { + if (domRenderer.windowHasFocus) { + // Focus is still on the body. Continue with blur. + owner.sendTextConnectionClosedToFrameworkIfAny(); + } else { + // Refocus. + domElement.focus(); + } + })); } @override @@ -508,7 +535,18 @@ class IOSTextEditingStrategy extends DefaultTextEditingStrategy { domElement.style.transform = 'translate(-9999px, -9999px)'; _canPosition = false; + } + @override + void addEventHandlers() { + // Subscribe to text and selection changes. + _subscriptions.add(domElement.onInput.listen(_handleChange)); + + _subscriptions.add(domElement.onKeyDown.listen(_maybeSendAction)); + + _subscriptions.add(html.document.onSelectionChange.listen(_handleChange)); + + // Position the DOM element after it is focused. _subscriptions.add(domElement.onFocus.listen((_) { // Cancel previous timer if exists. _positionInputElementTimer?.cancel(); @@ -516,15 +554,15 @@ class IOSTextEditingStrategy extends DefaultTextEditingStrategy { _canPosition = true; positionElement(); }); + })); - // When the virtual keyboard is closed on iOS, onBlur is triggered. - _subscriptions.add(domElement.onBlur.listen((_) { - // Cancel the timer since there is no need to set the location of the - // input element anymore. It needs to be focused again to be editable - // by the user. - _positionInputElementTimer?.cancel(); - _positionInputElementTimer = null; - })); + // On iOS, blur is trigerred if the virtual keyboard is closed or the + // browser is sent to background or the browser tab is changed. + // + // Since in all these cases, the connection needs to be closed, + // [domRenderer.windowHasFocus] is not checked in [IOSTextEditingStrategy]. + _subscriptions.add(domElement.onBlur.listen((_) { + owner.sendTextConnectionClosedToFrameworkIfAny(); })); } @@ -567,19 +605,24 @@ class AndroidTextEditingStrategy extends DefaultTextEditingStrategy { @override void addEventHandlers() { - super.addEventHandlers(); - // Chrome on Android will hide the onscreen keyboard when you tap outside - // the text box. Instead, we want the framework to tell us to hide the - // keyboard via `TextInput.clearClient` or `TextInput.hide`. - if (browserEngine == BrowserEngine.blink || - browserEngine == BrowserEngine.unknown) { - _subscriptions.add(domElement.onBlur.listen((_) { - if (isEnabled) { - // Refocus. - domElement.focus(); - } - })); - } + // Subscribe to text and selection changes. + _subscriptions.add(domElement.onInput.listen(_handleChange)); + + _subscriptions.add(domElement.onKeyDown.listen(_maybeSendAction)); + + _subscriptions.add(html.document.onSelectionChange.listen(_handleChange)); + + _subscriptions.add(domElement.onBlur.listen((_) { + if (domRenderer.windowHasFocus) { + // Chrome on Android will hide the onscreen keyboard when you tap outside + // the text box. Instead, we want the framework to tell us to hide the + // keyboard via `TextInput.clearClient` or `TextInput.hide`. Therefore + // refocus as long as [domRenderer.windowHasFocus] is true. + domElement.focus(); + } else { + owner.sendTextConnectionClosedToFrameworkIfAny(); + } + })); } } @@ -597,27 +640,49 @@ class FirefoxTextEditingStrategy extends DefaultTextEditingStrategy { _subscriptions.add(domElement.onKeyDown.listen(_maybeSendAction)); - /// Detects changes in text selection. - /// - /// In Firefox, when cursor moves, neither selectionChange nor onInput - /// events are triggered. We are listening to keyup event. Selection start, - /// end values are used to decide if the text cursor moved. - /// - /// Specific keycodes are not checked since users/applications can bind - /// their own keys to move the text cursor. - /// Decides if the selection has changed (cursor moved) compared to the - /// previous values. - /// - /// After each keyup, the start/end values of the selection is compared to - /// the previously saved editing state. + // Detects changes in text selection. + // + // In Firefox, when cursor moves, neither selectionChange nor onInput + // events are triggered. We are listening to keyup event. Selection start, + // end values are used to decide if the text cursor moved. + // + // Specific keycodes are not checked since users/applications can bind + // their own keys to move the text cursor. + // Decides if the selection has changed (cursor moved) compared to the + // previous values. + // + // After each keyup, the start/end values of the selection is compared to + // the previously saved editing state. _subscriptions.add(domElement.onKeyUp.listen((event) { _handleChange(event); })); - /// In Firefox the context menu item "Select All" does not work without - /// listening to onSelect. On the other browsers onSelectionChange is - /// enough for covering "Select All" functionality. + // In Firefox the context menu item "Select All" does not work without + // listening to onSelect. On the other browsers onSelectionChange is + // enough for covering "Select All" functionality. _subscriptions.add(domElement.onSelect.listen(_handleChange)); + + // For Firefox, we also use the same approach as the parent class. + // + // Do not blur the DOM element if the user goes to another tab or minimizes + // the browser. See [super.addEventHandlers] for more comments. + // + // The different part is, in Firefox, we are not able to get correct value + // when we check the window focus like [domRendered.windowHasFocus]. + // + // However [document.activeElement] always equals to [domElement] if the + // user goes to another tab, minimizes the browser or opens the dev tools. + // Hence [document.activeElement] is checked in this listener. + _subscriptions.add(domElement.onBlur.listen((_) { + html.Element activeElement = html.document.activeElement; + if (activeElement != domElement) { + // Focus is still on the body. Continue with blur. + owner.sendTextConnectionClosedToFrameworkIfAny(); + } else { + // Refocus. + domElement.focus(); + } + })); } } @@ -683,7 +748,6 @@ class PersistentTextEditingElement extends DefaultTextEditingStrategy { // Refocus after setting editing state. domElement.focus(); } - } /// Text editing singleton. @@ -857,6 +921,24 @@ class HybridTextEditing { _emptyCallback, ); } + + void sendTextConnectionClosedToFrameworkIfAny() { + if (isEditing) { + stopEditing(); + ui.window.onPlatformMessage( + 'flutter/textinput', + const JSONMethodCodec().encodeMethodCall( + MethodCall( + 'TextInputClient.onConnectionClosed', + [ + _clientId, + ], + ), + ), + _emptyCallback, + ); + } + } } /// Information on the font and alignment of a text editing element. diff --git a/engine/src/flutter/lib/web_ui/test/text_editing_test.dart b/engine/src/flutter/lib/web_ui/test/text_editing_test.dart index 125e80f7779..971f0ff49fe 100644 --- a/engine/src/flutter/lib/web_ui/test/text_editing_test.dart +++ b/engine/src/flutter/lib/web_ui/test/text_editing_test.dart @@ -598,6 +598,43 @@ void main() { expect(spy.messages, isEmpty); }); + test('close connection on blur', () async { + final MethodCall setClient = MethodCall( + 'TextInput.setClient', [123, flutterSinglelineConfig]); + textEditing.handleTextInput(codec.encodeMethodCall(setClient)); + + const MethodCall setEditingState = + MethodCall('TextInput.setEditingState', { + 'text': 'abcd', + 'selectionBase': 2, + 'selectionExtent': 3, + }); + textEditing.handleTextInput(codec.encodeMethodCall(setEditingState)); + + // Editing shouldn't have started yet. + expect(document.activeElement, document.body); + + const MethodCall show = MethodCall('TextInput.show'); + textEditing.handleTextInput(codec.encodeMethodCall(show)); + + checkInputEditingState( + textEditing.editingElement.domElement, 'abcd', 2, 3); + + // DOM element is blurred. + textEditing.editingElement.domElement.blur(); + + expect(spy.messages, hasLength(1)); + MethodCall call = spy.messages[0]; + spy.messages.clear(); + expect(call.method, 'TextInputClient.onConnectionClosed'); + expect( + call.arguments, + [ + 123, // Client ID + ], + ); + }); + test('setClient, setEditingState, show, setClient', () { final MethodCall setClient = MethodCall( 'TextInput.setClient', [123, flutterSinglelineConfig]);