From 3b19c2e5d83aac48d0629d7ff1f007076b17c32b Mon Sep 17 00:00:00 2001 From: tauu Date: Thu, 28 Sep 2023 18:43:04 +0200 Subject: [PATCH] [web] fix: do not call onSubmitted of TextField when switching browser tabs on mobile web (#134870) This PR fixes #134846. As discussed in the issue, the onSubmitted callback of a TextField is called when the browser switches tabs or is sent to the background if the flutter app is running in any mobile browser (desktop browsers are not affected). Furthermore there is no straight forward way to distinguish between onSubmitted being called because the user pressed the enter key and it being called because the user switched tabs. For example in a chat app this would cause a message to be sent when the user submits the text by pressing "send" on the virtual keyboard as well as when the user switches to another tab. The later action is likely not so much intended. The next section explains what causes the bug and explains the proposed fix. ## Bug Analysis The root cause for this behaviour is line 3494 in editable_text.dart: https://github.com/flutter/flutter/blob/0b540a87f1be9a5bb7e550c777dfe5221c53a112/packages/flutter/lib/src/widgets/editable_text.dart#L3487-L3499 Only if the app is running on the web `_finalizeEditing` is called and this will then trigger the onSubmitted callback. If flutter is running on the web, there are only exactly 3 cases, in which the function is called. The following call trace analysis will describe why. - `connectionClosed()` is only called by in one location, `_handleTextInputInvocation` of the TextInput service. https://github.com/flutter/flutter/blob/367203b3011fc1752cfa1f51adf9751d090c94e6/packages/flutter/lib/src/services/text_input.dart#L1896C12-L1899 - In particular it is only called if the TextInput service receives a 'TextInputClient.onConnectionClosed' message from the engine. - The only location where the web part of the engine send this message is the `onConnectionClosed` function of the TextEditingChannel. https://github.com/flutter/engine/blob/cbda68a720904137ee9dfdf840db323afcf52705/lib/web_ui/lib/src/engine/text_editing/text_editing.dart#L2242-L2254 - `onConnectionClosed` in turn is only called by the `sendTextConnectionClosedToFrameworkIfAny` function of `HybridTextEditing`. https://github.com/flutter/engine/blob/cbda68a720904137ee9dfdf840db323afcf52705/lib/web_ui/lib/src/engine/text_editing/text_editing.dart#L2340-L2345 The function `sendTextConnectionClosedToFrameworkIfAny` is only called at 3 distinct locations of the web engine. ### 1. IOSTextEditingStrategy As described in the comment `sendTextConnectionClosedToFrameworkIfAny` is called if the browser is sent to the background or the tab is changed. https://github.com/flutter/engine/blob/cbda68a720904137ee9dfdf840db323afcf52705/lib/web_ui/lib/src/engine/text_editing/text_editing.dart#L1632-L1656 ### 2. AndroidTextEditingStrategy Same situation as for iOS. `sendTextConnectionClosedToFrameworkIfAny` is also called if `windowHasFocus` is false, which is the case if the browser is sent to background or the tab is changed. https://github.com/flutter/engine/blob/cbda68a720904137ee9dfdf840db323afcf52705/lib/web_ui/lib/src/engine/text_editing/text_editing.dart#L1773-L1785 ### 3. TextInputFinishAutofillContext This call seems to always happen when `finishAutofillContext` is triggered by the framework. https://github.com/flutter/engine/blob/cbda68a720904137ee9dfdf840db323afcf52705/lib/web_ui/lib/src/engine/text_editing/text_editing.dart#L2075-L2083 ## Proposed Fix The fixed proposed and implemented by this PR is to simply delete the call to`_finalizeEditing` in the `connectionClosed` function of editable_text.dart. https://github.com/flutter/flutter/blob/0b540a87f1be9a5bb7e550c777dfe5221c53a112/packages/flutter/lib/src/widgets/editable_text.dart#L3487-L3499 The reasoning for this being: * `_finalizeEditing` is only called in `connectionClosed` for the web engine. * As explained by the trace analysis above, the web engine only triggers this `_finalizeEditing` call in 3 cases. * In the 2 cases for IOSTextEditingStrategy and AndroidTextEditingStrategy the web engine triggering the call only causes the undesired behaviour reported in the issue. * In the third case for TextInputFinishAutofillContext, I can't see a good reason why this would require calling `_finalizeEditing` as it only instructs the platform to save the current values. Other platforms also don't have anything that would trigger onSubmitted being called, so it seems safe to remove it. * For other platforms the onConnectionClosed function was recently incorporated to only unfocus the TextField. So removing the call `_finalizeEditing` unifies the platform behaviour. See also https://github.com/flutter/flutter/pull/123929 https://github.com/flutter/engine/pull/41500 *List which issues are fixed by this PR. You must list at least one issue.* #134846 To simplify the evaluation, here are two versions of the minimal example given in the issue, build with the current master and with this PR applied: current master: https://tauu.github.io/flutter-onsubmit-test/build/web-master/ current master + PR applied: https://tauu.github.io/flutter-onsubmit-test/build/web/ --- .../flutter/lib/src/widgets/editable_text.dart | 6 +----- .../test/widgets/editable_text_test.dart | 18 ++++++------------ 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/packages/flutter/lib/src/widgets/editable_text.dart b/packages/flutter/lib/src/widgets/editable_text.dart index 339c6405fa4..c4c376565aa 100644 --- a/packages/flutter/lib/src/widgets/editable_text.dart +++ b/packages/flutter/lib/src/widgets/editable_text.dart @@ -3488,11 +3488,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien _textInputConnection!.connectionClosedReceived(); _textInputConnection = null; _lastKnownRemoteTextEditingValue = null; - if (kIsWeb) { - _finalizeEditing(TextInputAction.done, shouldUnfocus: true); - } else { - widget.focusNode.unfocus(); - } + widget.focusNode.unfocus(); } } diff --git a/packages/flutter/test/widgets/editable_text_test.dart b/packages/flutter/test/widgets/editable_text_test.dart index dc5b7fb797a..e3dfc46c78e 100644 --- a/packages/flutter/test/widgets/editable_text_test.dart +++ b/packages/flutter/test/widgets/editable_text_test.dart @@ -1559,7 +1559,9 @@ void main() { expect(tester.testTextInput.setClientArgs!['inputAction'], equals('TextInputAction.done')); }); - // Test case for https://github.com/flutter/flutter/issues/123523. + // Test case for + // https://github.com/flutter/flutter/issues/123523 + // https://github.com/flutter/flutter/issues/134846 . testWidgetsWithLeakTracking( 'The focus and callback behavior are correct when TextInputClient.onConnectionClosed message received', (WidgetTester tester) async { @@ -1597,17 +1599,9 @@ void main() { editableText.connectionClosed(); await tester.pump(); - if (kIsWeb) { - expect(onSubmittedInvoked, isTrue); - expect(onEditingCompleteInvoked, isTrue); - // Because we add the onEditingComplete and we didn't unfocus there, so focus still exists. - expect(focusNode.hasFocus, isTrue); - } else { - // For mobile and other platforms, calling connectionClosed will only unfocus. - expect(focusNode.hasFocus, isFalse); - expect(onEditingCompleteInvoked, isFalse); - expect(onSubmittedInvoked, isFalse); - } + expect(focusNode.hasFocus, isFalse); + expect(onEditingCompleteInvoked, isFalse); + expect(onSubmittedInvoked, isFalse); }); testWidgetsWithLeakTracking('connection is closed when TextInputClient.onConnectionClosed message received', (WidgetTester tester) async {