From 444708d497626b935da2ac4663f7c168b1669057 Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Thu, 9 Mar 2023 15:40:55 -0800 Subject: [PATCH] [macOS] Forward mouseDown/Up to view controller This works around an AppKit bug in which mouseDown/mouseUp events are not correctly forwarded up the responder chain for views nested inside an NSPopover if (and only if) the macOS "Reduce Transparency" accessibility setting is enabled in the System Settings. When the above conditions are satisfied, the nested NSView receives the mouseDown:/mouseUp: call but if it delegates to the default implementation (implemented in NSResponder) mouseDown/mouseUp calls are triggered on containing views (in our case FlutterViewWrapper) but not triggered on the view controller and other responders in the responder chain until we an _NSPopoverWindow class is hit. A minimal AppKit-only (non-Flutter) repro shows this behaviour repros with even a minimal NSViewController implementation and an unmodified NSView. See: https://github.com/cbracken/PopoverRepro A radar has been filed with Apple and a copy posted to OpenRadar. See: http://www.openradar.me/FB12050037 In order to work around this bug, we override mouseDown/mouseUp in the topmost containing view of FlutterView (in our case, FlutterViewWrapper) to have the behaviour documented as the default behaviour in NSResponder's mouseDown/mouseUp documentation. In otherwords, to simply forward the call to self.nextResponder. See: https://developer.apple.com/documentation/appkit/nsresponder/1524634-mousedown Because replicating the exact configuration of a FlutterView contained in an NSPopover and System Settings that have been modified to enable the "Reduce Transparency" setting is difficult and likely error-prone in infra, we instead simulate the bug by testing that even if NSResponder's mouseDown/mouseUp method are swizzled to no-op, these calls are correctly forwarded to the next responder in the chain. If, in the future Apple does fix this issue, this workaround can be removed once Flutter's minimum supported macOS SDK is at least the version that contains the fix. Issue: https://github.com/flutter/flutter/issues/115015 --- .../framework/Source/FlutterViewController.mm | 28 +++++++ .../Source/FlutterViewControllerTest.mm | 75 +++++++++++++++++++ 2 files changed, 103 insertions(+) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm index ace1e145c1e17..b9999c0cfae01 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm @@ -278,6 +278,34 @@ - (NSArray*)accessibilityChildren { return @[ _flutterView ]; } +- (void)mouseDown:(NSEvent*)event { + // Work around an AppKit bug where mouseDown/mouseUp are not called on the view controller if the + // view is the content view of an NSPopover AND macOS's Reduced Transparency accessibility setting + // is enabled. + // + // This simply calls mouseDown on the next responder in the responder chain as the default + // implementation on NSResponder is documented to do. + // + // See: https://github.com/flutter/flutter/issues/115015 + // See: http://www.openradar.me/FB12050037 + // See: https://developer.apple.com/documentation/appkit/nsresponder/1524634-mousedown + [self.nextResponder mouseDown:event]; +} + +- (void)mouseUp:(NSEvent*)event { + // Work around an AppKit bug where mouseDown/mouseUp are not called on the view controller if the + // view is the content view of an NSPopover AND macOS's Reduced Transparency accessibility setting + // is enabled. + // + // This simply calls mouseUp on the next responder in the responder chain as the default + // implementation on NSResponder is documented to do. + // + // See: https://github.com/flutter/flutter/issues/115015 + // See: http://www.openradar.me/FB12050037 + // See: https://developer.apple.com/documentation/appkit/nsresponder/1535349-mouseup + [self.nextResponder mouseUp:event]; +} + @end #pragma mark - FlutterViewController implementation. diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm index ea96d6914d872..b48953923a38f 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm @@ -18,6 +18,8 @@ #include "flutter/shell/platform/embedder/test_utils/key_codes.g.h" #import "flutter/testing/testing.h" +#pragma mark - Test Helper Classes + // A wrap to convert FlutterKeyEvent to a ObjC class. @interface KeyEventWrapper : NSObject @property(nonatomic) FlutterKeyEvent* data; @@ -36,6 +38,23 @@ - (void)dealloc { } @end +// A FlutterViewController subclass for testing that mouseDown/mouseUp get called when +// mouse events are sent to the associated view. +@interface MouseEventFlutterViewController : FlutterViewController +@property(nonatomic, assign) BOOL mouseDownCalled; +@property(nonatomic, assign) BOOL mouseUpCalled; +@end + +@implementation MouseEventFlutterViewController +- (void)mouseDown:(NSEvent*)event { + self.mouseDownCalled = YES; +} + +- (void)mouseUp:(NSEvent*)event { + self.mouseUpCalled = YES; +} +@end + @interface FlutterViewControllerTestObjC : NSObject - (bool)testKeyEventsAreSentToFramework; - (bool)testKeyEventsArePropagatedIfNotHandled; @@ -43,6 +62,7 @@ - (bool)testKeyEventsAreNotPropagatedIfHandled; - (bool)testFlagsChangedEventsArePropagatedIfNotHandled; - (bool)testKeyboardIsRestartedOnEngineRestart; - (bool)testTrackpadGesturesAreSentToFramework; +- (bool)testMouseDownUpEventsSentToNextResponder; - (bool)testModifierKeysAreSynthesizedOnMouseMove; - (bool)testViewWillAppearCalledMultipleTimes; - (bool)testFlutterViewIsConfigured; @@ -52,6 +72,8 @@ + (void)respondFalseForSendEvent:(const FlutterKeyEvent&)event userData:(nullable void*)userData; @end +#pragma mark - Static helper functions + using namespace ::flutter::testing::keycodes; namespace flutter::testing { @@ -108,6 +130,8 @@ id MockGestureEvent(NSEventType type, NSEventPhase phase, double magnification, } // namespace +#pragma mark - gtest tests + TEST(FlutterViewController, HasViewThatHidesOtherViewsInAccessibility) { FlutterViewController* viewControllerMock = CreateMockViewController(); @@ -196,6 +220,10 @@ id MockGestureEvent(NSEventType type, NSEventPhase phase, double magnification, ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testTrackpadGesturesAreSentToFramework]); } +TEST(FlutterViewControllerTest, TestMouseDownUpEventsSentToNextResponder) { + ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testMouseDownUpEventsSentToNextResponder]); +} + TEST(FlutterViewControllerTest, TestModifierKeysAreSynthesizedOnMouseMove) { ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testModifierKeysAreSynthesizedOnMouseMove]); } @@ -210,6 +238,8 @@ id MockGestureEvent(NSEventType type, NSEventPhase phase, double magnification, } // namespace flutter::testing +#pragma mark - FlutterViewControllerTestObjC + @implementation FlutterViewControllerTestObjC - (bool)testKeyEventsAreSentToFramework { @@ -802,6 +832,51 @@ - (bool)testViewWillAppearCalledMultipleTimes { return true; } +static void SwizzledNoop(id self, SEL _cmd) {} + +// Verify workaround an AppKit bug where mouseDown/mouseUp are not called on the view controller if +// the view is the content view of an NSPopover AND macOS's Reduced Transparency accessibility +// setting is enabled. +// +// See: https://github.com/flutter/flutter/issues/115015 +// See: http://www.openradar.me/FB12050037 +// See: https://developer.apple.com/documentation/appkit/nsresponder/1524634-mousedown +- (bool)testMouseDownUpEventsSentToNextResponder { + // The root cause of the above bug is NSResponder mouseDown/mouseUp methods that don't correctly + // walk the responder chain calling the appropriate method on the next responder under certain + // conditions. Simulate this by swizzling out the default implementations and replacing them with + // no-ops. + Method mouseDown = class_getInstanceMethod([NSResponder class], @selector(mouseDown:)); + Method mouseUp = class_getInstanceMethod([NSResponder class], @selector(mouseUp:)); + IMP noopImp = (IMP)SwizzledNoop; + IMP origMouseDown = method_setImplementation(mouseDown, noopImp); + IMP origMouseUp = method_setImplementation(mouseUp, noopImp); + + // Verify that mouseDown/mouseUp trigger mouseDown/mouseUp calls on FlutterViewController. + id engineMock = flutter::testing::CreateMockFlutterEngine(@""); + MouseEventFlutterViewController* viewController = + [[MouseEventFlutterViewController alloc] initWithEngine:engineMock nibName:@"" bundle:nil]; + FlutterView* view = (FlutterView*)[viewController view]; + + EXPECT_FALSE(viewController.mouseDownCalled); + EXPECT_FALSE(viewController.mouseUpCalled); + + NSEvent* mouseEvent = flutter::testing::CreateMouseEvent(0x00); + [view mouseDown:mouseEvent]; + EXPECT_TRUE(viewController.mouseDownCalled); + EXPECT_FALSE(viewController.mouseUpCalled); + + viewController.mouseDownCalled = NO; + [view mouseUp:mouseEvent]; + EXPECT_FALSE(viewController.mouseDownCalled); + EXPECT_TRUE(viewController.mouseUpCalled); + + // Restore the original NSResponder mouseDown/mouseUp implementations. + method_setImplementation(mouseDown, origMouseDown); + method_setImplementation(mouseUp, origMouseUp); + return true; +} + - (bool)testModifierKeysAreSynthesizedOnMouseMove { id engineMock = flutter::testing::CreateMockFlutterEngine(@""); // Need to return a real renderer to allow view controller to load.