Allow permission bubbles to participate in key event dispatch as if they were a Browser.
Accomplish this by allowing the `CommandDispatcher` we use for
ChomeEventProcessingWindows to bubble up key handling, and command
validation/dispatch. Dispatch goes up to a parent window's
CommandDispatcher when certain conditions are met.
Currently all ChomeEventProcessingWindows and Views'
NativeWidgetMacNSWindows have a CommandDispatcher, but only browser
windows provide a CommandHandler. By asking the CommandHandler in the
parent window, we can validate commands in the mainMenu, then forward
the -commandDispatch: action from the menu item when NSMenu calls it on
the key window (which might not be a browser).
Adds a rather fun interactive UI test for this that runs with Views and
Cocoa permissions bubbles, and tests commands dispatched via the
mainMenu (Cmd+w, Cmd+Alt+Left) and performKeyEquivalent (Cmd+Shift+'{')
BUG=603881, 679339
Review-Url: https://codereview.chromium.org/2666523002
Cr-Commit-Position: refs/heads/master@{#447864}
diff --git a/chrome/browser/permissions/permission_request_manager.h b/chrome/browser/permissions/permission_request_manager.h
index 2c28ef0..f6c4c4a 100644
--- a/chrome/browser/permissions/permission_request_manager.h
+++ b/chrome/browser/permissions/permission_request_manager.h
@@ -21,6 +21,10 @@
class PermissionReporterBrowserTest;
}
+namespace test {
+class PermissionRequestManagerTestApi;
+}
+
// Provides access to permissions bubbles. Allows clients to add a request
// callback interface to the existing permission bubble configuration.
// Depending on the situation and policy, that may add new UI to an existing
@@ -107,7 +111,10 @@
}
private:
- // TODO(felt): Update testing so that it doesn't involve a lot of friends.
+ friend class test::PermissionRequestManagerTestApi;
+
+ // TODO(felt): Update testing to use the TestApi so that it doesn't involve a
+ // lot of friends.
friend class GeolocationBrowserTest;
friend class GeolocationPermissionContextTests;
friend class MockPermissionPrompt;
diff --git a/chrome/browser/permissions/permission_request_manager_test_api.cc b/chrome/browser/permissions/permission_request_manager_test_api.cc
new file mode 100644
index 0000000..d2e1bd4
--- /dev/null
+++ b/chrome/browser/permissions/permission_request_manager_test_api.cc
@@ -0,0 +1,63 @@
+// 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.
+
+#include "chrome/browser/permissions/permission_request_manager_test_api.h"
+
+#include <memory>
+
+#include "chrome/browser/permissions/permission_request_impl.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+
+namespace test {
+namespace {
+
+// Wraps a PermissionRequestImpl so that it can pass a closure to itself to the
+// PermissionRequestImpl constructor. Without this wrapper, there's no way to
+// handle all destruction paths.
+class TestPermisisonRequestOwner {
+ public:
+ TestPermisisonRequestOwner(Profile* profile, content::PermissionType type) {
+ bool user_gesture = true;
+ auto decided = [](bool, ContentSetting) {};
+ request_ = base::MakeUnique<PermissionRequestImpl>(
+ GURL("https://example.com"), type, profile, user_gesture,
+ base::Bind(decided), base::Bind(&TestPermisisonRequestOwner::DeleteThis,
+ base::Unretained(this)));
+ }
+
+ PermissionRequestImpl* request() { return request_.get(); }
+
+ private:
+ void DeleteThis() { delete this; }
+
+ std::unique_ptr<PermissionRequestImpl> request_;
+
+ DISALLOW_COPY_AND_ASSIGN(TestPermisisonRequestOwner);
+};
+
+} // namespace
+
+PermissionRequestManagerTestApi::PermissionRequestManagerTestApi(
+ PermissionRequestManager* manager)
+ : manager_(manager) {}
+
+PermissionRequestManagerTestApi::PermissionRequestManagerTestApi(
+ Browser* browser)
+ : PermissionRequestManagerTestApi(PermissionRequestManager::FromWebContents(
+ browser->tab_strip_model()->GetActiveWebContents())) {}
+
+void PermissionRequestManagerTestApi::AddSimpleRequest(
+ Profile* profile,
+ content::PermissionType type) {
+ TestPermisisonRequestOwner* request_owner =
+ new TestPermisisonRequestOwner(profile, type);
+ manager_->AddRequest(request_owner->request());
+}
+
+gfx::NativeWindow PermissionRequestManagerTestApi::GetPromptWindow() {
+ return manager_->view_ ? manager_->view_->GetNativeWindow() : nullptr;
+}
+
+} // namespace test
diff --git a/chrome/browser/permissions/permission_request_manager_test_api.h b/chrome/browser/permissions/permission_request_manager_test_api.h
new file mode 100644
index 0000000..ed68c301
--- /dev/null
+++ b/chrome/browser/permissions/permission_request_manager_test_api.h
@@ -0,0 +1,44 @@
+// 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.
+
+#ifndef CHROME_BROWSER_PERMISSIONS_PERMISSION_REQUEST_MANAGER_TEST_API_H_
+#define CHROME_BROWSER_PERMISSIONS_PERMISSION_REQUEST_MANAGER_TEST_API_H_
+
+#include "base/macros.h"
+#include "chrome/browser/permissions/permission_request_manager.h"
+#include "content/public/browser/permission_type.h"
+
+class Browser;
+class Profile;
+
+namespace test {
+
+class PermissionRequestManagerTestApi {
+ public:
+ explicit PermissionRequestManagerTestApi(PermissionRequestManager* manager);
+
+ // Wraps the PermissionRequestManager for the active tab in |browser|.
+ explicit PermissionRequestManagerTestApi(Browser* browser);
+
+ PermissionRequestManager* manager() { return manager_; }
+
+ // Add a "simple" permission request. One that uses PermissionRequestImpl,
+ // such as for content::PermissionType including MIDI_SYSEX, PUSH_MESSAGING,
+ // NOTIFICATIONS, GEOLOCATON, or FLASH. This can be called multiple times
+ // before a call to manager()->DisplayPendingRequests().
+ void AddSimpleRequest(Profile* profile, content::PermissionType type);
+
+ // Return the bubble window for the permission prompt or null if there is no
+ // prompt currently showing.
+ gfx::NativeWindow GetPromptWindow();
+
+ private:
+ PermissionRequestManager* manager_;
+
+ DISALLOW_COPY_AND_ASSIGN(PermissionRequestManagerTestApi);
+};
+
+} // namespace test
+
+#endif // CHROME_BROWSER_PERMISSIONS_PERMISSION_REQUEST_MANAGER_TEST_API_H_
diff --git a/chrome/browser/ui/cocoa/chrome_event_processing_window.mm b/chrome/browser/ui/cocoa/chrome_event_processing_window.mm
index 53ed04c..aff79f2 100644
--- a/chrome/browser/ui/cocoa/chrome_event_processing_window.mm
+++ b/chrome/browser/ui/cocoa/chrome_event_processing_window.mm
@@ -53,12 +53,18 @@
return [super performKeyEquivalent:event];
}
+- (BOOL)defaultValidateUserInterfaceItem:
+ (id<NSValidatedUserInterfaceItem>)item {
+ return [super validateUserInterfaceItem:item];
+}
+
- (void)commandDispatch:(id)sender {
- [commandHandler_ commandDispatch:sender window:self];
+ [commandDispatcher_ dispatch:sender forHandler:commandHandler_];
}
- (void)commandDispatchUsingKeyModifiers:(id)sender {
- [commandHandler_ commandDispatchUsingKeyModifiers:sender window:self];
+ [commandDispatcher_ dispatchUsingKeyModifiers:sender
+ forHandler:commandHandler_];
}
// NSWindow overrides.
@@ -75,20 +81,8 @@
// NSWindow overrides (NSUserInterfaceValidations implementation).
- (BOOL)validateUserInterfaceItem:(id<NSValidatedUserInterfaceItem>)item {
- // Since this class implements these selectors, |super| will always say they
- // are enabled. Only use [super] to validate other selectors. If there is no
- // command handler, defer to AppController.
- if ([item action] == @selector(commandDispatch:) ||
- [item action] == @selector(commandDispatchUsingKeyModifiers:)) {
- if (commandHandler_)
- return [commandHandler_ validateUserInterfaceItem:item window:self];
-
- AppController* appController =
- base::mac::ObjCCastStrict<AppController>([NSApp delegate]);
- return [appController validateUserInterfaceItem:item];
- }
-
- return [super validateUserInterfaceItem:item];
+ return [commandDispatcher_ validateUserInterfaceItem:item
+ forHandler:commandHandler_];
}
@end // ChromeEventProcessingWindow
diff --git a/chrome/browser/ui/cocoa/website_settings/permission_bubble_cocoa_interactive_uitest.mm b/chrome/browser/ui/cocoa/website_settings/permission_bubble_cocoa_interactive_uitest.mm
new file mode 100644
index 0000000..045db28
--- /dev/null
+++ b/chrome/browser/ui/cocoa/website_settings/permission_bubble_cocoa_interactive_uitest.mm
@@ -0,0 +1,179 @@
+// 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 <Cocoa/Cocoa.h>
+
+#include "base/command_line.h"
+#import "base/mac/scoped_nsobject.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/permissions/permission_request_manager_test_api.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_commands.h"
+#include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/interactive_test_utils.h"
+#include "ui/base/test/ui_controls.h"
+#import "ui/base/test/windowed_nsnotification_observer.h"
+#include "ui/base/ui_base_switches.h"
+#import "ui/events/test/cocoa_test_event_utils.h"
+
+namespace {
+
+enum class UiMode {
+ VIEWS,
+ COCOA,
+};
+
+std::string UiModeToString(const ::testing::TestParamInfo<UiMode>& info) {
+ return info.param == UiMode::VIEWS ? "Views" : "Cocoa";
+}
+
+} // namespace
+
+class PermissionBubbleInteractiveUITest
+ : public InProcessBrowserTest,
+ public ::testing::WithParamInterface<UiMode> {
+ public:
+ PermissionBubbleInteractiveUITest() {}
+
+ void EnsureWindowActive(NSWindow* window, const char* message) {
+ SCOPED_TRACE(message);
+ EXPECT_TRUE(window);
+
+ // Activation is asynchronous on Mac. If the window didn't become active,
+ // wait for it.
+ if (![window isKeyWindow]) {
+ base::scoped_nsobject<WindowedNSNotificationObserver> waiter(
+ [[WindowedNSNotificationObserver alloc]
+ initForNotification:NSWindowDidBecomeKeyNotification
+ object:window]);
+ [waiter wait];
+ }
+ EXPECT_TRUE([window isKeyWindow]);
+ }
+
+ // Send Cmd+keycode in the key window to NSApp.
+ void SendAccelerator(ui::KeyboardCode keycode, bool shift, bool alt) {
+ bool control = false;
+ bool command = true;
+ // Note that although this takes an NSWindow, it's just used to create the
+ // NSEvent* which will be dispatched to NSApp (i.e. not NSWindow).
+ ui_controls::SendKeyPress(
+ [NSApp keyWindow], keycode, control, shift, alt, command);
+ }
+
+ // InProcessBrowserTest:
+ void SetUpCommandLine(base::CommandLine* command_line) override {
+ if (GetParam() == UiMode::VIEWS)
+ command_line->AppendSwitch(switches::kExtendMdToSecondaryUi);
+ }
+
+ void SetUpOnMainThread() override {
+ // Make the browser active (ensures the app can receive key events).
+ EXPECT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
+
+ test_api_ =
+ base::MakeUnique<test::PermissionRequestManagerTestApi>(browser());
+ EXPECT_TRUE(test_api_->manager());
+
+ test_api_->AddSimpleRequest(browser()->profile(),
+ content::PermissionType::GEOLOCATION);
+
+ EXPECT_TRUE([browser()->window()->GetNativeWindow() isKeyWindow]);
+ test_api_->manager()->DisplayPendingRequests();
+
+ // The bubble should steal key focus when shown.
+ EnsureWindowActive(test_api_->GetPromptWindow(), "show permission bubble");
+ }
+
+ protected:
+ std::unique_ptr<test::PermissionRequestManagerTestApi> test_api_;
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(PermissionBubbleInteractiveUITest);
+};
+
+// There is only one tab. Cmd+w will close it along with the browser window.
+IN_PROC_BROWSER_TEST_P(PermissionBubbleInteractiveUITest, CmdWClosesWindow) {
+ base::scoped_nsobject<NSWindow> browser_window(
+ browser()->window()->GetNativeWindow(), base::scoped_policy::RETAIN);
+ EXPECT_TRUE([browser_window isVisible]);
+
+ content::WindowedNotificationObserver observer(
+ chrome::NOTIFICATION_BROWSER_CLOSED, content::Source<Browser>(browser()));
+
+ SendAccelerator(ui::VKEY_W, false, false);
+
+ // The actual window close happens via a posted task.
+ EXPECT_TRUE([browser_window isVisible]);
+ observer.Wait();
+ EXPECT_FALSE([browser_window isVisible]);
+}
+
+// Add a tab, ensure we can switch away and back using Cmd+Alt+Left/Right and
+// curly braces.
+IN_PROC_BROWSER_TEST_P(PermissionBubbleInteractiveUITest, SwitchTabs) {
+ NSWindow* browser_window = browser()->window()->GetNativeWindow();
+
+ EXPECT_EQ(0, browser()->tab_strip_model()->active_index());
+ EXPECT_TRUE(test_api_->GetPromptWindow());
+
+ // Add a blank tab in the foreground.
+ AddBlankTabAndShow(browser());
+ EXPECT_EQ(1, browser()->tab_strip_model()->active_index());
+
+ // The bubble should hide and give focus back to the browser. However, the
+ // test environment can't guarantee that macOS decides that the Browser window
+ // is actually the "best" window to activate upon closing the current key
+ // window. So activate it manually.
+ [browser_window makeKeyAndOrderFront:nil];
+ EnsureWindowActive(browser_window, "tab added");
+
+ // Prompt is hidden while its tab is not active.
+ EXPECT_FALSE(test_api_->GetPromptWindow());
+
+ // Now a webcontents is active, it gets a first shot at processing the
+ // accelerator before sending it back unhandled to the browser via IPC. That's
+ // all a bit much to handle in a test, so activate the location bar.
+ chrome::FocusLocationBar(browser());
+ SendAccelerator(ui::VKEY_LEFT, false, true);
+ EXPECT_EQ(0, browser()->tab_strip_model()->active_index());
+
+ // Note we don't need to makeKeyAndOrderFront: the permission window will take
+ // focus when it is shown again.
+ EnsureWindowActive(test_api_->GetPromptWindow(),
+ "switched to permission tab with arrow");
+ EXPECT_TRUE(test_api_->GetPromptWindow());
+
+ // Ensure we can switch away with the bubble active.
+ SendAccelerator(ui::VKEY_RIGHT, false, true);
+ EXPECT_EQ(1, browser()->tab_strip_model()->active_index());
+
+ [browser_window makeKeyAndOrderFront:nil];
+ EnsureWindowActive(browser_window, "switch away with arrow");
+ EXPECT_FALSE(test_api_->GetPromptWindow());
+
+ // Also test switching tabs with curly braces. "VKEY_OEM_4" is
+ // LeftBracket/Brace on a US keyboard, which ui::MacKeyCodeForWindowsKeyCode
+ // will map to '{' when shift is passed. Also note there are only two tabs so
+ // it doesn't matter which direction is taken (it wraps).
+ chrome::FocusLocationBar(browser());
+ SendAccelerator(ui::VKEY_OEM_4, true, false);
+ EXPECT_EQ(0, browser()->tab_strip_model()->active_index());
+ EnsureWindowActive(test_api_->GetPromptWindow(),
+ "switch to permission tab with curly brace");
+ EXPECT_TRUE(test_api_->GetPromptWindow());
+
+ SendAccelerator(ui::VKEY_OEM_4, true, false);
+ EXPECT_EQ(1, browser()->tab_strip_model()->active_index());
+ [browser_window makeKeyAndOrderFront:nil];
+ EnsureWindowActive(browser_window, "switch away with curly brace");
+ EXPECT_FALSE(test_api_->GetPromptWindow());
+}
+
+INSTANTIATE_TEST_CASE_P(,
+ PermissionBubbleInteractiveUITest,
+ ::testing::Values(UiMode::VIEWS, UiMode::COCOA),
+ &UiModeToString);
diff --git a/chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm b/chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm
index da462e6..79974b0 100644
--- a/chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm
+++ b/chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm
@@ -142,37 +142,6 @@
@end
-// The window used for the permission bubble controller.
-// Subclassed to allow browser-handled keyboard events to be passed from the
-// permission bubble to its parent window, which is a browser window.
-@interface PermissionBubbleWindow : InfoBubbleWindow
-@end
-
-@implementation PermissionBubbleWindow
-- (BOOL)performKeyEquivalent:(NSEvent*)event {
- // Before forwarding to parent, handle locally.
- if ([super performKeyEquivalent:event])
- return YES;
-
- // Only handle events if they should be forwarded to the parent window.
- if ([self allowShareParentKeyState]) {
- content::NativeWebKeyboardEvent wrappedEvent(event);
- if ([BrowserWindowUtils shouldHandleKeyboardEvent:wrappedEvent]) {
- // Turn off sharing of key window state while the keyboard event is
- // processed. This avoids recursion - with the key window state shared,
- // the parent window would just forward the event back to this class.
- [self setAllowShareParentKeyState:NO];
- BOOL eventHandled =
- [BrowserWindowUtils handleKeyboardEvent:event
- inWindow:[self parentWindow]];
- [self setAllowShareParentKeyState:YES];
- return eventHandled;
- }
- }
- return NO;
-}
-@end
-
@interface PermissionBubbleController ()
// Determines if the bubble has an anchor in a corner or no anchor at all.
@@ -239,12 +208,12 @@
DCHECK(browser);
DCHECK(bridge);
browser_ = browser;
- base::scoped_nsobject<PermissionBubbleWindow> window(
- [[PermissionBubbleWindow alloc]
- initWithContentRect:ui::kWindowSizeDeterminedLater
- styleMask:NSBorderlessWindowMask
- backing:NSBackingStoreBuffered
- defer:NO]);
+ base::scoped_nsobject<InfoBubbleWindow> window([[InfoBubbleWindow alloc]
+ initWithContentRect:ui::kWindowSizeDeterminedLater
+ styleMask:NSBorderlessWindowMask
+ backing:NSBackingStoreBuffered
+ defer:NO]);
+
[window setAllowedAnimations:info_bubble::kAnimateNone];
[window setReleasedWhenClosed:NO];
if ((self = [super initWithWindow:window
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index 32e9a70..6dec50a 100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -301,6 +301,8 @@
testonly = true
sources = [
+ "../browser/permissions/permission_request_manager_test_api.cc",
+ "../browser/permissions/permission_request_manager_test_api.h",
"base/in_process_browser_test.cc",
"base/in_process_browser_test.h",
"base/in_process_browser_test_mac.cc",
@@ -386,6 +388,7 @@
"../browser/ui/browser_focus_uitest.cc",
"../browser/ui/cocoa/apps/app_shim_menu_controller_mac_interactive_uitest.mm",
"../browser/ui/cocoa/apps/quit_with_apps_controller_mac_interactive_uitest.mm",
+ "../browser/ui/cocoa/website_settings/permission_bubble_cocoa_interactive_uitest.mm",
"../browser/ui/exclusive_access/flash_fullscreen_interactive_browsertest.cc",
"../browser/ui/exclusive_access/fullscreen_controller_interactive_browsertest.cc",
"../browser/ui/exclusive_access/fullscreen_controller_state_interactive_browsertest.cc",
diff --git a/ui/base/cocoa/command_dispatcher.h b/ui/base/cocoa/command_dispatcher.h
index 0212d1e2..efcf8b0 100644
--- a/ui/base/cocoa/command_dispatcher.h
+++ b/ui/base/cocoa/command_dispatcher.h
@@ -32,6 +32,11 @@
// YES if the event is handled.
- (BOOL)performKeyEquivalent:(NSEvent*)event;
+// Validate a user interface item (e.g. an NSMenuItem), consulting |handler| for
+// -commandDispatch: item actions.
+- (BOOL)validateUserInterfaceItem:(id<NSValidatedUserInterfaceItem>)item
+ forHandler:(id<UserInterfaceItemCommandHandler>)handler;
+
// Sends a key event to -[NSApp sendEvent:]. This is used to allow default
// AppKit handling of an event that comes back from CommandDispatcherTarget,
// e.g. key equivalents in the menu, or window manager commands like Cmd+`. Once
@@ -45,6 +50,13 @@
// reposted infinitely. Returns YES if the event is handled.
- (BOOL)preSendEvent:(NSEvent*)event;
+// Dispatch a -commandDispatch: action either to |handler| or a parent window's
+// handler.
+- (void)dispatch:(id)sender
+ forHandler:(id<UserInterfaceItemCommandHandler>)handler;
+- (void)dispatchUsingKeyModifiers:(id)sender
+ forHandler:(id<UserInterfaceItemCommandHandler>)handler;
+
@end
// If the NSWindow's firstResponder implements CommandDispatcherTarget, it is
@@ -95,6 +107,9 @@
// CommandDispatcher calls as part of its -performKeyEquivalent: flow.
- (BOOL)defaultPerformKeyEquivalent:(NSEvent*)event;
+// Short-circuit to the default -validateUserInterfaceItem: implementation.
+- (BOOL)defaultValidateUserInterfaceItem:(id<NSValidatedUserInterfaceItem>)item;
+
// AppKit will call -[NSUserInterfaceValidations validateUserInterfaceItem:] to
// validate UI items. Any item whose target is FirstResponder, or nil, will
// traverse the responder chain looking for a responder that implements the
diff --git a/ui/base/cocoa/command_dispatcher.mm b/ui/base/cocoa/command_dispatcher.mm
index 46a351b..2809dbe 100644
--- a/ui/base/cocoa/command_dispatcher.mm
+++ b/ui/base/cocoa/command_dispatcher.mm
@@ -6,6 +6,19 @@
#include "base/logging.h"
#include "ui/base/cocoa/cocoa_base_utils.h"
+#import "ui/base/cocoa/user_interface_item_command_handler.h"
+
+// Expose -[NSWindow hasKeyAppearance], which determines whether the traffic
+// lights on the window are "lit". CommandDispatcher uses this property on a
+// parent window to decide whether keys and commands should bubble up.
+@interface NSWindow (PrivateAPI)
+- (BOOL)hasKeyAppearance;
+@end
+
+@interface CommandDispatcher ()
+// The parent to bubble events to, or nil.
+- (NSWindow<CommandDispatchingWindow>*)bubbleParent;
+@end
namespace {
@@ -73,10 +86,13 @@
// Give a CommandDispatcherTarget (e.g. a web site) a chance to handle the
// event. If it doesn't want to handle it, it will call us back with
- // -redispatchKeyEvent:.
- NSResponder* r = [owner_ firstResponder];
- if ([r conformsToProtocol:@protocol(CommandDispatcherTarget)])
- return [r performKeyEquivalent:event];
+ // -redispatchKeyEvent:. Only allow this behavior when dispatching key events
+ // on the key window.
+ if ([owner_ isKeyWindow]) {
+ NSResponder* r = [owner_ firstResponder];
+ if ([r conformsToProtocol:@protocol(CommandDispatcherTarget)])
+ return [r performKeyEquivalent:event];
+ }
if ([delegate_ prePerformKeyEquivalent:event window:owner_])
return YES;
@@ -84,7 +100,45 @@
if ([owner_ defaultPerformKeyEquivalent:event])
return YES;
- return [delegate_ postPerformKeyEquivalent:event window:owner_];
+ if ([delegate_ postPerformKeyEquivalent:event window:owner_])
+ return YES;
+
+ // Allow commands to "bubble up" to CommandDispatchers in parent windows, if
+ // they were not handled here.
+ return [[self bubbleParent] performKeyEquivalent:event];
+}
+
+- (BOOL)validateUserInterfaceItem:(id<NSValidatedUserInterfaceItem>)item
+ forHandler:(id<UserInterfaceItemCommandHandler>)handler {
+ // Since this class implements these selectors, |super| will always say they
+ // are enabled. Only use [super] to validate other selectors. If there is no
+ // command handler, defer to AppController.
+ if ([item action] == @selector(commandDispatch:) ||
+ [item action] == @selector(commandDispatchUsingKeyModifiers:)) {
+ if (handler) {
+ // -dispatch:.. can't later decide to bubble events because
+ // -commandDispatch:.. is assumed to always succeed. So, if there is a
+ // |handler|, only validate against that for -commandDispatch:.
+ return [handler validateUserInterfaceItem:item window:owner_];
+ }
+
+ id appController = [NSApp delegate];
+ DCHECK([appController
+ conformsToProtocol:@protocol(NSUserInterfaceValidations)]);
+ if ([appController validateUserInterfaceItem:item])
+ return YES;
+ }
+
+ // Note this may validate an action bubbled up from a child window. However,
+ // if the child window also -respondsToSelector: (but validated it `NO`), the
+ // action will be dispatched to the child only, which may NSBeep().
+ // TODO(tapted): Fix this. E.g. bubble up validation via the bubbleParent's
+ // CommandDispatcher rather than the NSUserInterfaceValidations protocol, so
+ // that this step can be skipped.
+ if ([owner_ defaultValidateUserInterfaceItem:item])
+ return YES;
+
+ return [[self bubbleParent] validateUserInterfaceItem:item];
}
- (BOOL)redispatchKeyEvent:(NSEvent*)event {
@@ -128,4 +182,28 @@
return NO;
}
+- (void)dispatch:(id)sender
+ forHandler:(id<UserInterfaceItemCommandHandler>)handler {
+ if (handler)
+ [handler commandDispatch:sender window:owner_];
+ else
+ [[self bubbleParent] commandDispatch:sender];
+}
+
+- (void)dispatchUsingKeyModifiers:(id)sender
+ forHandler:(id<UserInterfaceItemCommandHandler>)handler {
+ if (handler)
+ [handler commandDispatchUsingKeyModifiers:sender window:owner_];
+ else
+ [[self bubbleParent] commandDispatchUsingKeyModifiers:sender];
+}
+
+- (NSWindow<CommandDispatchingWindow>*)bubbleParent {
+ NSWindow* parent = [owner_ parentWindow];
+ if (parent && [parent hasKeyAppearance] &&
+ [parent conformsToProtocol:@protocol(CommandDispatchingWindow)])
+ return static_cast<NSWindow<CommandDispatchingWindow>*>(parent);
+ return nil;
+}
+
@end
diff --git a/ui/views/cocoa/native_widget_mac_nswindow.mm b/ui/views/cocoa/native_widget_mac_nswindow.mm
index 0f077fe1..134c914 100644
--- a/ui/views/cocoa/native_widget_mac_nswindow.mm
+++ b/ui/views/cocoa/native_widget_mac_nswindow.mm
@@ -198,32 +198,25 @@
return [super performKeyEquivalent:event];
}
+- (BOOL)defaultValidateUserInterfaceItem:
+ (id<NSValidatedUserInterfaceItem>)item {
+ return [super validateUserInterfaceItem:item];
+}
+
- (void)commandDispatch:(id)sender {
- [commandHandler_ commandDispatch:sender window:self];
+ [commandDispatcher_ dispatch:sender forHandler:commandHandler_];
}
- (void)commandDispatchUsingKeyModifiers:(id)sender {
- [commandHandler_ commandDispatchUsingKeyModifiers:sender window:self];
+ [commandDispatcher_ dispatchUsingKeyModifiers:sender
+ forHandler:commandHandler_];
}
// NSWindow overrides (NSUserInterfaceItemValidations implementation)
- (BOOL)validateUserInterfaceItem:(id<NSValidatedUserInterfaceItem>)item {
- // Since this class implements these selectors, |super| will always say they
- // are enabled. Only use [super] to validate other selectors. If there is no
- // command handler, defer to AppController.
- if ([item action] == @selector(commandDispatch:) ||
- [item action] == @selector(commandDispatchUsingKeyModifiers:)) {
- if (commandHandler_)
- return [commandHandler_ validateUserInterfaceItem:item window:self];
-
- id appController = [NSApp delegate];
- DCHECK([appController
- conformsToProtocol:@protocol(NSUserInterfaceValidations)]);
- return [appController validateUserInterfaceItem:item];
- }
-
- return [super validateUserInterfaceItem:item];
+ return [commandDispatcher_ validateUserInterfaceItem:item
+ forHandler:commandHandler_];
}
@end
