From 40ce12e1e1b9fa4dd55a79a073370e4fef9dfa0c Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Mon, 26 Aug 2024 15:04:24 -0700 Subject: [PATCH] macOS: Add @available check at macOS 12 workaround (flutter/engine#54784) Use default mouse event handling behaviour on macOS 13.3.1 onwards. This has two positive effects: * Avoids the workaround on newer macOS versions where it's unnecessary, thereby giving us confidence that the underlying AppKit issue is fixed and the whole method can later be removed. * Will be caught by tooling when we drop support for versions of macOS prior to the fixed version. Issue: https://github.com/flutter/flutter/issues/154063 Issue: https://github.com/flutter/flutter/issues/115015 No tests modified since there is no semantic change, either on versions of macOS where the issue is fixed (and thus the default event handler is correct) or on versions where it is not (and we still use the workaround). Re-tested manually with the reduced transparency setting on macOS 14.6.1. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style --- .../framework/Source/FlutterViewController.mm | 62 ++++++++++--------- .../Source/FlutterViewControllerTest.mm | 9 +++ 2 files changed, 43 insertions(+), 28 deletions(-) diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm index 1af5c9b1a5..aa1d157088 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm @@ -286,38 +286,44 @@ static void OnKeyboardLayoutChanged(CFNotificationCenterRef center, return @[ _flutterView ]; } +// TODO(cbracken): https://github.com/flutter/flutter/issues/154063 +// Remove this whole method override when we drop support for macOS 12 (Monterey). - (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 - // - // TODO(cbracken): https://github.com/flutter/flutter/issues/154063 - // Remove this workaround when we drop support for macOS 12 (Monterey). - [self.nextResponder mouseDown:event]; + if (@available(macOS 13.3.1, *)) { + [super mouseDown:event]; + } else { + // 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]; + } } +// TODO(cbracken): https://github.com/flutter/flutter/issues/154063 +// Remove this workaround when we drop support for macOS 12 (Monterey). - (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 - // - // TODO(cbracken): https://github.com/flutter/flutter/issues/154063 - // Remove this workaround when we drop support for macOS 12 (Monterey). - [self.nextResponder mouseUp:event]; + if (@available(macOS 13.3.1, *)) { + [super mouseUp:event]; + } else { + // 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 diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm index 3421d78019..b63cf439c9 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm @@ -1082,7 +1082,16 @@ static void SwizzledNoop(id self, SEL _cmd) {} // See: https://github.com/flutter/flutter/issues/115015 // See: http://www.openradar.me/FB12050037 // See: https://developer.apple.com/documentation/appkit/nsresponder/1524634-mousedown +// +// TODO(cbracken): https://github.com/flutter/flutter/issues/154063 +// Remove this test when we drop support for macOS 12 (Monterey). - (bool)testMouseDownUpEventsSentToNextResponder:(id)engineMock { + if (@available(macOS 13.3.1, *)) { + // This workaround is disabled for macOS 13.3.1 onwards, since the underlying AppKit bug is + // fixed. + return true; + } + // 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