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

Unified Diff: chrome/browser/extensions/context_menu_matcher.cc

Issue 359493005: Extend contextMenus API to support browser/page actions (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Revert loop change in render_view_context_menu.cc Created 6 years, 4 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/extensions/context_menu_matcher.cc
diff --git a/chrome/browser/extensions/context_menu_matcher.cc b/chrome/browser/extensions/context_menu_matcher.cc
index 1fd4e635c08bc3d1468959cce5ed6e67c4e799d9..b10b861b6a98534213f4655ffeb6368eb129d06d 100644
--- a/chrome/browser/extensions/context_menu_matcher.cc
+++ b/chrome/browser/extensions/context_menu_matcher.cc
@@ -8,6 +8,7 @@
#include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_util.h"
+#include "chrome/common/extensions/api/context_menus.h"
gpdavis 2014/08/05 01:02:13 This appears to be the cause of the failed trybots
Yoyo Zhou 2014/08/05 01:06:35 Yes, sort of. Move this to the bottom of the inclu
gpdavis 2014/08/05 01:52:28 Okay, cool. Don't I also need to wrap the code th
Yoyo Zhou 2014/08/05 02:16:52 Ah, sorry. Yes, you need to put that in an ifdef a
#include "content/public/browser/browser_context.h"
#include "content/public/common/context_menu_params.h"
#include "extensions/browser/extension_system.h"
@@ -53,7 +54,8 @@ ContextMenuMatcher::ContextMenuMatcher(
void ContextMenuMatcher::AppendExtensionItems(
const MenuItem::ExtensionKey& extension_key,
const base::string16& selection_text,
- int* index) {
+ int* index,
+ bool is_action_menu) {
DCHECK_GE(*index, 0);
int max_index =
extensions_context_custom_last - extensions_context_custom_first;
@@ -77,11 +79,18 @@ void ContextMenuMatcher::AppendExtensionItems(
// Extensions (other than platform apps) are only allowed one top-level slot
// (and it can't be a radio or checkbox item because we are going to put the
- // extension icon next to it).
- // If they have more than that, we automatically push them into a submenu.
- if (extension->is_platform_app()) {
- RecursivelyAppendExtensionItems(items, can_cross_incognito, selection_text,
- menu_model_, index);
+ // extension icon next to it), unless the context menu is an an action menu.
+ // Action menus do not include the extension action, and they only include
+ // items from one extension, so they are not placed within a submenu.
+ // Otherwise, we automatically push them into a submenu if there is more than
+ // one top-level item.
+ if (extension->is_platform_app() || is_action_menu) {
+ RecursivelyAppendExtensionItems(items,
+ can_cross_incognito,
+ selection_text,
+ menu_model_,
+ index,
+ is_action_menu);
} else {
int menu_id = ConvertToExtensionsCustomCommandId(*index);
(*index)++;
@@ -107,10 +116,15 @@ void ContextMenuMatcher::AppendExtensionItems(
ui::SimpleMenuModel* submenu = new ui::SimpleMenuModel(delegate_);
extension_menu_models_.push_back(submenu);
menu_model_->AddSubMenu(menu_id, title, submenu);
- RecursivelyAppendExtensionItems(submenu_items, can_cross_incognito,
- selection_text, submenu, index);
+ RecursivelyAppendExtensionItems(submenu_items,
+ can_cross_incognito,
+ selection_text,
+ submenu,
+ index,
+ false); // is_action_menu_top_level
}
- SetExtensionIcon(extension_key.extension_id);
+ if (!is_action_menu)
+ SetExtensionIcon(extension_key.extension_id);
}
}
@@ -215,10 +229,11 @@ void ContextMenuMatcher::RecursivelyAppendExtensionItems(
bool can_cross_incognito,
const base::string16& selection_text,
ui::SimpleMenuModel* menu_model,
- int* index)
-{
+ int* index,
+ bool is_action_menu_top_level) {
MenuItem::Type last_type = MenuItem::NORMAL;
int radio_group_id = 1;
+ int num_items = 0;
for (MenuItem::List::const_iterator i = items.begin();
i != items.end(); ++i) {
@@ -233,9 +248,16 @@ void ContextMenuMatcher::RecursivelyAppendExtensionItems(
}
int menu_id = ConvertToExtensionsCustomCommandId(*index);
- (*index)++;
- if (menu_id >= extensions_context_custom_last)
+ ++(*index);
+ ++num_items;
+ // Action context menus have a limit for top level extension items to
+ // prevent control items from being pushed off the screen, since extension
+ // items will not be placed in a submenu.
+ if (menu_id >= extensions_context_custom_last ||
+ (is_action_menu_top_level &&
+ num_items >= api::context_menus::ACTION_MENU_TOP_LEVEL_LIMIT))
return;
+
extension_item_map_[menu_id] = item->id();
base::string16 title = item->TitleWithReplacement(selection_text,
kMaxExtensionItemTitleLength);
@@ -248,8 +270,12 @@ void ContextMenuMatcher::RecursivelyAppendExtensionItems(
ui::SimpleMenuModel* submenu = new ui::SimpleMenuModel(delegate_);
extension_menu_models_.push_back(submenu);
menu_model->AddSubMenu(menu_id, title, submenu);
- RecursivelyAppendExtensionItems(children, can_cross_incognito,
- selection_text, submenu, index);
+ RecursivelyAppendExtensionItems(children,
+ can_cross_incognito,
+ selection_text,
+ submenu,
+ index,
+ false); // is_action_menu_top_level
}
} else if (item->type() == MenuItem::CHECKBOX) {
menu_model->AddCheckItem(menu_id, title);
« no previous file with comments | « chrome/browser/extensions/context_menu_matcher.h ('k') | chrome/browser/extensions/extension_context_menu_model.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698