[macOS] Put FlutterTextInputPlugin in view hierarchy (flutter#33827)

Author: knopp

shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.mm

@@ -128,7 +128,7 @@
         // first responder.
         FlutterTextField* native_text_field = (FlutterTextField*)focused;
         if (native_text_field == mac_platform_node_delegate->GetFocus()) {
-          [native_text_field becomeFirstResponder];
+          [native_text_field startEditing];
         }
         break;
       }
@@ -172,7 +172,7 @@
             (FlutterTextField*)mac_platform_node_delegate->GetNativeViewAccessible();
         id focused = mac_platform_node_delegate->GetFocus();
         if (!focused || native_text_field == focused) {
-          [native_text_field becomeFirstResponder];
+          [native_text_field startEditing];
         }
         break;
       }

shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.h

@@ -39,4 +39,13 @@
  */
 - (void)handleEvent:(nonnull NSEvent*)event;
 
+/**
+ * Returns yes if is event currently being redispatched.
+ *
+ * In some instances (i.e. emoji shortcut) the event may be redelivered by cocoa
+ * as key equivalent to FlutterTextInput, in which case it shouldn't be
+ * processed again.
+ */
+- (BOOL)isDispatchingKeyEvent:(nonnull NSEvent*)event;
+
 @end

shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.mm

@@ -70,6 +70,8 @@ @interface FlutterKeyboardManager ()
 
 @property(nonatomic) NSMutableDictionary<NSNumber*, NSNumber*>* layoutMap;
 
+@property(nonatomic, nullable) NSEvent* eventBeingDispatched;
+
 /**
  * Add a primary responder, which asynchronously decides whether to handle an
  * event.
@@ -168,6 +170,10 @@ - (void)handleEvent:(nonnull NSEvent*)event {
   [self processNextEvent];
 }
 
+- (BOOL)isDispatchingKeyEvent:(NSEvent*)event {
+  return _eventBeingDispatched == event;
+}
+
 #pragma mark - Private
 
 - (void)processNextEvent {
@@ -230,6 +236,8 @@ - (void)dispatchTextEvent:(NSEvent*)event {
   if (nextResponder == nil) {
     return;
   }
+  NSAssert(_eventBeingDispatched == nil, @"An event is already being dispached.");
+  _eventBeingDispatched = event;
   switch (event.type) {
     case NSEventTypeKeyDown:
       if ([nextResponder respondsToSelector:@selector(keyDown:)]) {
@@ -249,6 +257,8 @@ - (void)dispatchTextEvent:(NSEvent*)event {
     default:
       NSAssert(false, @"Unexpected key event type (got %lu).", event.type);
   }
+  NSAssert(_eventBeingDispatched != nil, @"_eventBeingDispatched was cleared unexpectedly.");
+  _eventBeingDispatched = nil;
 }
 
 - (void)buildLayout {

shell/platform/darwin/macos/framework/Source/FlutterPlatformNodeDelegateMacTest.mm

@@ -293,9 +293,8 @@
   EXPECT_EQ(NSEqualRects(native_text_field.frame, NSMakeRect(0, 600 - expectedFrameSize,
                                                              expectedFrameSize, expectedFrameSize)),
             YES);
-  // The text of TextInputPlugin only starts syncing editing state to the
-  // native text field when it becomes the first responder.
-  [native_text_field becomeFirstResponder];
+
+  [native_text_field startEditing];
   EXPECT_EQ([native_text_field.stringValue isEqualToString:@"textfield"], YES);
 }
 

shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm

@@ -237,6 +237,16 @@ - (void)dealloc {
 
 #pragma mark - Private
 
+- (void)resignAndRemoveFromSuperview {
+  if (self.superview != nil) {
+    // With accessiblity enabled TextInputPlugin is inside _client, so take the
+    // nextResponder from the _client.
+    NSResponder* nextResponder = _client != nil ? _client.nextResponder : self.nextResponder;
+    [self.window makeFirstResponder:nextResponder];
+    [self removeFromSuperview];
+  }
+}
+
 - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result {
   BOOL handled = YES;
   NSString* method = call.method;
@@ -262,12 +272,19 @@ - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result {
       _activeModel = std::make_unique<flutter::TextInputModel>();
     }
   } else if ([method isEqualToString:kShowMethod]) {
+    // Ensure the plugin is in hierarchy. Only do this with accessibility disabled.
+    // When accessibility is enabled cocoa will reparent the plugin inside
+    // FlutterTextField in [FlutterTextField startEditing].
+    if (_client == nil) {
+      [_flutterViewController.view addSubview:self];
+    }
+    [self.window makeFirstResponder:self];
     _shown = TRUE;
-    [_textInputContext activate];
   } else if ([method isEqualToString:kHideMethod]) {
+    [self resignAndRemoveFromSuperview];
     _shown = FALSE;
-    [_textInputContext deactivate];
   } else if ([method isEqualToString:kClearClientMethod]) {
+    [self resignAndRemoveFromSuperview];
     // If there's an active mark region, commit it, end composing, and clear the IME's mark text.
     if (_activeModel && _activeModel->composing()) {
       _activeModel->CommitComposing();
@@ -362,7 +379,8 @@ - (void)setEditingState:(NSDictionary*)state {
   if (composing_range.collapsed() && wasComposing) {
     [_textInputContext discardMarkedText];
   }
-  [_client becomeFirstResponder];
+  [_client startEditing];
+
   [self updateTextAndSelection];
 }
 
@@ -464,12 +482,6 @@ - (BOOL)handleKeyEvent:(NSEvent*)event {
     return NO;
   }
 
-  // NSTextInputContext sometimes deactivates itself without calling
-  // deactivate. One such example is when the composing region is deleted.
-  // TODO(LongCatIsLooong): put FlutterTextInputPlugin in the view hierarchy and
-  // request/resign first responder when needed. Activate/deactivate shouldn't
-  // be called by the application.
-  [_textInputContext activate];
   return [_textInputContext handleEvent:event];
 }
 
@@ -485,7 +497,20 @@ - (void)keyUp:(NSEvent*)event {
 }
 
 - (BOOL)performKeyEquivalent:(NSEvent*)event {
-  return [self.flutterViewController performKeyEquivalent:event];
+  if ([_flutterViewController isDispatchingKeyEvent:event]) {
+    // When NSWindow is nextResponder, keyboard manager will send to it
+    // unhandled events (through [NSWindow keyDown:]). If event has has both
+    // control and cmd modifiers set (i.e. cmd+control+space - emoji picker)
+    // NSWindow will then send this event as performKeyEquivalent: to first
+    // responder, which is FlutterTextInputPlugin. If that's the case, the
+    // plugin must not handle the event, otherwise the emoji picker would not
+    // work (due to first responder returning YES from performKeyEquivalent:)
+    // and there would be endless loop, because FlutterViewController will
+    // send the event back to [keyboardManager handleEvent:].
+    return NO;
+  }
+  [self.flutterViewController keyDown:event];
+  return YES;
 }
 
 - (void)flagsChanged:(NSEvent*)event {

shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm

@@ -250,60 +250,6 @@ - (bool)testComposingRegionRemovedByFramework {
   return true;
 }
 
-- (bool)testInputContextIsKeptActive {
-  id engineMock = OCMClassMock([FlutterEngine class]);
-  FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engineMock
-                                                                                nibName:@""
-                                                                                 bundle:nil];
-
-  FlutterTextInputPlugin* plugin =
-      [[FlutterTextInputPlugin alloc] initWithViewController:viewController];
-
-  [plugin handleMethodCall:[FlutterMethodCall
-                               methodCallWithMethodName:@"TextInput.setClient"
-                                              arguments:@[
-                                                @(1), @{
-                                                  @"inputAction" : @"action",
-                                                  @"inputType" : @{@"name" : @"inputName"},
-                                                }
-                                              ]]
-                    result:^(id){
-                    }];
-
-  [plugin handleMethodCall:[FlutterMethodCall methodCallWithMethodName:@"TextInput.setEditingState"
-                                                             arguments:@{
-                                                               @"text" : @"",
-                                                               @"selectionBase" : @(0),
-                                                               @"selectionExtent" : @(0),
-                                                               @"composingBase" : @(-1),
-                                                               @"composingExtent" : @(-1),
-                                                             }]
-                    result:^(id){
-                    }];
-
-  [plugin handleMethodCall:[FlutterMethodCall methodCallWithMethodName:@"TextInput.show"
-                                                             arguments:@[]]
-                    result:^(id){
-                    }];
-
-  [plugin.inputContext deactivate];
-  EXPECT_EQ(plugin.inputContext.isActive, NO);
-  NSEvent* keyEvent = [NSEvent keyEventWithType:NSEventTypeKeyDown
-                                       location:NSZeroPoint
-                                  modifierFlags:0x100
-                                      timestamp:0
-                                   windowNumber:0
-                                        context:nil
-                                     characters:@""
-                    charactersIgnoringModifiers:@""
-                                      isARepeat:NO
-                                        keyCode:0x50];
-
-  [plugin handleKeyEvent:keyEvent];
-  EXPECT_EQ(plugin.inputContext.isActive, YES);
-  return true;
-}
-
 - (bool)testClearClientDuringComposing {
   // Set up FlutterTextInputPlugin.
   id engineMock = OCMClassMock([FlutterEngine class]);
@@ -917,6 +863,63 @@ - (bool)testComposingWithDeltasWhenSelectionIsActive {
   return true;
 }
 
+- (bool)testPerformKeyEquivalent {
+  __block NSEvent* eventBeingDispatchedByKeyboardManager = nil;
+  FlutterViewController* viewControllerMock = OCMClassMock([FlutterViewController class]);
+  OCMStub([viewControllerMock isDispatchingKeyEvent:[OCMArg any]])
+      .andDo(^(NSInvocation* invocation) {
+        NSEvent* event;
+        [invocation getArgument:(void*)&event atIndex:2];
+        BOOL result = event == eventBeingDispatchedByKeyboardManager;
+        [invocation setReturnValue:&result];
+      });
+
+  NSEvent* event = [NSEvent keyEventWithType:NSEventTypeKeyDown
+                                    location:NSZeroPoint
+                               modifierFlags:0x100
+                                   timestamp:0
+                                windowNumber:0
+                                     context:nil
+                                  characters:@""
+                 charactersIgnoringModifiers:@""
+                                   isARepeat:NO
+                                     keyCode:0x50];
+
+  FlutterTextInputPlugin* plugin =
+      [[FlutterTextInputPlugin alloc] initWithViewController:viewControllerMock];
+
+  OCMExpect([viewControllerMock keyDown:event]);
+
+  // Require that event is handled (returns YES)
+  if (![plugin performKeyEquivalent:event]) {
+    return false;
+  };
+
+  @try {
+    OCMVerify(  // NOLINT(google-objc-avoid-throwing-exception)
+        [viewControllerMock keyDown:event]);
+  } @catch (...) {
+    return false;
+  }
+
+  // performKeyEquivalent must not forward event if it is being
+  // dispatched by keyboard manager
+  eventBeingDispatchedByKeyboardManager = event;
+
+  OCMReject([viewControllerMock keyDown:event]);
+  @try {
+    // Require that event is not handled (returns NO) and not
+    // forwarded to controller
+    if ([plugin performKeyEquivalent:event]) {
+      return false;
+    };
+  } @catch (...) {
+    return false;
+  }
+
+  return true;
+}
+
 - (bool)testLocalTextAndSelectionUpdateAfterDelta {
   id engineMock = OCMClassMock([FlutterEngine class]);
   id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger));
@@ -1005,10 +1008,6 @@ - (bool)testLocalTextAndSelectionUpdateAfterDelta {
   ASSERT_TRUE([[FlutterInputPluginTestObjc alloc] testComposingRegionRemovedByFramework]);
 }
 
-TEST(FlutterTextInputPluginTest, TestTextInputContextIsKeptAlive) {
-  ASSERT_TRUE([[FlutterInputPluginTestObjc alloc] testInputContextIsKeptActive]);
-}
-
 TEST(FlutterTextInputPluginTest, TestClearClientDuringComposing) {
   ASSERT_TRUE([[FlutterInputPluginTestObjc alloc] testClearClientDuringComposing]);
 }
@@ -1037,6 +1036,10 @@ - (bool)testLocalTextAndSelectionUpdateAfterDelta {
   ASSERT_TRUE([[FlutterInputPluginTestObjc alloc] testLocalTextAndSelectionUpdateAfterDelta]);
 }
 
+TEST(FlutterTextInputPluginTest, TestPerformKeyEquivalent) {
+  ASSERT_TRUE([[FlutterInputPluginTestObjc alloc] testPerformKeyEquivalent]);
+}
+
 TEST(FlutterTextInputPluginTest, CanWorkWithFlutterTextField) {
   FlutterEngine* engine = CreateTestEngine();
   NSString* fixtures = @(testing::GetFixturesPath());
@@ -1069,7 +1072,7 @@ - (bool)testLocalTextAndSelectionUpdateAfterDelta {
       [[FlutterTextFieldMock alloc] initWithPlatformNode:&text_platform_node
                                              fieldEditor:viewController.textInputPlugin];
   [viewController.view addSubview:mockTextField];
-  [mockTextField becomeFirstResponder];
+  [mockTextField startEditing];
 
   NSDictionary* arguments = @{
     @"inputAction" : @"action",
@@ -1133,4 +1136,40 @@ - (bool)testLocalTextAndSelectionUpdateAfterDelta {
   EXPECT_EQ([textField becomeFirstResponder], NO);
 }
 
+TEST(FlutterTextInputPluginTest, IsAddedAndRemovedFromViewHierarchy) {
+  FlutterEngine* engine = CreateTestEngine();
+  NSString* fixtures = @(testing::GetFixturesPath());
+  FlutterDartProject* project = [[FlutterDartProject alloc]
+      initWithAssetsPath:fixtures
+             ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
+  FlutterViewController* viewController = [[FlutterViewController alloc] initWithProject:project];
+  [viewController loadView];
+  [engine setViewController:viewController];
+
+  NSWindow* window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 800, 600)
+                                                 styleMask:NSBorderlessWindowMask
+                                                   backing:NSBackingStoreBuffered
+                                                     defer:NO];
+  window.contentView = viewController.view;
+
+  ASSERT_EQ(viewController.textInputPlugin.superview, nil);
+  ASSERT_FALSE(window.firstResponder == viewController.textInputPlugin);
+
+  [viewController.textInputPlugin
+      handleMethodCall:[FlutterMethodCall methodCallWithMethodName:@"TextInput.show" arguments:@[]]
+                result:^(id){
+                }];
+
+  ASSERT_EQ(viewController.textInputPlugin.superview, viewController.view);
+  ASSERT_TRUE(window.firstResponder == viewController.textInputPlugin);
+
+  [viewController.textInputPlugin
+      handleMethodCall:[FlutterMethodCall methodCallWithMethodName:@"TextInput.hide" arguments:@[]]
+                result:^(id){
+                }];
+
+  ASSERT_EQ(viewController.textInputPlugin.superview, nil);
+  ASSERT_FALSE(window.firstResponder == viewController.textInputPlugin);
+}
+
 }  // namespace flutter::testing

shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObject.h

@@ -89,4 +89,10 @@ class FlutterTextPlatformNode : public ui::AXPlatformNodeBase {
  */
 - (void)updateString:(NSString*)string withSelection:(NSRange)selection;
 
+/**
+ * Makes the field editor (plugin) current editor for this TextField, meaning
+ * that the text field will start getting editing events.
+ */
+- (void)startEditing;
+
 @end