-
Notifications
You must be signed in to change notification settings - Fork 6k
[macOS] Clear IME mark text on clear input client #31849
[macOS] Clear IME mark text on clear input client #31849
Conversation
When the embedder receives a TextInput.clearClient message from the framework (typically when a text field loses focus), if the user is currently inputting composing text using an IME, commit the composing text, end composing, and clear the IME's composing state. This also exposes a public `editingState` getter on FlutterTextInputPlugin as part of the TestMethods informal protocol. This allows us to get at the text editing state as a dictionary in tests. Issue: flutter/flutter#92060
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.
One question about the text in the field after unfocusing, otherwise LGTM 👍
I'm not the strictest person for engine code quality, but the way you tested this seems ok to me. I also have struggled to find ways to test stuff like this without exposing something, and it's better than not testing it properly I think.
| _activeModel->CommitComposing(); | ||
| _activeModel->EndComposing(); |
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.
Is the end result that the composing text (e.g. "nihao") is in the field or the first choice characters ("你好")? If I try this on non-Flutter web or Mac, it seems to keep the composing text.
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.
Yep the end result is that we commit the current composing text and it remains in the field. Then we clear it from the IME's buffer.
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.
Cool that matches what I see on native then 👍
| if (!composing_range.collapsed() && !_activeModel->composing()) { | ||
| _activeModel->BeginComposing(); |
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.
Does this let you resume composing after unfocusing the field and coming back? Or maybe it's just cleanup.
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 is technically unrelated cleanup. This just ensures that if the framework tells us we have composing text, but the engine wasn't already composing, we set it to composing mode. Adding the test uncovered this bug.
When the embedder receives a TextInput.clearClient message from the
framework (typically when a text field loses focus), if the user is
currently inputting composing text using an IME, commit the composing
text, end composing, and clear the IME's composing state.
This also exposes a public
editingStategetter onFlutterTextInputPlugin as part of the TestMethods informal protocol.
This allows us to get at the text editing state as a dictionary in
tests.
Issue: flutter/flutter#92060
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.