Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Cancel IME composing on clear text input client - #29146

Merged
cbracken merged 6 commits into
flutter-team-archive:masterfrom
cbracken:reset-composing-on-lose-focus
Oct 13, 2021
Merged

Cancel IME composing on clear text input client#29146
cbracken merged 6 commits into
flutter-team-archive:masterfrom
cbracken:reset-composing-on-lose-focus

Conversation

@cbracken

@cbracken cbracken commented Oct 12, 2021

Copy link
Copy Markdown
Contributor

When a text input client is cleared, such as when the user changes focus
from one text input widget to another, or when the window itself loses
focus, cancel IME composing mode, close the composing candidates window
(if any), and reset the composing text.

Issue: flutter/flutter#90503

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.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@google-cla google-cla Bot added the cla: yes label Oct 12, 2021
stuartmorgan-g
stuartmorgan-g approved these changes Oct 13, 2021
View reviewed changes

@stuartmorgan-g stuartmorgan-g left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with nit.

Comment thread shell/platform/windows/text_input_plugin_unittest.cc Resolved
@cbracken
Cancel IME composing on clear text input client
af6a80c 
When a text input client is cleared, such as when the user changes focus
from one text input widget to another, or when the window itself loses
focus, cancel IME composing mode, close the composing candidates window
(if any), and reset the composing text.

Issue: flutter/flutter#90503
@cbracken
Review feedback: Make ime_was_reset_ private
d961d51
@cbracken
Formatting fix
37cba10
@cbracken
Guard against a potential nullptr deref
cb2f63a
@cbracken
Rename local variable
ae14dfd
@cbracken
Revert formatting fix
ccb5239
@cbracken
cbracken merged commit de07651 into flutter-team-archive:master Oct 13, 2021
@cbracken
cbracken deleted the reset-composing-on-lose-focus branch October 13, 2021 17:29
@cbracken cbracken mentioned this pull request Oct 13, 2021
Closed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Oct 13, 2021
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 13, 2021
@engine-flutter-autoroll
de07651 Cancel IME composing on clear text input client (flutter-team…
fbbf72c 
…-archive/engine#29146)
@cbracken cbracken mentioned this pull request Oct 19, 2021
Closed
@cbracken cbracken mentioned this pull request Nov 9, 2021
Closed
@letungcntt letungcntt mentioned this pull request Nov 23, 2021
Open
@paul1989889

paul1989889 commented Dec 1, 2021

Copy link
Copy Markdown

I had tested this issue‘s code, but on windows os version 10.0.14393. when I switch focus node between two textfield，It would be crash， @cbracken

@dkwingsmt

dkwingsmt commented Dec 1, 2021

Copy link
Copy Markdown
Contributor

@paul1989889 Can you post your flutter doctor -v?

@paul1989889

paul1989889 commented Dec 2, 2021

Copy link
Copy Markdown

[√] Flutter (Channel master, 2.6.0-12.0.pre.868, on Microsoft Windows [Version 10.0.14393.79518], locale zh-CN)
• Flutter version 2.6.0-12.0.pre.868 at D:\thirdsdk\flutter
• Upstream repository https://github.com/flutter/flutter.git
• Framework revision fff85f9 (22 hours ago), 2021-11-30 23:09:05 -0500
• Engine revision c2fcade
• Dart version 2.16.0 (build 2.16.0-56.0.dev)
• DevTools version 2.8.0

[!] Android toolchain - develop for Android devices (Android SDK version 31.0.0)
• Android SDK at C:\Users\wujing\AppData\Local\Android\sdk
X cmdline-tools component is missing
Run path/to/sdkmanager --install "cmdline-tools;latest"
See https://developer.android.com/studio/command-line for more details.
X Android license status unknown.
Run flutter doctor --android-licenses to accept the SDK licenses.
See https://flutter.dev/docs/get-started/install/windows#android-setup for more details.

[√] Chrome - develop for the web
• Chrome at C:\Program Files\Google\Chrome\Application\chrome.exe

[√] Visual Studio - develop for Windows (Visual Studio Community 2019 16.10.4)
• Visual Studio at C:\Program Files (x86)\Microsoft Visual Studio\2019\Community
• Visual Studio Community 2019 version 16.10.31515.178
• Windows 10 SDK version 10.0.14393.795

[√] Android Studio (version 2020.3)
• Android Studio at C:\Program Files\Android\Android Studio
• Flutter plugin can be installed from:
https://plugins.jetbrains.com/plugin/9212-flutter
• Dart plugin can be installed from:
https://plugins.jetbrains.com/plugin/6351-dart
• Java version OpenJDK Runtime Environment (build 11.0.10+0-b96-7249189)

[√] VS Code (version 1.62.3)
• VS Code at C:\Users\wujing\AppData\Local\Programs\Microsoft VS Code
• Flutter extension version 3.28.0

[√] Connected device (2 available)
• Windows (desktop) • windows • windows-x64 • Microsoft Windows [Version 10.0.17763.2237]
• Chrome (web) • chrome • web-javascript • Google Chrome 96.0.4664.45

! Doctor found issues in 1 category.

@dkwingsmt

@dkwingsmt

dkwingsmt commented Dec 2, 2021

Copy link
Copy Markdown
Contributor

Yeah it does seem to have applied this PR. @cbracken

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@dkwingsmt dkwingsmt Awaiting requested review from dkwingsmt
1 more reviewer
@stuartmorgan-g stuartmorgan-g stuartmorgan-g approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes platform-windows

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cbracken @paul1989889 @dkwingsmt @stuartmorgan-g