Skip to content

Commit

Permalink
[macOS] TextInputPlugin should mark navigation events in IME popover …
Browse files Browse the repository at this point in the history
…as handled (flutter#46141)

Fixes flutter/flutter#134699

Because of NSTextInputContext API limitations it is not straightforward to determine whether `TextInputPlugin` has handled a text equivalent event or whether it should pass it on. Previously we marked all event  that didn't result in a TextInputClient action as unhandled, but that's does not work for arrow key events while the IME popover is active.

This PR will mark arrow keys event as handled if there is active composition.

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
  • Loading branch information
knopp authored Sep 28, 2023
1 parent 44e23eb commit 20d1be0
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -69,5 +69,6 @@
- (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result;
- (NSRect)firstRectForCharacterRange:(NSRange)range actualRange:(NSRangePointer)actualRange;
- (NSDictionary*)editingState;
@property(nonatomic) NSTextInputContext* textInputContext;
@property(readwrite, nonatomic) NSString* customRunLoopMode;
@end
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,15 @@ - (BOOL)handleKeyEvent:(NSEvent*)event {
// text command (indicated by calling doCommandBySelector) or might not (for example, Cmd+Q). In
// the latter case, this command somehow has not been executed yet and Flutter must dispatch it to
// the next responder. See https://github.com/flutter/flutter/issues/106354 .
if (event.isKeyEquivalent && !_eventProducedOutput) {
// The event is also not redispatched if there is IME composition active, because it might be
// handled by the IME. See https://github.com/flutter/flutter/issues/134699

// both NSEventModifierFlagNumericPad and NSEventModifierFlagFunction are set for arrow keys.
bool is_navigation = event.modifierFlags & NSEventModifierFlagFunction &&
event.modifierFlags & NSEventModifierFlagNumericPad;
bool is_navigation_in_ime = is_navigation && self.hasMarkedText;

if (event.isKeyEquivalent && !is_navigation_in_ime && !_eventProducedOutput) {
return NO;
}
return res;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1361,6 +1361,89 @@ - (bool)testPerformKeyEquivalent {
return true;
}

- (bool)handleArrowKeyWhenImePopoverIsActive {
id engineMock = flutter::testing::CreateMockFlutterEngine(@"");
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]);

NSTextInputContext* textInputContext = OCMClassMock([NSTextInputContext class]);
OCMStub([textInputContext handleEvent:[OCMArg any]]).andReturn(YES);

FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engineMock
nibName:@""
bundle:nil];

FlutterTextInputPlugin* plugin =
[[FlutterTextInputPlugin alloc] initWithViewController:viewController];

plugin.textInputContext = textInputContext;

NSDictionary* setClientConfig = @{
@"inputAction" : @"action",
@"enableDeltaModel" : @"true",
@"inputType" : @{@"name" : @"inputName"},
};
[plugin handleMethodCall:[FlutterMethodCall methodCallWithMethodName:@"TextInput.setClient"
arguments:@[ @(1), setClientConfig ]]
result:^(id){
}];

[plugin handleMethodCall:[FlutterMethodCall methodCallWithMethodName:@"TextInput.show"
arguments:@[]]
result:^(id){
}];

// Set marked text, simulate active IME popover.
[plugin setMarkedText:@"m"
selectedRange:NSMakeRange(0, 1)
replacementRange:NSMakeRange(NSNotFound, 0)];

// Right arrow key. This, unlike the key below should be handled by the plugin.
NSEvent* event = [NSEvent keyEventWithType:NSEventTypeKeyDown
location:NSZeroPoint
modifierFlags:0xa00100
timestamp:0
windowNumber:0
context:nil
characters:@"\uF702"
charactersIgnoringModifiers:@"\uF702"
isARepeat:NO
keyCode:0x4];

// Plugin should mark the event as key equivalent.
[plugin performKeyEquivalent:event];

if ([plugin handleKeyEvent:event] != true) {
return false;
}

// CTRL+H (delete backwards)
event = [NSEvent keyEventWithType:NSEventTypeKeyDown
location:NSZeroPoint
modifierFlags:0x40101
timestamp:0
windowNumber:0
context:nil
characters:@"\uF702"
charactersIgnoringModifiers:@"\uF702"
isARepeat:NO
keyCode:0x4];

// Plugin should mark the event as key equivalent.
[plugin performKeyEquivalent:event];

if ([plugin handleKeyEvent:event] != false) {
return false;
}

return true;
}

- (bool)unhandledKeyEquivalent {
id engineMock = flutter::testing::CreateMockFlutterEngine(@"");
id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger));
Expand Down Expand Up @@ -1814,6 +1897,10 @@ - (bool)testSelectorsAreForwardedToFramework {
ASSERT_TRUE([[FlutterInputPluginTestObjc alloc] testPerformKeyEquivalent]);
}

TEST(FlutterTextInputPluginTest, HandleArrowKeyWhenImePopoverIsActive) {
ASSERT_TRUE([[FlutterInputPluginTestObjc alloc] handleArrowKeyWhenImePopoverIsActive]);
}

TEST(FlutterTextInputPluginTest, UnhandledKeyEquivalent) {
ASSERT_TRUE([[FlutterInputPluginTestObjc alloc] unhandledKeyEquivalent]);
}
Expand Down

0 comments on commit 20d1be0

Please sign in to comment.