Close connection on keyboard close #41500

ksballetba · 2023-04-26T02:51:41Z

Fix:

Flutter TextField focus state is not correct when keyboard closed from platform side. flutter#123523
Engine android CI should verify VisibleForTests field are not used flutter#124890

Before this patch:

before.fix.mp4

After this patch:

after.fix.mp4

Pre-launch Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I read the [Tree Hygiene] wiki page, which explains my responsibilities.
I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides].
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the [CLA].
All existing and new tests are passing.

…correct.

…s state is correct.

mossmana · 2023-04-26T20:14:23Z

shell/platform/android/io/flutter/embedding/engine/systemchannels/TextInputChannel.java

@@ -367,6 +367,14 @@ public void performPrivateCommand(
        "TextInputClient.performPrivateCommand", Arrays.asList(inputClientId, json));
  }

+  /** Instructs Flutter to execute a "onConnectionClosed" action. */
+  public void onConnectionClosed(int inputClientId) {
+    Log.v(TAG, "Sending 'onConnectionClosed' message.");


If this log message is not actionable, I suggest removing it. https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#only-log-actionable-messages-to-the-console

Because other function in TextInputChannel has same log.

Sounds good! LGTM!

luckysmg · 2023-05-05T02:54:03Z

The code diff between this and #40746 is only adding a

@VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)

on ImeSyncDeferringInsetsCallback to solve the lint issue
So I think LGTM ^_^

luckysmg · 2023-05-05T02:54:33Z

Adding label for merging, let's see if the issue gone ^_^

…126124) flutter/engine@269ce2d...c97a0de 2023-05-05 [email protected] Close connection on keyboard close (flutter/engine#41500) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…r 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 #123929 flutter/engine#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/

…r tabs on mobile web (flutter#134870) This PR fixes flutter#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 flutter#123929 flutter/engine#41500 *List which issues are fixed by this PR. You must list at least one issue.* flutter#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/

DBOutlink · 2023-12-12T20:45:24Z

From the tests I have done this is not the default behavior of a native Android app: in fact when the keyboard is hidden the text field maintains focus

This reverts commit c97a0de.

Related issue: flutter/flutter#140508

ksballetba and others added 10 commits March 28, 2023 20:09

[Android] Send unfocus message to framework to ensure focus state is …

09f2260

…correct.

[Android] Send onConnectionClosed message to framework to ensure focu…

3dec297

…s state is correct.

Merge branch 'main' into handle_unfocus

8e5b2de

add test.

86cc4a2

add test.

61f6e6c

add test.

b093212

fix test problem.

e5b1867

fix nits.

6570eaa

Merge branch 'handle_unfocus' into close_connection_on_keyboard_close

d3cfb05

fix lint problem.

b2708f3

flutter-dashboard bot added the platform-android label Apr 26, 2023

luckysmg assigned ksballetba Apr 26, 2023

luckysmg requested review from reidbaker and chinmaygarde April 26, 2023 02:55

add comments

b700728

reidbaker requested review from mossmana and removed request for reidbaker April 26, 2023 19:52

mossmana requested changes Apr 27, 2023

View reviewed changes

mossmana approved these changes May 4, 2023

View reviewed changes

luckysmg approved these changes May 5, 2023

View reviewed changes

luckysmg added the autosubmit Merge PR when tree becomes green via auto submit App label May 5, 2023

auto-submit bot merged commit c97a0de into flutter:main May 5, 2023

luckysmg mentioned this pull request May 5, 2023

Flutter TextField focus state is not correct when keyboard closed from platform side. flutter/flutter#123523

Closed

3 tasks

engine-flutter-autoroll mentioned this pull request May 5, 2023

Roll Flutter Engine from 269ce2deebeb to c97a0deccbc1 (1 revision) flutter/flutter#126124

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 5, 2023

c97a0decc Close connection on keyboard close (flutter/engine#41500)

2b54fb7

tauu mentioned this pull request Sep 16, 2023

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

Merged

8 tasks

MeandNi mentioned this pull request Dec 21, 2023

Whether focus exists has nothing to do with whether the keyboard is open flutter/flutter#140508

Open

MeandNi added a commit to MeandNi/engine that referenced this pull request Dec 22, 2023

Revert "Close connection on keyboard close (flutter#41500)"

6bd07db

This reverts commit c97a0de.

auto-submit bot pushed a commit that referenced this pull request Jan 30, 2024

Revert "Close connection on keyboard close (#41500)" (#49366)

500ae6c

Related issue: flutter/flutter#140508

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Close connection on keyboard close #41500

Close connection on keyboard close #41500

ksballetba commented Apr 26, 2023

mossmana Apr 26, 2023

ksballetchan May 4, 2023

ksballetchan May 4, 2023

mossmana May 4, 2023

luckysmg commented May 5, 2023

luckysmg commented May 5, 2023

DBOutlink commented Dec 12, 2023

Close connection on keyboard close #41500

Close connection on keyboard close #41500

Conversation

ksballetba commented Apr 26, 2023

Before this patch:

After this patch:

Pre-launch Checklist

mossmana Apr 26, 2023

Choose a reason for hiding this comment

ksballetchan May 4, 2023

Choose a reason for hiding this comment

ksballetchan May 4, 2023

Choose a reason for hiding this comment

mossmana May 4, 2023

Choose a reason for hiding this comment

luckysmg commented May 5, 2023

luckysmg commented May 5, 2023

DBOutlink commented Dec 12, 2023