Commit af2fb789 authored by andresantoso's avatar andresantoso Committed by Commit bot

Mac: Possible fix for Cmd-W closing window instead of a tab

This logic was simplified and made more robust with
http://crrev.com/848003006, but there is still a report that this is still a
problem.

I am suspicious of the popover detection logic. The popover could belong
to any window, and could be in the middle of fading in or out. Change the
logic to a hopefully more robust on-demand detection using
[NSApplication targetForAction:@selector(performClose:].

BUG=47134

Review URL: https://codereview.chromium.org/1025793002

Cr-Commit-Position: refs/heads/master@{#322075}
parent c20184d4
...@@ -85,9 +85,6 @@ class WorkAreaWatcherObserver; ...@@ -85,9 +85,6 @@ class WorkAreaWatcherObserver;
// to it. // to it.
IBOutlet NSMenu* helpMenu_; IBOutlet NSMenu* helpMenu_;
// Indicates wheter an NSPopover is currently being shown.
BOOL hasPopover_;
// If we are expecting a workspace change in response to a reopen // If we are expecting a workspace change in response to a reopen
// event, the time we got the event. A null time otherwise. // event, the time we got the event. A null time otherwise.
base::TimeTicks reopenTime_; base::TimeTicks reopenTime_;
......
...@@ -111,13 +111,6 @@ using content::DownloadManager; ...@@ -111,13 +111,6 @@ using content::DownloadManager;
namespace { namespace {
// Declare notification names from the 10.7 SDK.
#if !defined(MAC_OS_X_VERSION_10_7) || \
MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
NSString* NSPopoverDidShowNotification = @"NSPopoverDidShowNotification";
NSString* NSPopoverDidCloseNotification = @"NSPopoverDidCloseNotification";
#endif
// How long we allow a workspace change notification to wait to be // How long we allow a workspace change notification to wait to be
// associated with a dock activation. The animation lasts 250ms. See // associated with a dock activation. The animation lasts 250ms. See
// applicationShouldHandleReopen:hasVisibleWindows:. // applicationShouldHandleReopen:hasVisibleWindows:.
...@@ -353,19 +346,6 @@ class AppControllerProfileObserver : public ProfileInfoCacheObserver { ...@@ -353,19 +346,6 @@ class AppControllerProfileObserver : public ProfileInfoCacheObserver {
name:NSWindowDidResignMainNotification name:NSWindowDidResignMainNotification
object:nil]; object:nil];
if (base::mac::IsOSLionOrLater()) {
[notificationCenter
addObserver:self
selector:@selector(popoverDidShow:)
name:NSPopoverDidShowNotification
object:nil];
[notificationCenter
addObserver:self
selector:@selector(popoverDidClose:)
name:NSPopoverDidCloseNotification
object:nil];
}
// Register for space change notifications. // Register for space change notifications.
[[[NSWorkspace sharedWorkspace] notificationCenter] [[[NSWorkspace sharedWorkspace] notificationCenter]
addObserver:self addObserver:self
...@@ -562,21 +542,26 @@ class AppControllerProfileObserver : public ProfileInfoCacheObserver { ...@@ -562,21 +542,26 @@ class AppControllerProfileObserver : public ProfileInfoCacheObserver {
// Close Tab/Close Window accordingly. // Close Tab/Close Window accordingly.
- (void)menuNeedsUpdate:(NSMenu*)menu { - (void)menuNeedsUpdate:(NSMenu*)menu {
DCHECK(menu == [closeTabMenuItem_ menu]); DCHECK(menu == [closeTabMenuItem_ menu]);
NSWindow* window = [NSApp keyWindow];
NSWindow* mainWindow = [NSApp mainWindow]; BOOL enableCloseTabShortcut = NO;
if (!window || ([window parentWindow] == mainWindow)) { id target = [NSApp targetForAction:@selector(performClose:)];
// If the key window is a child of the main window (e.g. a bubble), the main
// window should be the one that handles the close menu item action. // |target| is an instance of NSPopover or NSWindow.
// Also, there might be a small amount of time where there is no key window; // If a popover (likely the dictionary lookup popover), we want Cmd-W to
// in that case as well, just use our main browser window if there is one. // close the popover so map it to "Close Window".
// You might think that we should just always use the main window, but the // Otherwise, map Cmd-W to "Close Tab" if it's a browser window.
// "About Chrome" window serves as a counterexample. if ([target isKindOfClass:[NSWindow class]]) {
window = mainWindow; NSWindow* window = target;
NSWindow* mainWindow = [NSApp mainWindow];
if (!window || ([window parentWindow] == mainWindow)) {
// If the target window is a child of the main window (e.g. a bubble), the
// main window should be the one that handles the close menu item action.
window = mainWindow;
}
enableCloseTabShortcut =
[[window windowController] isKindOfClass:[TabWindowController class]];
} }
BOOL hasTabs =
[[window windowController] isKindOfClass:[TabWindowController class]];
BOOL enableCloseTabShortcut = hasTabs && !hasPopover_;
[self adjustCloseWindowMenuItemKeyEquivalent:enableCloseTabShortcut]; [self adjustCloseWindowMenuItemKeyEquivalent:enableCloseTabShortcut];
[self adjustCloseTabMenuItemKeyEquivalent:enableCloseTabShortcut]; [self adjustCloseTabMenuItemKeyEquivalent:enableCloseTabShortcut];
} }
...@@ -639,16 +624,6 @@ class AppControllerProfileObserver : public ProfileInfoCacheObserver { ...@@ -639,16 +624,6 @@ class AppControllerProfileObserver : public ProfileInfoCacheObserver {
isPoweringOff_ = YES; isPoweringOff_ = YES;
} }
// Called on Lion and later when a popover (e.g. dictionary) is shown.
- (void)popoverDidShow:(NSNotification*)notify {
hasPopover_ = YES;
}
// Called on Lion and later when a popover (e.g. dictionary) is closed.
- (void)popoverDidClose:(NSNotification*)notify {
hasPopover_ = NO;
}
- (void)checkForAnyKeyWindows { - (void)checkForAnyKeyWindows {
if ([NSApp keyWindow]) if ([NSApp keyWindow])
return; return;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment