[macOS] Put FlutterTextInputPlugin in view hierarchy #33827
Conversation
knopp
force-pushed
the
text_input_plugin_in_view_hierarchy
branch
from
eef9255
to
3f51a94
Compare
knopp
changed the title
knopp
changed the title
knopp
force-pushed
the
text_input_plugin_in_view_hierarchy
branch
from
00a82f0
to
f50304a
Compare
shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.h
Outdated
Show resolved
Hide resolved
| @@ -262,11 +268,22 @@ - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result { | |||
| _activeModel = std::make_unique<flutter::TextInputModel>(); | |||
| } | |||
| } else if ([method isEqualToString:kShowMethod]) { | |||
| // Ensure the plugin is in hierarchy. | |||
| // When accessibility text field becomes first responder AppKit sometimes | |||
| // removes the plugin from hierarchy. | |||
There was a problem hiding this comment.
I couldn't find the place where the text input plugin is added to the view hierarchy. You mean with this change the accessibility text field still causes the text input plugin to be removed as a subview when it has the focus?
There was a problem hiding this comment.
When FlutterTextField has focus and FlutterTextInputPlugin becomes the field editor of ([FlutterTextField startEditing]) the input plugin gets reparented inside the text field.
shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm
Outdated
Show resolved
Hide resolved
| @@ -362,7 +379,9 @@ - (void)setEditingState:(NSDictionary*)state { | |||
| if (composing_range.collapsed() && wasComposing) { | |||
| [_textInputContext discardMarkedText]; | |||
| } | |||
| [_client becomeFirstResponder]; | |||
| if (_client != nil) { | |||
| [self.window makeFirstResponder:_client]; | |||
There was a problem hiding this comment.
thanks for fixing this! 👍
There was a problem hiding this comment.
This was the wrong fix. The TextInputField is actually not meant to be first responder. Instead the FlutterTextInputPlugin is supposed to be the first responder. What the original becomeFirstResponder did was not making the text field first responder, it made FlutterTextInputPlugin current field editor for the text field. It happened as a sideeffect of becomeFirstResponder calling selectText, which sets the field editor as current editor.
So instead of overriding becomeFirstResponder and calling it directly (which shouldn't be done) I replaced it with [FlutterTextField startEditing], which makes the text input plugin current field editor.
knopp
force-pushed
the
text_input_plugin_in_view_hierarchy
branch
2 times, most recently
from
75fdba0
to
eb2b414
Compare
knopp
force-pushed
the
text_input_plugin_in_view_hierarchy
branch
2 times, most recently
from
252ba98
to
4c8aa84
Compare
|
@cbracken, @LongCatIsLooong, any objections to getting this merged? |
shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm
Outdated
Show resolved
Hide resolved
Emoji picker is different than the other IME popovers. In order for the picker to be attached properly,
FlutterTextInputPluginmust be in view hierarchy and first responder.Notable changes in this PR:
FlutterTextInputPluginis added to hierarchy inTextInput.show, removed inTextInput.hide.[TextInputContext activate]anddeactivatecalls are not needed anymore and were removed. The context gets activated automatically now. I also removed theTestTextInputContextIsKeptAliveunit test. Unfortunately I wasn't able to get the context to activate in "headless" unit test environment, but in actual application the context gets activated/deactivated as expected (tested by overriding theactivate/deactivatemethods on context).performKeyEquivalent:. ThekeyUpevent monitor inFlutterViewControllertakes care of this. It needed to be modified to allowTextInputPluginas first responder (which is also works with accessibility text field)[FlutterTestInputPlugin performKeyEquivalent:]now simply calls[FlutterViewController keyDown:]. With one caveat: When theCMD+CTRL+SPACEemoji picker shortcut event gets redispached tonextResponderbyFlutterKeyboardManager, thenextResponderis usuallyNSWindowand it routes the event to[FlutterTestInputPlugin performKeyEquivalent:]. Sending it again to controller would result in endless cycle, so theFlutterTextInputPluginchecks if the event in question is currently being redispatched by manager and if it is, it will not handle it. Not handling the event is also required for the emoji picker to actually show up.becomeFirstResponderwith[NSWindow makeFirstResponder:]. Unlike UIKit, in AppKit the becomeFirstResponder documentation explicitely says not to call this method directly and in some cases it leads to crash with unhandledNSInternalInconsistencyException.[FlutterTextField becomeFirstResponder], which was called directly has been replaced by[FlutterTextField startEditing]. This method setsFlutterTextPluginas field editor to theFlutterTextField. This was previously happening as a side effect ofbecomeFirstRespondercallingselectText.Partially fixes flutter/flutter#85328
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.