-
Notifications
You must be signed in to change notification settings - Fork 27.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Text Input] Only unfocus when receive connectionClosed on platform except web #123929
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure that nothing in _finalizeEditing needs to be called when the connection is closed? There could be people that are relying on that behavior. It's reassuring that no tests have failed, though. At least no one in the past has had a strong enough opinion about this to add a test for it.
Emm seems google test failed again😳😳 |
(triage) On first look, the google testing failures look unrelated. Can you rebase this PR with the latest master to trigger another run? |
@justinmc |
@goderbauer google test still failed😳😳 |
The Google Testing service is having some issues right now. We'll have to wait until those are fixed. Sorry about that. |
All tests passed. Applying label for merging. ^_^ |
…xcept web (flutter#123929) [Text Input] Only unfocus when receive connectionClosed on platform except web
…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/
…atform except web (flutter#123929)" This reverts commit f4cd897.
One of #123523
Currently from engine side, in web engine we call
connectionClosed
fromhttps://github.com/flutter/engine/blob/b789b197c2188717dd48a15c2188cb1a1300d397/lib/web_ui/lib/src/engine/text_editing/text_editing.dart#L2162
But no call from other platform. When on platform like iOS or android, we actually expect a simple unfocus operation without calling other things.
Pre-launch Checklist
///
).