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

Unified Diff: chrome/browser/ui/views/toolbar/browser_action_view.cc

Issue 360423002: Allow WebContents key handling to supplant extension overrides of the bookmark shortcut (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@lkgr
Patch Set: update test Created 6 years, 6 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
Index: chrome/browser/ui/views/toolbar/browser_action_view.cc
diff --git a/chrome/browser/ui/views/toolbar/browser_action_view.cc b/chrome/browser/ui/views/toolbar/browser_action_view.cc
index 02bbbd62458ca7b80c0cc5a9fe1ebdaf06693b9a..97ce75262fd683612225d2d5261a472482372d90 100644
--- a/chrome/browser/ui/views/toolbar/browser_action_view.cc
+++ b/chrome/browser/ui/views/toolbar/browser_action_view.cc
@@ -5,6 +5,7 @@
#include "chrome/browser/ui/views/toolbar/browser_action_view.h"
#include "base/strings/utf_string_conversions.h"
+#include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/extensions/api/commands/command_service.h"
#include "chrome/browser/extensions/extension_action.h"
@@ -14,10 +15,12 @@
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/themes/theme_service.h"
#include "chrome/browser/themes/theme_service_factory.h"
+#include "chrome/browser/ui/accelerator_utils.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/view_ids.h"
#include "chrome/browser/ui/views/toolbar/browser_actions_container.h"
#include "chrome/browser/ui/views/toolbar/toolbar_view.h"
+#include "chrome/common/extensions/manifest_handlers/ui_overrides_handler.h"
#include "extensions/common/extension.h"
#include "extensions/common/manifest_constants.h"
#include "grit/generated_resources.h"
@@ -346,6 +349,17 @@ void BrowserActionButton::OnGestureEvent(ui::GestureEvent* event) {
bool BrowserActionButton::AcceleratorPressed(
const ui::Accelerator& accelerator) {
+ // As a special case, if the extension removes and reassigns the bookmark
+ // shortcut we don't respond here but instead allow it to be handled by the
+ // standard browser command processing. This preserves the same shortcut key
+ // handling of Chrome's built-in bookmarking function, which can be overridden
+ // by key handling on web pages.
+ if (accelerator ==
+ chrome::GetPrimaryChromeAcceleratorForCommandId(IDC_BOOKMARK_PAGE) &&
+ extensions::UIOverrides::RemovesBookmarkShortcut(extension_)) {
+ return false;
+ }
+
delegate_->OnBrowserActionExecuted(this);
return true;
}
@@ -389,10 +403,19 @@ void BrowserActionButton::MaybeRegisterExtensionCommand() {
extensions::CommandService::ACTIVE_ONLY,
&browser_action_command,
NULL)) {
+ ui::AcceleratorManager::HandlerPriority priority =
+ ui::AcceleratorManager::kHighPriority;
+ // Extensions overriding the bookmark shortcut need normal priority to
+ // preserve the built-in processing order of the key and not override
+ // WebContents key handling.
+ if (browser_action_command.accelerator() ==
+ chrome::GetPrimaryChromeAcceleratorForCommandId(IDC_BOOKMARK_PAGE) &&
+ extensions::UIOverrides::RemovesBookmarkShortcut(extension_)) {
+ priority = ui::AcceleratorManager::kNormalPriority;
+ }
keybinding_.reset(new ui::Accelerator(
browser_action_command.accelerator()));
- GetFocusManager()->RegisterAccelerator(
- *keybinding_.get(), ui::AcceleratorManager::kHighPriority, this);
+ GetFocusManager()->RegisterAccelerator(*keybinding_.get(), priority, this);
}
}

Powered by Google App Engine
This is Rietveld 408576698