Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
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,40 @@ - (bool)textInputPlugin {
return true;
}

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

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

NSMutableArray<FlutterAsyncKeyCallback>* channelCallbacks =
[NSMutableArray<FlutterAsyncKeyCallback> array];
NSMutableArray<FlutterAsyncKeyCallback>* embedderCallbacks =
[NSMutableArray<FlutterAsyncKeyCallback> array];
[tester recordEmbedderCallsTo:embedderCallbacks];
[tester recordChannelCallsTo:channelCallbacks];
// 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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// 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.

Or can you create a way for the mocked text input plugin to record the events, just like the embedder and channel ones?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure ideally I want to verify that the text input plugin did get the event.

[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);

// Send another down event with composing == NO.
tester.isComposing = NO;
[tester respondTextInputWith:YES];
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
[tester respondTextInputWith:YES];

[tester.manager handleEvent:keyDownEvent(0x50)];
// Now the responders get the event.
EXPECT_EQ([channelCallbacks count], 1u);
EXPECT_EQ([embedderCallbacks count], 1u);
embedderCallbacks[0](TRUE);
channelCallbacks[0](TRUE);

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