Skip to content

Commit 8b4e633

Browse files
knoppknopp
authored
[macOS] Ensure first responder is consistent during and after text input (flutter#46032)
Fixes flutter#134906 Fixes flutter#133832 This ensures that there are only two first responder widgets - `FlutterView` when text input is not active and `TextInputPlugin` when text input is active. The PR also prevents `FlutterView` stealing first responder status on mouse click events during text input. Previously when `TextInputClient` resigned it made `nextResponder` the first responder, but that was incorrect - `nextResponder` being the superview (`FlutterViewWrapper`). ## Pre-launch Checklist - [X] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [X] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [X] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [X] I listed at least one issue that this PR fixes in the description above. - [X] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [X] I updated/added relevant documentation (doc comments with `///`). - [X] I signed the [CLA]. - [X] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat
1 parent 09130bf commit 8b4e633

File tree

6 files changed

+73
-20
lines changed

6 files changed

+73
-20
lines changed

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -384,10 +384,7 @@ - (void)dealloc {
384384

385385
- (void)resignAndRemoveFromSuperview {
386386
if (self.superview != nil) {
387-
// With accessiblity enabled TextInputPlugin is inside _client, so take the
388-
// nextResponder from the _client.
389-
NSResponder* nextResponder = _client != nil ? _client.nextResponder : self.nextResponder;
390-
[self.window makeFirstResponder:nextResponder];
387+
[self.window makeFirstResponder:_flutterViewController.flutterView];
391388
[self removeFromSuperview];
392389
}
393390
}

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2049,6 +2049,41 @@ - (bool)testSelectorsAreForwardedToFramework {
20492049
ASSERT_FALSE(window.firstResponder == viewController.textInputPlugin);
20502050
}
20512051

2052+
TEST(FlutterTextInputPluginTest, FirstResponderIsCorrect) {
2053+
FlutterEngine* engine = CreateTestEngine();
2054+
FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine
2055+
nibName:nil
2056+
bundle:nil];
2057+
[viewController loadView];
2058+
2059+
NSWindow* window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 800, 600)
2060+
styleMask:NSBorderlessWindowMask
2061+
backing:NSBackingStoreBuffered
2062+
defer:NO];
2063+
window.contentView = viewController.view;
2064+
2065+
ASSERT_TRUE(viewController.flutterView.acceptsFirstResponder);
2066+
2067+
[window makeFirstResponder:viewController.flutterView];
2068+
2069+
[viewController.textInputPlugin
2070+
handleMethodCall:[FlutterMethodCall methodCallWithMethodName:@"TextInput.show" arguments:@[]]
2071+
result:^(id){
2072+
}];
2073+
2074+
ASSERT_TRUE(window.firstResponder == viewController.textInputPlugin);
2075+
2076+
ASSERT_FALSE(viewController.flutterView.acceptsFirstResponder);
2077+
2078+
[viewController.textInputPlugin
2079+
handleMethodCall:[FlutterMethodCall methodCallWithMethodName:@"TextInput.hide" arguments:@[]]
2080+
result:^(id){
2081+
}];
2082+
2083+
ASSERT_TRUE(viewController.flutterView.acceptsFirstResponder);
2084+
ASSERT_TRUE(window.firstResponder == viewController.flutterView);
2085+
}
2086+
20522087
TEST(FlutterTextInputPluginTest, HasZeroSizeAndClipsToBounds) {
20532088
id engineMock = flutter::testing::CreateMockFlutterEngine(@"");
20542089
id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger));

‎shell/platform/darwin/macos/framework/Source/FlutterView.h‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,19 @@ typedef int64_t FlutterViewId;
2323
constexpr FlutterViewId kFlutterImplicitViewId = 0ll;
2424

2525
/**
26-
* Listener for view resizing.
26+
* Delegate for FlutterView.
2727
*/
28-
@protocol FlutterViewReshapeListener <NSObject>
28+
@protocol FlutterViewDelegate <NSObject>
2929
/**
3030
* Called when the view's backing store changes size.
3131
*/
3232
- (void)viewDidReshape:(nonnull NSView*)view;
33+
34+
/**
35+
* Called to determine whether the view should accept first responder status.
36+
*/
37+
- (BOOL)viewShouldAcceptFirstResponder:(nonnull NSView*)view;
38+
3339
@end
3440

3541
/**
@@ -43,7 +49,7 @@ constexpr FlutterViewId kFlutterImplicitViewId = 0ll;
4349
*/
4450
- (nullable instancetype)initWithMTLDevice:(nonnull id<MTLDevice>)device
4551
commandQueue:(nonnull id<MTLCommandQueue>)commandQueue
46-
reshapeListener:(nonnull id<FlutterViewReshapeListener>)reshapeListener
52+
delegate:(nonnull id<FlutterViewDelegate>)delegate
4753
threadSynchronizer:(nonnull FlutterThreadSynchronizer*)threadSynchronizer
4854
viewId:(int64_t)viewId NS_DESIGNATED_INITIALIZER;
4955

‎shell/platform/darwin/macos/framework/Source/FlutterView.mm‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
@interface FlutterView () <FlutterSurfaceManagerDelegate> {
1313
int64_t _viewId;
14-
__weak id<FlutterViewReshapeListener> _reshapeListener;
14+
__weak id<FlutterViewDelegate> _viewDelegate;
1515
FlutterThreadSynchronizer* _threadSynchronizer;
1616
FlutterSurfaceManager* _surfaceManager;
1717
}
@@ -22,7 +22,7 @@ @implementation FlutterView
2222

2323
- (instancetype)initWithMTLDevice:(id<MTLDevice>)device
2424
commandQueue:(id<MTLCommandQueue>)commandQueue
25-
reshapeListener:(id<FlutterViewReshapeListener>)reshapeListener
25+
delegate:(id<FlutterViewDelegate>)delegate
2626
threadSynchronizer:(FlutterThreadSynchronizer*)threadSynchronizer
2727
viewId:(int64_t)viewId {
2828
self = [super initWithFrame:NSZeroRect];
@@ -31,7 +31,7 @@ - (instancetype)initWithMTLDevice:(id<MTLDevice>)device
3131
[self setBackgroundColor:[NSColor blackColor]];
3232
[self setLayerContentsRedrawPolicy:NSViewLayerContentsRedrawDuringViewResize];
3333
_viewId = viewId;
34-
_reshapeListener = reshapeListener;
34+
_viewDelegate = delegate;
3535
_threadSynchronizer = threadSynchronizer;
3636
_surfaceManager = [[FlutterSurfaceManager alloc] initWithDevice:device
3737
commandQueue:commandQueue
@@ -54,7 +54,7 @@ - (void)reshaped {
5454
[_threadSynchronizer beginResizeForView:_viewId
5555
size:scaledSize
5656
notify:^{
57-
[_reshapeListener viewDidReshape:self];
57+
[_viewDelegate viewDidReshape:self];
5858
}];
5959
}
6060

@@ -89,7 +89,9 @@ - (BOOL)acceptsFirstMouse:(NSEvent*)event {
8989
}
9090

9191
- (BOOL)acceptsFirstResponder {
92-
return YES;
92+
// This is to ensure that FlutterView does not take first responder status from TextInputPlugin
93+
// on mouse clicks.
94+
return [_viewDelegate viewShouldAcceptFirstResponder:self];
9395
}
9496

9597
- (void)cursorUpdate:(NSEvent*)event {
@@ -104,7 +106,7 @@ - (void)cursorUpdate:(NSEvent*)event {
104106
- (void)viewDidChangeBackingProperties {
105107
[super viewDidChangeBackingProperties];
106108
// Force redraw
107-
[_reshapeListener viewDidReshape:self];
109+
[_viewDelegate viewDidReshape:self];
108110
}
109111

110112
- (BOOL)layer:(CALayer*)layer

‎shell/platform/darwin/macos/framework/Source/FlutterViewController.mm‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ - (void)setBackgroundColor:(NSColor*)color;
169169
/**
170170
* Private interface declaration for FlutterViewController.
171171
*/
172-
@interface FlutterViewController () <FlutterViewReshapeListener>
172+
@interface FlutterViewController () <FlutterViewDelegate>
173173

174174
/**
175175
* The tracking area used to generate hover events, if enabled.
@@ -831,7 +831,7 @@ - (nonnull FlutterView*)createFlutterViewWithMTLDevice:(id<MTLDevice>)device
831831
commandQueue:(id<MTLCommandQueue>)commandQueue {
832832
return [[FlutterView alloc] initWithMTLDevice:device
833833
commandQueue:commandQueue
834-
reshapeListener:self
834+
delegate:self
835835
threadSynchronizer:_threadSynchronizer
836836
viewId:_viewId];
837837
}
@@ -851,15 +851,24 @@ - (NSString*)lookupKeyForAsset:(NSString*)asset fromPackage:(NSString*)package {
851851
return [FlutterDartProject lookupKeyForAsset:asset fromPackage:package];
852852
}
853853

854-
#pragma mark - FlutterViewReshapeListener
854+
#pragma mark - FlutterViewDelegate
855855

856856
/**
857857
* Responds to view reshape by notifying the engine of the change in dimensions.
858858
*/
859859
- (void)viewDidReshape:(NSView*)view {
860+
FML_DCHECK(view == _flutterView);
860861
[_engine updateWindowMetricsForViewController:self];
861862
}
862863

864+
- (BOOL)viewShouldAcceptFirstResponder:(NSView*)view {
865+
FML_DCHECK(view == _flutterView);
866+
// Only allow FlutterView to become first responder if TextInputPlugin is
867+
// not active. Otherwise a mouse event inside FlutterView would cause the
868+
// TextInputPlugin to lose first responder status.
869+
return !_textInputPlugin.isFirstResponder;
870+
}
871+
863872
#pragma mark - FlutterPluginRegistry
864873

865874
- (id<FlutterPluginRegistrar>)registrarForPlugin:(NSString*)pluginName {

‎shell/platform/darwin/macos/framework/Source/FlutterViewTest.mm‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,25 +10,29 @@
1010

1111
constexpr int64_t kImplicitViewId = 0ll;
1212

13-
@interface TestReshapeListener : NSObject <FlutterViewReshapeListener>
13+
@interface TestFlutterViewDelegate : NSObject <FlutterViewDelegate>
1414

1515
@end
1616

17-
@implementation TestReshapeListener
17+
@implementation TestFlutterViewDelegate
1818

1919
- (void)viewDidReshape:(nonnull NSView*)view {
2020
}
2121

22+
- (BOOL)viewShouldAcceptFirstResponder:(NSView*)view {
23+
return YES;
24+
}
25+
2226
@end
2327

2428
TEST(FlutterView, ShouldInheritContentsScaleReturnsYes) {
2529
id<MTLDevice> device = MTLCreateSystemDefaultDevice();
2630
id<MTLCommandQueue> queue = [device newCommandQueue];
27-
TestReshapeListener* listener = [[TestReshapeListener alloc] init];
31+
TestFlutterViewDelegate* delegate = [[TestFlutterViewDelegate alloc] init];
2832
FlutterThreadSynchronizer* threadSynchronizer = [[FlutterThreadSynchronizer alloc] init];
2933
FlutterView* view = [[FlutterView alloc] initWithMTLDevice:device
3034
commandQueue:queue
31-
reshapeListener:listener
35+
delegate:delegate
3236
threadSynchronizer:threadSynchronizer
3337
viewId:kImplicitViewId];
3438
EXPECT_EQ([view layer:view.layer shouldInheritContentsScale:3.0 fromWindow:view.window], YES);

0 commit comments

Comments
 (0)