Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(27)

Unified Diff: chrome/browser/mac/patch_nsmenu_key_window_bug.mm

Issue 2710763003: Work around an AppKit bug that can leave two windows in a key state.
Patch Set: Use base::mac::ScopedObjCClassSwizzler. Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/BUILD.gn ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/mac/patch_nsmenu_key_window_bug.mm
diff --git a/chrome/browser/mac/patch_nsmenu_key_window_bug.mm b/chrome/browser/mac/patch_nsmenu_key_window_bug.mm
new file mode 100644
index 0000000000000000000000000000000000000000..d6c295411a022a3e68c7657fdeeb26b73cdaf1fb
--- /dev/null
+++ b/chrome/browser/mac/patch_nsmenu_key_window_bug.mm
@@ -0,0 +1,77 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import <AppKit/AppKit.h>
+
+#import "base/logging.h"
tapted 2017/03/02 09:34:59 nit: include
Sidney San Martín 2017/03/02 18:40:08 Done.
+#import "base/mac/scoped_objc_class_swizzler.h"
+
+// https://crbug.com/692408
+// rdar://30572648
+//
+// Back as far as macOS 10.9, and up to at least macOS 10.12.3, an AppKit bug
+// can leave window A in what looks like a key state while window B is actually
+// key. Key presses go to window B even after you click on window A.
+//
+// To trigger the bug, open a menu which has custom views (e.g. the Help menu)
+// and then, while the menu is open, click the titlebar of a different window
+// to bring it to the front. Then, click the first window to bring it *back* to
+// the front and try to type. Input will go to the other window.
+//
+// ## Cause
+// When a menu has any custom views, AppKit uses a different class
+// (NSCarbonMenuWindow) to represent its window than it does otherwise
+// (NSLimitedMenuViewWindow). NSCarbonMenuWindow overrides -[NSWindow
+// makeKeyWindow] to make itself the key window without telling the old key
+// window that it's lost key status (so that its appearance doesn't change). It
+// saves the old key window in an instance variable.
+//
+// When the menu closes, it doesn't just make the saved key window key again.
+// Instead, it picks the frontmost window that can become key. When the
+// frontmost window is different (clicking a window's titlebar brings it to the
+// front) it never sends -[NSWindow resignKeyWindow] to the old key window, so
+// it never loses key appearance and doesn't ask NSApplication to make it key
+// when you click on it (since it thinks it's already key).
+//
+// ## Workaround
+// This file hooks a method on NSCarbonMenuWindow to send -resignKeyWindow to
+// the saved window if a different window has become key.
+//
+// ## If it's an AppKit bug, why work around it?
+// The app/wrench/hotdog menu uses custom views, which gives Chrome more
+// exposure to this bug than many apps, and users have run into it in the wild.
+
+@interface NSCarbonMenuWindowPatcher : NSObject
+- (void)_restorePreviousKeyWindowFromSavedProperties;
+@end
+
+@implementation NSCarbonMenuWindowPatcher
+
+static IMP g_original__restorePreviousKeyWindowFromSavedProperties;
+
++ (void)load {
+ Class NSCarbonMenuWindowClass = NSClassFromString(@"NSCarbonMenuWindow");
tapted 2017/03/02 09:34:59 nit: should start with lowercase (perhaps theNSCar
Sidney San Martín 2017/03/02 18:40:09 Done. I made 'ns' lowercase, which seems to match
+ DCHECK(NSCarbonMenuWindowClass);
+ if (NSCarbonMenuWindowClass == nil)
+ return;
+ CR_DEFINE_STATIC_LOCAL(
+ base::mac::ScopedObjCClassSwizzler, swizzler,
+ (NSCarbonMenuWindowClass, [self class],
+ @selector(_restorePreviousKeyWindowFromSavedProperties)));
+ g_original__restorePreviousKeyWindowFromSavedProperties =
+ swizzler.GetOriginalImplementation();
+}
+
+- (void)_restorePreviousKeyWindowFromSavedProperties {
+ NSWindow* rememberedKeyWindow = nil;
+ Ivar ivar = object_getInstanceVariable(self, "_rememberedKeyWindow",
+ (void**)&rememberedKeyWindow);
tapted 2017/03/02 09:34:59 nit: static_cast<void**> ? We try to avoid c-style
Sidney San Martín 2017/03/02 18:40:08 Done.
+ DCHECK(ivar);
+ g_original__restorePreviousKeyWindowFromSavedProperties(self, _cmd);
+ if ([NSApp keyWindow] != rememberedKeyWindow) {
+ [rememberedKeyWindow resignKeyWindow];
+ }
+}
+
+@end
« no previous file with comments | « chrome/browser/BUILD.gn ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698