From d81de323f4eeaa4e15ace56e1e245178933b37b8 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Sat, 4 Jun 2022 17:09:46 +0200 Subject: [PATCH 01/14] Put FlutterTextInputPlugin in view hierarchy --- .../Source/AccessibilityBridgeMacDelegate.mm | 4 +- .../framework/Source/FlutterKeyboardManager.h | 9 ++ .../Source/FlutterKeyboardManager.mm | 2 + .../FlutterPlatformNodeDelegateMacTest.mm | 2 +- .../Source/FlutterTextInputPlugin.mm | 43 +++++++--- .../Source/FlutterTextInputPluginTest.mm | 58 ------------- .../framework/Source/FlutterViewController.mm | 36 +------- .../Source/FlutterViewControllerTest.mm | 85 ------------------- .../Source/FlutterViewController_Internal.h | 8 ++ 9 files changed, 59 insertions(+), 188 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.mm b/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.mm index a1f3ae9447e3e..4e6fe8c672f86 100644 --- a/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.mm +++ b/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.window makeFirstResponder:native_text_field]; } 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.window makeFirstResponder:native_text_field]; } break; } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.h b/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.h index 02a2565ee4575..cf9411a4a3fd2 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.h @@ -39,4 +39,13 @@ */ - (void)handleEvent:(nonnull NSEvent*)event; +/** + * The event currently being redispatched. + * + * In some instances (i.e. emoji shortcut) the event may redelivered by cocoa + * as key equivalent to FlutterTextInput, in which case it shouldn't be + * processed again. + */ +@property(nonatomic, nullable) NSEvent* eventBeingDispatched; + @end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.mm b/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.mm index 90ebdf5362865..80c3cab7b6286 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.mm @@ -230,6 +230,7 @@ - (void)dispatchTextEvent:(NSEvent*)event { if (nextResponder == nil) { return; } + _eventBeingDispatched = event; switch (event.type) { case NSEventTypeKeyDown: if ([nextResponder respondsToSelector:@selector(keyDown:)]) { @@ -249,6 +250,7 @@ - (void)dispatchTextEvent:(NSEvent*)event { default: NSAssert(false, @"Unexpected key event type (got %lu).", event.type); } + _eventBeingDispatched = nil; } - (void)buildLayout { diff --git a/shell/platform/darwin/macos/framework/Source/FlutterPlatformNodeDelegateMacTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterPlatformNodeDelegateMacTest.mm index b2932ff6f69b4..bb2b5c28c6880 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterPlatformNodeDelegateMacTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterPlatformNodeDelegateMacTest.mm @@ -295,7 +295,7 @@ 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.window makeFirstResponder:native_text_field]; EXPECT_EQ([native_text_field.stringValue isEqualToString:@"textfield"], YES); } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm index 1431dc2772f38..b1a45b781b990 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm @@ -138,6 +138,12 @@ @interface FlutterTextInputPlugin () */ @property(nonatomic) BOOL enableDeltaModel; +/** + * When plugin becomes first responder it remembers previous responder, + * which will be restored after the plugin is hidden. + */ +@property(nonatomic, weak) NSResponder* previousResponder; + /** * Handles a Flutter system message on the text input channel. */ @@ -262,11 +268,22 @@ - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result { _activeModel = std::make_unique(); } } 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. + if (_client == nil) { + [_flutterViewController.view addSubview:self]; + if (_previousResponder == nil) { + _previousResponder = self.window.firstResponder; + } + [self.window makeFirstResponder:self]; + } _shown = TRUE; - [_textInputContext activate]; } else if ([method isEqualToString:kHideMethod]) { + [self.window makeFirstResponder:_previousResponder]; + _previousResponder = nil; + [self removeFromSuperview]; _shown = FALSE; - [_textInputContext deactivate]; } else if ([method isEqualToString:kClearClientMethod]) { // If there's an active mark region, commit it, end composing, and clear the IME's mark text. if (_activeModel && _activeModel->composing()) { @@ -362,7 +379,9 @@ - (void)setEditingState:(NSDictionary*)state { if (composing_range.collapsed() && wasComposing) { [_textInputContext discardMarkedText]; } - [_client becomeFirstResponder]; + if (_client != nil) { + [self.window makeFirstResponder:_client]; + } [self updateTextAndSelection]; } @@ -464,12 +483,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]; } @@ -484,8 +497,18 @@ - (void)keyUp:(NSEvent*)event { [self.flutterViewController keyUp:event]; } +// Invoked through NSWindow processing of key down event. This can be either +// regular event sent from NSApplication with CMD modifier, in which case the +// event is processed as keyDown, or keyboard manager redispatching the event +// if nextResponder is NSWindow, in which case the event needs to be ignored, +// otherwise it will cause endless loop. - (BOOL)performKeyEquivalent:(NSEvent*)event { - return [self.flutterViewController performKeyEquivalent:event]; + if (_flutterViewController.keyboardManager.eventBeingDispatched == event) { + // This happens with cmd+contorl+space (emoji picker) + return NO; + } + [self.flutterViewController keyDown:event]; + return YES; } - (void)flagsChanged:(NSEvent*)event { diff --git a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm index a724b112b7239..bd5b2089c1745 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm +++ b/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]); @@ -1005,10 +951,6 @@ - (bool)testLocalTextAndSelectionUpdateAfterDelta { ASSERT_TRUE([[FlutterInputPluginTestObjc alloc] testComposingRegionRemovedByFramework]); } -TEST(FlutterTextInputPluginTest, TestTextInputContextIsKeptAlive) { - ASSERT_TRUE([[FlutterInputPluginTestObjc alloc] testInputContextIsKeptActive]); -} - TEST(FlutterTextInputPluginTest, TestClearClientDuringComposing) { ASSERT_TRUE([[FlutterInputPluginTestObjc alloc] testClearClientDuringComposing]); } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm index f01e3cfd26a00..5acf4eb8307d7 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm @@ -176,13 +176,6 @@ @interface FlutterViewController () */ @property(nonatomic) id keyUpMonitor; -/** - * Pointer to a keyboard manager, a hub that manages how key events are - * dispatched to various Flutter key responders, and whether the event is - * propagated to the next NSResponder. - */ -@property(nonatomic) FlutterKeyboardManager* keyboardManager; - @property(nonatomic) KeyboardLayoutNotifier keyboardLayoutNotifier; @property(nonatomic) NSData* keyboardLayoutData; @@ -431,10 +424,11 @@ - (void)listenForMetaModifiedKeyUpEvents { addLocalMonitorForEventsMatchingMask:NSEventMaskKeyUp handler:^NSEvent*(NSEvent* event) { // Intercept keyUp only for events triggered on the current - // view. + // view or textInputPlugin. + NSResponder* firstResponder = [[event window] firstResponder]; if (weakSelf.viewLoaded && weakSelf.flutterView && - ([[event window] firstResponder] == - weakSelf.flutterView) && + (firstResponder == weakSelf.flutterView || + firstResponder == weakSelf.textInputPlugin) && ([event modifierFlags] & NSEventModifierFlagCommand) && ([event type] == NSEventTypeKeyUp)) { [weakSelf keyUp:event]; @@ -481,7 +475,6 @@ - (void)initializeKeyboard { // TODO(goderbauer): Seperate keyboard/textinput stuff into ViewController specific and Engine // global parts. Move the global parts to FlutterEngine. __weak FlutterViewController* weakSelf = self; - _textInputPlugin = [[FlutterTextInputPlugin alloc] initWithViewController:weakSelf]; _keyboardManager = [[FlutterKeyboardManager alloc] initWithViewDelegate:weakSelf]; } @@ -740,27 +733,6 @@ - (void)keyUp:(NSEvent*)event { [_keyboardManager handleEvent:event]; } -- (BOOL)performKeyEquivalent:(NSEvent*)event { - [_keyboardManager handleEvent:event]; - if (event.type == NSEventTypeKeyDown) { - // macOS only sends keydown for performKeyEquivalent, but the Flutter framework - // always expects a keyup for every keydown. Synthesizes a key up event so that - // the Flutter framework continues to work. - NSEvent* synthesizedUp = [NSEvent keyEventWithType:NSEventTypeKeyUp - location:event.locationInWindow - modifierFlags:event.modifierFlags - timestamp:event.timestamp - windowNumber:event.windowNumber - context:event.context - characters:event.characters - charactersIgnoringModifiers:event.charactersIgnoringModifiers - isARepeat:event.isARepeat - keyCode:event.keyCode]; - [_keyboardManager handleEvent:synthesizedUp]; - } - return YES; -} - - (void)flagsChanged:(NSEvent*)event { [_keyboardManager handleEvent:event]; } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm index 6252453d78283..5677bd010f4ee 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm @@ -20,7 +20,6 @@ - (bool)testKeyEventsAreSentToFramework; - (bool)testKeyEventsArePropagatedIfNotHandled; - (bool)testKeyEventsAreNotPropagatedIfHandled; - (bool)testFlagsChangedEventsArePropagatedIfNotHandled; -- (bool)testPerformKeyEquivalentSynthesizesKeyUp; - (bool)testKeyboardIsRestartedOnEngineRestart; - (bool)testTrackpadGesturesAreSentToFramework; @@ -124,10 +123,6 @@ + (void)respondFalseForSendEvent:(const FlutterKeyEvent&)event [[FlutterViewControllerTestObjC alloc] testFlagsChangedEventsArePropagatedIfNotHandled]); } -TEST(FlutterViewControllerTest, TestPerformKeyEquivalentSynthesizesKeyUp) { - ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testPerformKeyEquivalentSynthesizesKeyUp]); -} - TEST(FlutterViewControllerTest, TestKeyboardIsRestartedOnEngineRestart) { ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testKeyboardIsRestartedOnEngineRestart]); } @@ -341,86 +336,6 @@ - (bool)testKeyEventsAreNotPropagatedIfHandled { return true; } -- (bool)testPerformKeyEquivalentSynthesizesKeyUp { - id engineMock = OCMClassMock([FlutterEngine class]); - id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); - OCMStub( // NOLINT(google-objc-avoid-throwing-exception) - [engineMock binaryMessenger]) - .andReturn(binaryMessengerMock); - OCMStub([[engineMock ignoringNonObjectArgs] sendKeyEvent:FlutterKeyEvent {} - callback:nil - userData:nil]) - .andCall([FlutterViewControllerTestObjC class], - @selector(respondFalseForSendEvent:callback:userData:)); - FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engineMock - nibName:@"" - bundle:nil]; - id responderMock = flutter::testing::mockResponder(); - viewController.nextResponder = responderMock; - NSDictionary* expectedKeyDownEvent = @{ - @"keymap" : @"macos", - @"type" : @"keydown", - @"keyCode" : @(65), - @"modifiers" : @(538968064), - @"characters" : @".", - @"charactersIgnoringModifiers" : @".", - }; - NSData* encodedKeyDownEvent = - [[FlutterJSONMessageCodec sharedInstance] encode:expectedKeyDownEvent]; - NSDictionary* expectedKeyUpEvent = @{ - @"keymap" : @"macos", - @"type" : @"keyup", - @"keyCode" : @(65), - @"modifiers" : @(538968064), - @"characters" : @".", - @"charactersIgnoringModifiers" : @".", - }; - NSData* encodedKeyUpEvent = [[FlutterJSONMessageCodec sharedInstance] encode:expectedKeyUpEvent]; - CGEventRef cgEvent = CGEventCreateKeyboardEvent(NULL, 65, TRUE); - NSEvent* event = [NSEvent eventWithCGEvent:cgEvent]; - OCMExpect( // NOLINT(google-objc-avoid-throwing-exception) - [binaryMessengerMock sendOnChannel:@"flutter/keyevent" - message:encodedKeyDownEvent - binaryReply:[OCMArg any]]) - .andDo((^(NSInvocation* invocation) { - FlutterBinaryReply handler; - [invocation getArgument:&handler atIndex:4]; - NSDictionary* reply = @{ - @"handled" : @(true), - }; - NSData* encodedReply = [[FlutterJSONMessageCodec sharedInstance] encode:reply]; - handler(encodedReply); - })); - OCMExpect( // NOLINT(google-objc-avoid-throwing-exception) - [binaryMessengerMock sendOnChannel:@"flutter/keyevent" - message:encodedKeyUpEvent - binaryReply:[OCMArg any]]) - .andDo((^(NSInvocation* invocation) { - FlutterBinaryReply handler; - [invocation getArgument:&handler atIndex:4]; - NSDictionary* reply = @{ - @"handled" : @(true), - }; - NSData* encodedReply = [[FlutterJSONMessageCodec sharedInstance] encode:reply]; - handler(encodedReply); - })); - [viewController viewWillAppear]; // Initializes the event channel. - [viewController performKeyEquivalent:event]; - @try { - OCMVerify( // NOLINT(google-objc-avoid-throwing-exception) - [binaryMessengerMock sendOnChannel:@"flutter/keyevent" - message:encodedKeyDownEvent - binaryReply:[OCMArg any]]); - OCMVerify( // NOLINT(google-objc-avoid-throwing-exception) - [binaryMessengerMock sendOnChannel:@"flutter/keyevent" - message:encodedKeyUpEvent - binaryReply:[OCMArg any]]); - } @catch (...) { - return false; - } - return true; -} - - (bool)testKeyboardIsRestartedOnEngineRestart { id engineMock = OCMClassMock([FlutterEngine class]); id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h b/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h index 1da1ab3ecba68..ebce0072c535f 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h @@ -4,6 +4,7 @@ #import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterKeyboardViewDelegate.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h" @@ -18,6 +19,13 @@ */ @property(nonatomic, readonly, nonnull) FlutterTextInputPlugin* textInputPlugin; +/** + * Pointer to a keyboard manager, a hub that manages how key events are + * dispatched to various Flutter key responders, and whether the event is + * propagated to the next NSResponder. + */ +@property(nonatomic, readonly, nonnull) FlutterKeyboardManager* keyboardManager; + /** * Initializes this FlutterViewController with the specified `FlutterEngine`. * From 8d734ccfa95418bb9d0bd1272e93f29b6193fa80 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Sun, 5 Jun 2022 13:26:54 +0200 Subject: [PATCH 02/14] Add IsAddedAndRemovedFromViewHierarchy test --- .../Source/FlutterTextInputPluginTest.mm | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm index bd5b2089c1745..eebc76d00b157 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm @@ -1075,4 +1075,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 From b1eda1c2a01a8fe2ba520d849185257347275527 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Sun, 5 Jun 2022 14:16:55 +0200 Subject: [PATCH 03/14] Add TestPerformKeyEquivalent --- .../Source/FlutterTextInputPluginTest.mm | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm index eebc76d00b157..d3bb8ab3844b1 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm @@ -863,6 +863,61 @@ - (bool)testComposingWithDeltasWhenSelectionIsActive { return true; } +- (bool)testPerformKeyEquivalent { + __block NSEvent* eventBeingDispatchedByKeyboardManager = nil; + id keyboardManagerMock = OCMClassMock([FlutterKeyboardManager class]); + OCMStub([keyboardManagerMock eventBeingDispatched]).andDo(^(NSInvocation* invocation) { + [invocation setReturnValue:&eventBeingDispatchedByKeyboardManager]; + }); + FlutterViewController* viewControllerMock = OCMClassMock([FlutterViewController class]); + OCMStub([viewControllerMock keyboardManager]).andReturn(keyboardManagerMock); + + 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)); @@ -979,6 +1034,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()); From d6a2b0ea68205c3efc564e6ecdaff330851bbe19 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Sun, 5 Jun 2022 14:50:13 +0200 Subject: [PATCH 04/14] Ensure no reentrancy --- .../darwin/macos/framework/Source/FlutterKeyboardManager.mm | 2 ++ 1 file changed, 2 insertions(+) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.mm b/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.mm index 80c3cab7b6286..e4c95fcd6570d 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.mm @@ -230,6 +230,7 @@ - (void)dispatchTextEvent:(NSEvent*)event { if (nextResponder == nil) { return; } + NSAssert(_eventBeingDispatched == nil, @"An event is already being dispached."); _eventBeingDispatched = event; switch (event.type) { case NSEventTypeKeyDown: @@ -250,6 +251,7 @@ - (void)dispatchTextEvent:(NSEvent*)event { default: NSAssert(false, @"Unexpected key event type (got %lu).", event.type); } + NSAssert(_eventBeingDispatched != nil, @"_eventBeingDispatched was cleared unexpectedly."); _eventBeingDispatched = nil; } From 7d9c47f966b83583e56f5b9590db450e9e529cca Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Sun, 5 Jun 2022 21:44:07 +0200 Subject: [PATCH 05/14] Update comment --- .../framework/Source/FlutterTextInputPlugin.mm | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm index b1a45b781b990..78497d669d9a4 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm @@ -497,14 +497,17 @@ - (void)keyUp:(NSEvent*)event { [self.flutterViewController keyUp:event]; } -// Invoked through NSWindow processing of key down event. This can be either -// regular event sent from NSApplication with CMD modifier, in which case the -// event is processed as keyDown, or keyboard manager redispatching the event -// if nextResponder is NSWindow, in which case the event needs to be ignored, -// otherwise it will cause endless loop. - (BOOL)performKeyEquivalent:(NSEvent*)event { if (_flutterViewController.keyboardManager.eventBeingDispatched == event) { - // This happens with cmd+contorl+space (emoji picker) + // 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]; From bc0198604d16578be8a9e44eec5b279570cba782 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 7 Jun 2022 15:35:33 +0200 Subject: [PATCH 06/14] Remove previousResponder field --- .../Source/FlutterTextInputPlugin.mm | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm index 78497d669d9a4..4409e8ee8952b 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm @@ -138,12 +138,6 @@ @interface FlutterTextInputPlugin () */ @property(nonatomic) BOOL enableDeltaModel; -/** - * When plugin becomes first responder it remembers previous responder, - * which will be restored after the plugin is hidden. - */ -@property(nonatomic, weak) NSResponder* previousResponder; - /** * Handles a Flutter system message on the text input channel. */ @@ -268,20 +262,19 @@ - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result { _activeModel = std::make_unique(); } } 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. + // 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]; - if (_previousResponder == nil) { - _previousResponder = self.window.firstResponder; - } - [self.window makeFirstResponder:self]; } + [self.window makeFirstResponder:self]; _shown = TRUE; } else if ([method isEqualToString:kHideMethod]) { - [self.window makeFirstResponder:_previousResponder]; - _previousResponder = 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]; _shown = FALSE; } else if ([method isEqualToString:kClearClientMethod]) { From 99742bb62b30431db337f2adfd0e98d2bf32fcfd Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 7 Jun 2022 15:36:14 +0200 Subject: [PATCH 07/14] Do not use becomeFirstResponder to start editing --- .../Source/AccessibilityBridgeMacDelegate.mm | 4 +- .../FlutterPlatformNodeDelegateMacTest.mm | 5 +- .../Source/FlutterTextInputPlugin.mm | 2 +- .../Source/FlutterTextInputPluginTest.mm | 2 +- .../Source/FlutterTextInputSemanticsObject.h | 6 ++ .../Source/FlutterTextInputSemanticsObject.mm | 62 ++++++++----------- 6 files changed, 39 insertions(+), 42 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.mm b/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.mm index 4e6fe8c672f86..1f70690d9d4a1 100644 --- a/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.mm +++ b/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.window makeFirstResponder:native_text_field]; + [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.window makeFirstResponder:native_text_field]; + [native_text_field startEditing]; } break; } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterPlatformNodeDelegateMacTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterPlatformNodeDelegateMacTest.mm index bb2b5c28c6880..18b2f21386ea4 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterPlatformNodeDelegateMacTest.mm +++ b/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.window makeFirstResponder:native_text_field]; + + [native_text_field startEditing]; EXPECT_EQ([native_text_field.stringValue isEqualToString:@"textfield"], YES); } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm index 4409e8ee8952b..b5a4380dcf3f7 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm @@ -373,7 +373,7 @@ - (void)setEditingState:(NSDictionary*)state { [_textInputContext discardMarkedText]; } if (_client != nil) { - [self.window makeFirstResponder:_client]; + [_client startEditing]; } [self updateTextAndSelection]; } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm index d3bb8ab3844b1..b220a9633fa77 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm @@ -1070,7 +1070,7 @@ - (bool)testLocalTextAndSelectionUpdateAfterDelta { [[FlutterTextFieldMock alloc] initWithPlatformNode:&text_platform_node fieldEditor:viewController.textInputPlugin]; [viewController.view addSubview:mockTextField]; - [mockTextField becomeFirstResponder]; + [mockTextField startEditing]; NSDictionary* arguments = @{ @"inputAction" : @"action", diff --git a/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObject.h b/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObject.h index 759581889d8d4..e3e4ef4901691 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObject.h +++ b/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 diff --git a/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObject.mm b/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObject.mm index 856da8f4d1154..75fecca90163c 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObject.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObject.mm @@ -103,50 +103,42 @@ - (void)setAccessibilityFocused:(BOOL)isFocused { _node->GetDelegate()->AccessibilityPerformAction(data); } -#pragma mark - NSResponder - -- (BOOL)becomeFirstResponder { +- (void)startEditing { if (!_plugin) { - return NO; + return; } - if (_plugin.client == self && [_plugin isFirstResponder]) { - // This text field is already the first responder. - return YES; + if (self.currentEditor == _plugin) { + return; } - BOOL result = [super becomeFirstResponder]; - if (result) { - _plugin.client = self; - // The default implementation of the becomeFirstResponder will change the - // text editing state. Need to manually set it back. - NSString* textValue = @(_node->GetStringAttribute(ax::mojom::StringAttribute::kValue).data()); - int start = _node->GetIntAttribute(ax::mojom::IntAttribute::kTextSelStart); - int end = _node->GetIntAttribute(ax::mojom::IntAttribute::kTextSelEnd); - NSAssert((start >= 0 && end >= 0) || (start == -1 && end == -1), @"selection is invalid"); - NSRange selection; - if (start >= 0 && end >= 0) { - selection = NSMakeRange(MIN(start, end), ABS(end - start)); - } else { - // The native behavior is to place the cursor at the end of the string if - // there is no selection. - selection = NSMakeRange([self stringValue].length, 0); - } - [self updateString:textValue withSelection:selection]; + // Selecting text seems to be the only way to make the field editor + // current editor. + [self selectText:self]; + NSAssert(self.currentEditor == _plugin, @"Failed to set current editor"); + + _plugin.client = self; + + // Restore previous selection. + NSString* textValue = @(_node->GetStringAttribute(ax::mojom::StringAttribute::kValue).data()); + int start = _node->GetIntAttribute(ax::mojom::IntAttribute::kTextSelStart); + int end = _node->GetIntAttribute(ax::mojom::IntAttribute::kTextSelEnd); + NSAssert((start >= 0 && end >= 0) || (start == -1 && end == -1), @"selection is invalid"); + NSRange selection; + if (start >= 0 && end >= 0) { + selection = NSMakeRange(MIN(start, end), ABS(end - start)); + } else { + // The native behavior is to place the cursor at the end of the string if + // there is no selection. + selection = NSMakeRange([self stringValue].length, 0); } - return result; -} - -- (BOOL)resignFirstResponder { - BOOL result = [super resignFirstResponder]; - if (result && _plugin.client == self) { - _plugin.client = nil; - } - return result; + [self updateString:textValue withSelection:selection]; } #pragma mark - NSObject - (void)dealloc { - [self resignFirstResponder]; + if (_plugin.client == self) { + _plugin.client = nil; + } } @end From 512404eb51448b19af084f696ec9b37f83223fd2 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 7 Jun 2022 16:27:34 +0200 Subject: [PATCH 08/14] Reparent FlutterTextPlugin to wrapper view when disabling accessibility --- .../framework/Source/FlutterViewController.mm | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm index 5acf4eb8307d7..1435ba89839be 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm @@ -628,16 +628,11 @@ - (void)dispatchMouseEvent:(NSEvent*)event phase:(FlutterPointerPhase)phase { - (void)onAccessibilityStatusChanged:(BOOL)enabled { if (!enabled && self.viewLoaded && [_textInputPlugin isFirstResponder]) { - // The client (i.e. the FlutterTextField) of the textInputPlugin is a sibling - // of the FlutterView. macOS will pick the ancestor to be the next responder - // when the client is removed from the view hierarchy, which is the result of - // turning off semantics. This will cause the keyboard focus to stick at the - // NSWindow. - // - // Since the view controller creates the illustion that the FlutterTextField is - // below the FlutterView in accessibility (See FlutterViewWrapper), it has to - // manually pick the next responder. - [self.view.window makeFirstResponder:_flutterView]; + // Normally TextInputPlugin, when editing, is child of FlutterViewWrapper. + // When accessiblity is enabled the TextInputPlugin gets added as an indirect + // child to FlutterTextField. When disabling the plugin needs to be reparented + // back. + [self.view addSubview:_textInputPlugin]; } } From aeeb33980fcb807809c50db28bfe521303a714c4 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 7 Jun 2022 16:31:06 +0200 Subject: [PATCH 09/14] typo --- .../darwin/macos/framework/Source/FlutterKeyboardManager.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.h b/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.h index cf9411a4a3fd2..93c6215b58015 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.h @@ -42,7 +42,7 @@ /** * The event currently being redispatched. * - * In some instances (i.e. emoji shortcut) the event may redelivered by cocoa + * 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. */ From 89f1d8dfca64c3535c3b4fa82295c59390297139 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 7 Jun 2022 16:32:47 +0200 Subject: [PATCH 10/14] Remove nil check --- .../darwin/macos/framework/Source/FlutterTextInputPlugin.mm | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm index b5a4380dcf3f7..155965430a97e 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm @@ -372,9 +372,8 @@ - (void)setEditingState:(NSDictionary*)state { if (composing_range.collapsed() && wasComposing) { [_textInputContext discardMarkedText]; } - if (_client != nil) { - [_client startEditing]; - } + [_client startEditing]; + [self updateTextAndSelection]; } From b8afeb5e484eb950a769ffac18effdbf23956880 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 7 Jun 2022 16:44:30 +0200 Subject: [PATCH 11/14] Update tests --- .../framework/Source/FlutterViewControllerTest.mm | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm index 5677bd010f4ee..538c29d8fc3ab 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm @@ -71,7 +71,7 @@ + (void)respondFalseForSendEvent:(const FlutterKeyEvent&)event EXPECT_EQ(accessibilityChildren[0], viewControllerMock.flutterView); } -TEST(FlutterViewController, SetsFlutterViewFirstResponderWhenAccessibilityDisabled) { +TEST(FlutterViewController, ReparentsPluginWhenAccessibilityDisabled) { FlutterEngine* engine = CreateTestEngine(); NSString* fixtures = @(testing::GetFixturesPath()); FlutterDartProject* project = [[FlutterDartProject alloc] @@ -86,14 +86,17 @@ + (void)respondFalseForSendEvent:(const FlutterKeyEvent&)event backing:NSBackingStoreBuffered defer:NO]; window.contentView = viewController.view; + NSView* dummyView = [[NSView alloc] initWithFrame:CGRectZero]; + [viewController.view addSubview:dummyView]; // Attaches FlutterTextInputPlugin to the view; - [viewController.view addSubview:viewController.textInputPlugin]; + [dummyView addSubview:viewController.textInputPlugin]; // Makes sure the textInputPlugin can be the first responder. EXPECT_TRUE([window makeFirstResponder:viewController.textInputPlugin]); EXPECT_EQ([window firstResponder], viewController.textInputPlugin); + EXPECT_FALSE(viewController.textInputPlugin.superview == viewController.view); [viewController onAccessibilityStatusChanged:NO]; - // FlutterView becomes the first responder. - EXPECT_EQ([window firstResponder], viewController.flutterView); + // FlutterView becomes child of view controller + EXPECT_TRUE(viewController.textInputPlugin.superview == viewController.view); } TEST(FlutterViewController, CanSetMouseTrackingModeBeforeViewLoaded) { From 997ae3f669db85bff1f5113cd07fbcd929bcfae9 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Mon, 13 Jun 2022 16:03:29 +0200 Subject: [PATCH 12/14] Do not expose KeyboardManager from controller --- .../macos/framework/Source/FlutterKeyboardManager.h | 4 ++-- .../macos/framework/Source/FlutterKeyboardManager.mm | 6 ++++++ .../macos/framework/Source/FlutterTextInputPlugin.mm | 2 +- .../framework/Source/FlutterTextInputPluginTest.mm | 12 +++++++----- .../macos/framework/Source/FlutterViewController.mm | 11 +++++++++++ .../Source/FlutterViewController_Internal.h | 12 +++++------- 6 files changed, 32 insertions(+), 15 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.h b/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.h index 93c6215b58015..c8ba5058aabfb 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.h @@ -40,12 +40,12 @@ - (void)handleEvent:(nonnull NSEvent*)event; /** - * The event currently being redispatched. + * 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. */ -@property(nonatomic, nullable) NSEvent* eventBeingDispatched; +- (BOOL)isDispatchingKeyEvent:(nonnull NSEvent*)event; @end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.mm b/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.mm index e4c95fcd6570d..67dd257cc9c7e 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.mm @@ -70,6 +70,8 @@ @interface FlutterKeyboardManager () @property(nonatomic) NSMutableDictionary* 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 { diff --git a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm index 155965430a97e..166cc7232be8c 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm @@ -490,7 +490,7 @@ - (void)keyUp:(NSEvent*)event { } - (BOOL)performKeyEquivalent:(NSEvent*)event { - if (_flutterViewController.keyboardManager.eventBeingDispatched == 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) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm index b220a9633fa77..2cfded0631969 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm @@ -865,12 +865,14 @@ - (bool)testComposingWithDeltasWhenSelectionIsActive { - (bool)testPerformKeyEquivalent { __block NSEvent* eventBeingDispatchedByKeyboardManager = nil; - id keyboardManagerMock = OCMClassMock([FlutterKeyboardManager class]); - OCMStub([keyboardManagerMock eventBeingDispatched]).andDo(^(NSInvocation* invocation) { - [invocation setReturnValue:&eventBeingDispatchedByKeyboardManager]; - }); FlutterViewController* viewControllerMock = OCMClassMock([FlutterViewController class]); - OCMStub([viewControllerMock keyboardManager]).andReturn(keyboardManagerMock); + 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 diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm index 1435ba89839be..19c9911e03bcf 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm @@ -176,6 +176,13 @@ @interface FlutterViewController () */ @property(nonatomic) id keyUpMonitor; +/** + * Pointer to a keyboard manager, a hub that manages how key events are + * dispatched to various Flutter key responders, and whether the event is + * propagated to the next NSResponder. + */ +@property(nonatomic, readonly, nonnull) FlutterKeyboardManager* keyboardManager; + @property(nonatomic) KeyboardLayoutNotifier keyboardLayoutNotifier; @property(nonatomic) NSData* keyboardLayoutData; @@ -334,6 +341,10 @@ - (instancetype)initWithEngine:(nonnull FlutterEngine*)engine return self; } +- (BOOL)isDispatchingKeyEvent:(NSEvent*)event { + return [_keyboardManager isDispatchingKeyEvent:event]; +} + - (void)loadView { FlutterView* flutterView; if ([FlutterRenderingBackend renderUsingMetal]) { diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h b/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h index ebce0072c535f..cb5587bcfc41d 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h @@ -19,13 +19,6 @@ */ @property(nonatomic, readonly, nonnull) FlutterTextInputPlugin* textInputPlugin; -/** - * Pointer to a keyboard manager, a hub that manages how key events are - * dispatched to various Flutter key responders, and whether the event is - * propagated to the next NSResponder. - */ -@property(nonatomic, readonly, nonnull) FlutterKeyboardManager* keyboardManager; - /** * Initializes this FlutterViewController with the specified `FlutterEngine`. * @@ -39,6 +32,11 @@ nibName:(nullable NSString*)nibName bundle:(nullable NSBundle*)nibBundle NS_DESIGNATED_INITIALIZER; +/** + * Returns YES if provided event is being currently redispatched by keyboard manager. + */ +- (BOOL)isDispatchingKeyEvent:(nonnull NSEvent*)event; + @end // Private methods made visible for testing From d7f43faafc7abd1a00ca0b7753395fb14eeeb993 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Mon, 13 Jun 2022 17:35:06 +0200 Subject: [PATCH 13/14] Remove unused import --- .../macos/framework/Source/FlutterViewController_Internal.h | 1 - 1 file changed, 1 deletion(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h b/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h index cb5587bcfc41d..04ed474602cf7 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h @@ -4,7 +4,6 @@ #import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h" -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterKeyboardManager.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterKeyboardViewDelegate.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h" From 74b21feabbe927156baf796ea91843a18db7cd0f Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 14 Jun 2022 20:49:32 +0200 Subject: [PATCH 14/14] Remove from superview in TextInput.clearClient --- .../framework/Source/FlutterTextInputPlugin.mm | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm b/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm index 166cc7232be8c..9e9f93dad4252 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm +++ b/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; @@ -271,13 +281,10 @@ - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result { [self.window makeFirstResponder:self]; _shown = TRUE; } else if ([method isEqualToString:kHideMethod]) { - // 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]; + [self resignAndRemoveFromSuperview]; _shown = FALSE; } 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();