Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[macOS] Forward key events to NSTextInputContext when there's composing text #32051

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@ - (void)handleEvent:(nonnull NSEvent*)event {
event.type != NSEventTypeFlagsChanged) {
return;
}

if (_viewDelegate.isComposing) {
[self dispatchToSecondaryResponders:event];
return;
}

// Having no primary responders require extra logic, but Flutter hard-codes
// all primary responders, so this is a situation that Flutter will never
// encounter.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ - (void)recordChannelCallsTo:(nonnull NSMutableArray<FlutterAsyncKeyCallback>*)s

@property(nonatomic) FlutterKeyboardManager* manager;
@property(nonatomic) NSResponder* nextResponder;
@property(nonatomic, assign) BOOL isComposing;

#pragma mark - Private

Expand Down Expand Up @@ -105,6 +106,7 @@ - (nonnull instancetype)init {
[self respondChannelCallsWith:FALSE];
[self respondEmbedderCallsWith:FALSE];
[self respondTextInputWith:FALSE];
_isComposing = NO;

id messengerMock = OCMStrictProtocolMock(@protocol(FlutterBinaryMessenger));
OCMStub([messengerMock sendOnChannel:@"flutter/keyevent"
Expand All @@ -117,6 +119,7 @@ - (nonnull instancetype)init {
OCMStub([viewDelegateMock onTextInputKeyEvent:[OCMArg any]])
.andCall(self, @selector(handleTextInputKeyEvent:));
OCMStub([viewDelegateMock getBinaryMessenger]).andReturn(messengerMock);
OCMStub([viewDelegateMock isComposing]).andCall(self, @selector(isComposing));
OCMStub([viewDelegateMock sendKeyEvent:FlutterKeyEvent {} callback:nil userData:nil])
.ignoringNonObjectArgs()
.andCall(self, @selector(handleEmbedderEvent:callback:userData:));
Expand Down Expand Up @@ -188,6 +191,7 @@ - (bool)nextResponderShouldThrowOnKeyUp;
- (bool)singlePrimaryResponder;
- (bool)doublePrimaryResponder;
- (bool)textInputPlugin;
- (bool)forwardKeyEventsToSystemWhenComposing;
- (bool)emptyNextResponder;
@end

Expand All @@ -208,6 +212,10 @@ - (bool)emptyNextResponder;
ASSERT_TRUE([[FlutterKeyboardManagerUnittestsObjC alloc] textInputPlugin]);
}

TEST(FlutterKeyboardManagerUnittests, handlingComposingText) {
ASSERT_TRUE([[FlutterKeyboardManagerUnittestsObjC alloc] forwardKeyEventsToSystemWhenComposing]);
}

TEST(FlutterKeyboardManagerUnittests, EmptyNextResponder) {
ASSERT_TRUE([[FlutterKeyboardManagerUnittestsObjC alloc] emptyNextResponder]);
}
Expand Down Expand Up @@ -356,6 +364,31 @@ - (bool)textInputPlugin {
return true;
}

- (bool)forwardKeyEventsToSystemWhenComposing {
KeyboardTester* tester = OCMPartialMock([[KeyboardTester alloc] init]);

NSMutableArray<FlutterAsyncKeyCallback>* channelCallbacks =
[NSMutableArray<FlutterAsyncKeyCallback> array];
NSMutableArray<FlutterAsyncKeyCallback>* embedderCallbacks =
[NSMutableArray<FlutterAsyncKeyCallback> array];
[tester recordEmbedderCallsTo:embedderCallbacks];
[tester recordChannelCallsTo:channelCallbacks];
// The event shouldn't propagate further even if TextInputPlugin does not
// claim the event.
[tester respondTextInputWith:NO];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is used to set the responses from the mocked TextInputPlugin, not to check. (Maybe I should add a method to check its value...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I didn't mean to use this as a "verification" statement. The verification step is at line 385 & 386 where we make sure the event doesn't go to the primary responder(s) even with text input not claiming the key event for itself.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah so this line does little and should be removed. And still we haven't ensured that onTextInputKeyEvent is called.


tester.isComposing = YES;
// Send a down event with composing == YES.
[tester.manager handleEvent:keyUpEvent(0x50)];

// Nobody gets the event except for the text input plugin.
EXPECT_EQ([channelCallbacks count], 0u);
EXPECT_EQ([embedderCallbacks count], 0u);
OCMVerify(times(1), [tester handleTextInputKeyEvent:checkKeyDownEvent(0x50)]);

return true;
}

- (bool)emptyNextResponder {
KeyboardTester* tester = [[KeyboardTester alloc] init];
tester.nextResponder = nil;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,14 @@
*/
- (BOOL)onTextInputKeyEvent:(nonnull NSEvent*)event;

/**
* Whether this FlutterKeyboardViewDelegate is actively taking provisional user text input.
*
* This is typically true when a Flutter text field is focused, and the user is entering composing
* text into the text field.
*/
// TODO (LongCatIsLooong): remove this method and implement a long-term fix for
// https://github.com/flutter/flutter/issues/85328.
- (BOOL)isComposing;

@end
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,15 @@
*/
- (BOOL)isFirstResponder;

/**
* Whether this plugin has composing text.
*
* This is only true when the text input plugin is actively taking user input with composing text.
*/
// TODO (LongCatIsLooong): remove this method and implement a long-term fix for
// https://github.com/flutter/flutter/issues/85328.
- (BOOL)isComposing;

/**
* Handles key down events received from the view controller, responding YES if
* the event was handled.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,10 @@ - (NSString*)textAffinityString {
: kTextAffinityDownstream;
}

- (BOOL)isComposing {
return _activeModel && !_activeModel->composing_range().collapsed();
}

- (BOOL)handleKeyEvent:(NSEvent*)event {
if (event.type == NSEventTypeKeyUp ||
(event.type == NSEventTypeFlagsChanged && event.modifierFlags < _previouslyPressedFlags)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,10 @@ - (void)viewDidReshape:(NSView*)view {

#pragma mark - FlutterKeyboardViewDelegate

- (BOOL)isComposing {
return [_textInputPlugin isComposing];
}

- (void)sendKeyEvent:(const FlutterKeyEvent&)event
callback:(nullable FlutterKeyEventCallback)callback
userData:(nullable void*)userData {
Expand Down