-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Fix previous text input connection interrupts enter/space #171973
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
Conversation
81c80a3 to
617dbdc
Compare
camsim99
left a comment
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.
Overall looks good! I just have one question about the test.
| } | ||
|
|
||
| @Test | ||
| public void clearTextInputClient_restartsImmWhenIMEHidden() { |
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.
I can see how this tests the changes to the TextInputPlugin constructor, but does this actually call/test clearTextInputClient? Sorry if I'm missing it; from what I can tell, that is only called via method channel?
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.
Good point, I added a new test that calls clearTextInputClient explicitly.
aadb99a to
388498f
Compare
justinmc
left a comment
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.
LGTM 👍
Restarting the IME when it disappears seems like it would be a good idea. I know we have had lots of bugs related to lingering state in Samsung IMEs and have tried a lot of hacks to restart the IME. It seems to me like you always want it to restart when it disappears though, and I'm encouraged by seeing this close several different issues.
|
|
||
| @Test | ||
| public void destroy_clearTextInputMethodHandler() { | ||
|
|
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.
This new line looks unintentional? (Feel free to skip if this is intentional!)
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.
Thanks for catching that!
50022d6 to
100856a
Compare
camsim99
left a comment
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.
LGTM!
100856a to
632c77c
Compare
…1973) Fixes an issue where a previously closed text input connection may cause interference with hardware keyboard on Samsung devices. On the Samsung Galaxy S10 Tab the Samsung IME will request the operating system to show the IME when pressing enter/space on components like a radio option or a checkbox if there was a previously opened text input connection. It seems the Samsung Keyboard is caching some text input related state, and when it detects an enter/space key it acts on this previously cached state even if the text input connection has been closed. This prevents the radio option/checkbox and framework as a whole from responding to the key events. To solve this issue we should reset the input method manager when the IME has been hidden, and when the framework closes the input connection we should also reset the input method manager if the IME is hidden at the time of closing. This effectively resets any state cached by the Samsung keyboard and it no longer acts on stale state. Fixes flutter#168099 Fixes flutter#51478 Fixes flutter#70546 Related: flutter#136745 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. --------- Co-authored-by: Renzo Olivares <[email protected]>
Roll Flutter from 20788c07fcb8 to cfbaa18bdecc (43 revisions) flutter/flutter@20788c0...cfbaa18 2025-07-28 [email protected] Remove shortner (flutter/flutter#172844) 2025-07-28 [email protected] Update `flutter pub get` to use `flutter.version.json` (instead of `version`) (flutter/flutter#172798) 2025-07-27 [email protected] Roll Fuchsia Linux SDK from i7RS-4iBtdndwa3Jy... to tK_PAaLeo0pUxi8hv... (flutter/flutter#172827) 2025-07-26 [email protected] Roll Fuchsia Linux SDK from 4IewCwqcEm7r4QsXd... to i7RS-4iBtdndwa3Jy... (flutter/flutter#172808) 2025-07-26 98614782+auto-submit[bot]@users.noreply.github.com Reverts "fix: get content hash for master on local engine branches (#172792)" (flutter/flutter#172805) 2025-07-26 [email protected] fix: build size_exp artifacts in post (flutter/flutter#172802) 2025-07-26 [email protected] fix: get content hash for master on local engine branches (flutter/flutter#172792) 2025-07-25 [email protected] Update `main`/`master` repoExceptions analysis set (flutter/flutter#172796) 2025-07-25 [email protected] Remove deprecated `--[no-]-disable-dds` (flutter/flutter#172791) 2025-07-25 [email protected] `last_engine_commit.ps1`: Use `$flutterRoot` instead of `$gitTopLevel` (flutter/flutter#172786) 2025-07-25 [email protected] Check file contents, remove tests that are now invalid (`: 'none'`) (flutter/flutter#172788) 2025-07-25 [email protected] windows: allow pwsh.bat wrapper (flutter/flutter#171778) 2025-07-25 [email protected] [Impeller] Log glCheckFrameBufferStatus output when it fails in blit pass. (flutter/flutter#172592) 2025-07-25 [email protected] Remove deprecated `AssetManifest.json` file (flutter/flutter#172594) 2025-07-25 [email protected] Refactor `verifyVersion` into typed `Version` object, use it (flutter/flutter#172780) 2025-07-25 [email protected] Bump the customer tests to pick up an update to Zulip's tests, for th… (flutter/flutter#172733) 2025-07-25 [email protected] Rename `AppRunLogger`, stop writing status messages that break JSON (flutter/flutter#172591) 2025-07-25 [email protected] Add `--dart-define`, `-D` to `assemble`, deprecate `--define`, `-d`. (flutter/flutter#172510) 2025-07-25 [email protected] Fix previous text input connection interrupts enter/space (flutter/flutter#171973) 2025-07-25 [email protected] Update CHANGELOG.md for 3.32.8 (flutter/flutter#172779) 2025-07-25 [email protected] Fix an `omit-type` lint that is breaking the tree (flutter/flutter#172776) 2025-07-25 [email protected] feat(tool): Respect user-data-dir flag from web-browser-flag (flutter/flutter#169445) 2025-07-25 [email protected] [web] Fix empty first frame in multiview mode (flutter/flutter#172493) 2025-07-25 [email protected] Migrate to null aware elements - Part 3 (flutter/flutter#172307) 2025-07-25 [email protected] Update few links in docs (flutter/flutter#172166) 2025-07-25 [email protected] Roll Skia from 768ace3fd144 to 409e1c7ba09b (1 revision) (flutter/flutter#172745) 2025-07-25 [email protected] Roll Fuchsia Linux SDK from a_55b5hqpRnigzuNC... to 4IewCwqcEm7r4QsXd... (flutter/flutter#172742) 2025-07-25 [email protected] Roll Skia from 094ac350125f to 768ace3fd144 (18 revisions) (flutter/flutter#172735) 2025-07-25 [email protected] Remove 2023 deprecated `'platforms'` key from daemon output (flutter/flutter#172593) 2025-07-24 [email protected] Manual roll of Dart from 26dece893751 to 5ede85aff798 (flutter/flutter#172731) 2025-07-24 [email protected] 169108 ci debug (flutter/flutter#172589) 2025-07-24 [email protected] Improve assertion message in `_MixedBorderRadius.resolve()` (flutter/flutter#172100) 2025-07-24 [email protected] Make sure that a BottomAppBar doesn't crash in 0x0 environment (flutter/flutter#172097) 2025-07-24 [email protected] Handle#6537 bottom navigation bar (flutter/flutter#172404) 2025-07-24 [email protected] Make sure that a MaterialBanner doesn't crash in 0x0 environment (flutter/flutter#172074) 2025-07-24 [email protected] Remove support for `pluginClass: none` (on `master`; `> 3.35.0`) (flutter/flutter#172579) 2025-07-24 [email protected] Make sure that a Card doesn't crash in 0x0 environment (flutter/flutter#172373) 2025-07-24 [email protected] Make sure that an AboutDialog doesn't crash in 0x0 environment (flutter/flutter#172444) 2025-07-24 [email protected] Make sure that an ActionChip doesn't crash in 0x0 environment (flutter/flutter#172684) 2025-07-24 [email protected] Delete (unused) `dev/checks_tool` directory (flutter/flutter#172715) 2025-07-24 [email protected] [Android] Add missing ci configuration for `Linux android_java17_tool_integration_tests` (flutter/flutter#172702) 2025-07-24 [email protected] Flutter-Cherrypick-Process.md - Spell cherry-pick consistently (flutter/flutter#172697) 2025-07-24 [email protected] Bringup `linux_web_engine_tests`; failing 4+ times in a row (flutter/flutter#172714) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: ...
…r#9688) Roll Flutter from 20788c07fcb8 to cfbaa18bdecc (43 revisions) flutter/flutter@20788c0...cfbaa18 2025-07-28 [email protected] Remove shortner (flutter/flutter#172844) 2025-07-28 [email protected] Update `flutter pub get` to use `flutter.version.json` (instead of `version`) (flutter/flutter#172798) 2025-07-27 [email protected] Roll Fuchsia Linux SDK from i7RS-4iBtdndwa3Jy... to tK_PAaLeo0pUxi8hv... (flutter/flutter#172827) 2025-07-26 [email protected] Roll Fuchsia Linux SDK from 4IewCwqcEm7r4QsXd... to i7RS-4iBtdndwa3Jy... (flutter/flutter#172808) 2025-07-26 98614782+auto-submit[bot]@users.noreply.github.com Reverts "fix: get content hash for master on local engine branches (#172792)" (flutter/flutter#172805) 2025-07-26 [email protected] fix: build size_exp artifacts in post (flutter/flutter#172802) 2025-07-26 [email protected] fix: get content hash for master on local engine branches (flutter/flutter#172792) 2025-07-25 [email protected] Update `main`/`master` repoExceptions analysis set (flutter/flutter#172796) 2025-07-25 [email protected] Remove deprecated `--[no-]-disable-dds` (flutter/flutter#172791) 2025-07-25 [email protected] `last_engine_commit.ps1`: Use `$flutterRoot` instead of `$gitTopLevel` (flutter/flutter#172786) 2025-07-25 [email protected] Check file contents, remove tests that are now invalid (`: 'none'`) (flutter/flutter#172788) 2025-07-25 [email protected] windows: allow pwsh.bat wrapper (flutter/flutter#171778) 2025-07-25 [email protected] [Impeller] Log glCheckFrameBufferStatus output when it fails in blit pass. (flutter/flutter#172592) 2025-07-25 [email protected] Remove deprecated `AssetManifest.json` file (flutter/flutter#172594) 2025-07-25 [email protected] Refactor `verifyVersion` into typed `Version` object, use it (flutter/flutter#172780) 2025-07-25 [email protected] Bump the customer tests to pick up an update to Zulip's tests, for th… (flutter/flutter#172733) 2025-07-25 [email protected] Rename `AppRunLogger`, stop writing status messages that break JSON (flutter/flutter#172591) 2025-07-25 [email protected] Add `--dart-define`, `-D` to `assemble`, deprecate `--define`, `-d`. (flutter/flutter#172510) 2025-07-25 [email protected] Fix previous text input connection interrupts enter/space (flutter/flutter#171973) 2025-07-25 [email protected] Update CHANGELOG.md for 3.32.8 (flutter/flutter#172779) 2025-07-25 [email protected] Fix an `omit-type` lint that is breaking the tree (flutter/flutter#172776) 2025-07-25 [email protected] feat(tool): Respect user-data-dir flag from web-browser-flag (flutter/flutter#169445) 2025-07-25 [email protected] [web] Fix empty first frame in multiview mode (flutter/flutter#172493) 2025-07-25 [email protected] Migrate to null aware elements - Part 3 (flutter/flutter#172307) 2025-07-25 [email protected] Update few links in docs (flutter/flutter#172166) 2025-07-25 [email protected] Roll Skia from 768ace3fd144 to 409e1c7ba09b (1 revision) (flutter/flutter#172745) 2025-07-25 [email protected] Roll Fuchsia Linux SDK from a_55b5hqpRnigzuNC... to 4IewCwqcEm7r4QsXd... (flutter/flutter#172742) 2025-07-25 [email protected] Roll Skia from 094ac350125f to 768ace3fd144 (18 revisions) (flutter/flutter#172735) 2025-07-25 [email protected] Remove 2023 deprecated `'platforms'` key from daemon output (flutter/flutter#172593) 2025-07-24 [email protected] Manual roll of Dart from 26dece893751 to 5ede85aff798 (flutter/flutter#172731) 2025-07-24 [email protected] 169108 ci debug (flutter/flutter#172589) 2025-07-24 [email protected] Improve assertion message in `_MixedBorderRadius.resolve()` (flutter/flutter#172100) 2025-07-24 [email protected] Make sure that a BottomAppBar doesn't crash in 0x0 environment (flutter/flutter#172097) 2025-07-24 [email protected] Handle#6537 bottom navigation bar (flutter/flutter#172404) 2025-07-24 [email protected] Make sure that a MaterialBanner doesn't crash in 0x0 environment (flutter/flutter#172074) 2025-07-24 [email protected] Remove support for `pluginClass: none` (on `master`; `> 3.35.0`) (flutter/flutter#172579) 2025-07-24 [email protected] Make sure that a Card doesn't crash in 0x0 environment (flutter/flutter#172373) 2025-07-24 [email protected] Make sure that an AboutDialog doesn't crash in 0x0 environment (flutter/flutter#172444) 2025-07-24 [email protected] Make sure that an ActionChip doesn't crash in 0x0 environment (flutter/flutter#172684) 2025-07-24 [email protected] Delete (unused) `dev/checks_tool` directory (flutter/flutter#172715) 2025-07-24 [email protected] [Android] Add missing ci configuration for `Linux android_java17_tool_integration_tests` (flutter/flutter#172702) 2025-07-24 [email protected] Flutter-Cherrypick-Process.md - Spell cherry-pick consistently (flutter/flutter#172697) 2025-07-24 [email protected] Bringup `linux_web_engine_tests`; failing 4+ times in a row (flutter/flutter#172714) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: ...
…1973) Fixes an issue where a previously closed text input connection may cause interference with hardware keyboard on Samsung devices. On the Samsung Galaxy S10 Tab the Samsung IME will request the operating system to show the IME when pressing enter/space on components like a radio option or a checkbox if there was a previously opened text input connection. It seems the Samsung Keyboard is caching some text input related state, and when it detects an enter/space key it acts on this previously cached state even if the text input connection has been closed. This prevents the radio option/checkbox and framework as a whole from responding to the key events. To solve this issue we should reset the input method manager when the IME has been hidden, and when the framework closes the input connection we should also reset the input method manager if the IME is hidden at the time of closing. This effectively resets any state cached by the Samsung keyboard and it no longer acts on stale state. Fixes flutter#168099 Fixes flutter#51478 Fixes flutter#70546 Related: flutter#136745 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. --------- Co-authored-by: Renzo Olivares <[email protected]>
…1973) Fixes an issue where a previously closed text input connection may cause interference with hardware keyboard on Samsung devices. On the Samsung Galaxy S10 Tab the Samsung IME will request the operating system to show the IME when pressing enter/space on components like a radio option or a checkbox if there was a previously opened text input connection. It seems the Samsung Keyboard is caching some text input related state, and when it detects an enter/space key it acts on this previously cached state even if the text input connection has been closed. This prevents the radio option/checkbox and framework as a whole from responding to the key events. To solve this issue we should reset the input method manager when the IME has been hidden, and when the framework closes the input connection we should also reset the input method manager if the IME is hidden at the time of closing. This effectively resets any state cached by the Samsung keyboard and it no longer acts on stale state. Fixes flutter#168099 Fixes flutter#51478 Fixes flutter#70546 Related: flutter#136745 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. --------- Co-authored-by: Renzo Olivares <[email protected]>
…1973) Fixes an issue where a previously closed text input connection may cause interference with hardware keyboard on Samsung devices. On the Samsung Galaxy S10 Tab the Samsung IME will request the operating system to show the IME when pressing enter/space on components like a radio option or a checkbox if there was a previously opened text input connection. It seems the Samsung Keyboard is caching some text input related state, and when it detects an enter/space key it acts on this previously cached state even if the text input connection has been closed. This prevents the radio option/checkbox and framework as a whole from responding to the key events. To solve this issue we should reset the input method manager when the IME has been hidden, and when the framework closes the input connection we should also reset the input method manager if the IME is hidden at the time of closing. This effectively resets any state cached by the Samsung keyboard and it no longer acts on stale state. Fixes flutter#168099 Fixes flutter#51478 Fixes flutter#70546 Related: flutter#136745 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. --------- Co-authored-by: Renzo Olivares <[email protected]>
…1973) Fixes an issue where a previously closed text input connection may cause interference with hardware keyboard on Samsung devices. On the Samsung Galaxy S10 Tab the Samsung IME will request the operating system to show the IME when pressing enter/space on components like a radio option or a checkbox if there was a previously opened text input connection. It seems the Samsung Keyboard is caching some text input related state, and when it detects an enter/space key it acts on this previously cached state even if the text input connection has been closed. This prevents the radio option/checkbox and framework as a whole from responding to the key events. To solve this issue we should reset the input method manager when the IME has been hidden, and when the framework closes the input connection we should also reset the input method manager if the IME is hidden at the time of closing. This effectively resets any state cached by the Samsung keyboard and it no longer acts on stale state. Fixes flutter#168099 Fixes flutter#51478 Fixes flutter#70546 Related: flutter#136745 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. --------- Co-authored-by: Renzo Olivares <[email protected]>
Fixes an issue where a previously closed text input connection may cause interference with hardware keyboard on Samsung devices.
On the Samsung Galaxy S10 Tab the Samsung IME will request the operating system to show the IME when pressing enter/space on components like a radio option or a checkbox if there was a previously opened text input connection. It seems the Samsung Keyboard is caching some text input related state, and when it detects an enter/space key it acts on this previously cached state even if the text input connection has been closed. This prevents the radio option/checkbox and framework as a whole from responding to the key events. To solve this issue we should reset the input method manager when the IME has been hidden, and when the framework closes the input connection we should also reset the input method manager if the IME is hidden at the time of closing. This effectively resets any state cached by the Samsung keyboard and it no longer acts on stale state.
Fixes #168099
Fixes #51478
Fixes #70546
Related:
#136745
Pre-launch Checklist
///).