development/flutter
development/flutter
1
0
Fork 0
mirror of https://github.com/flutter/flutter synced 2024-07-17 10:58:39 +00:00
Code Issues Actions 3 Packages Projects Releases Wiki Activity

[web] fix: do not call onSubmitted of TextField when switching browser tabs on mobile web (#134870)

Browse source 
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: 0b540a87f1/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.
367203b301/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.
cbda68a720/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`.
cbda68a720/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.
cbda68a720/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.
cbda68a720/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.
cbda68a720/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.
0b540a87f1/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/
This commit is contained in:
tauu 2023-09-28 18:43:04 +02:00 committed by GitHub
parent 880a0a1903
commit 3b19c2e5d8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 7 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
packages/flutter/lib/src/widgets/editable_text.dart
View file

@ -3488,11 +3488,7 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
_textInputConnection!.connectionClosedReceived();
_textInputConnection = null;
_lastKnownRemoteTextEditingValue = null;
if (kIsWeb) {
_finalizeEditing(TextInputAction.done, shouldUnfocus: true);
} else {
widget.focusNode.unfocus();
}
widget.focusNode.unfocus();
}
}

18
packages/flutter/test/widgets/editable_text_test.dart
View file

@ -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 {