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

Unified Diff: chrome/browser/tab_contents/render_view_context_menu.cc

Issue 12299013: Fix top-level context menus sorting by name (Closed) Base URL: https://src.chromium.org/chrome/trunk/src/
Patch Set: Created 7 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/extensions/context_menu_matcher.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/tab_contents/render_view_context_menu.cc
===================================================================
--- chrome/browser/tab_contents/render_view_context_menu.cc (revision 182231)
+++ chrome/browser/tab_contents/render_view_context_menu.cc (working copy)
@@ -364,17 +364,29 @@
return; // In unit-tests, we may not have an ExtensionService.
MenuManager* menu_manager = service->menu_manager();
- // Get a list of extension id's that have context menu items, and sort it by
- // the extension's name.
+ // TODO(fbeaufort) Use EscapeAmpersands when
Yoyo Zhou 2013/03/06 20:18:58 nit: TODOs should have a colon: TODO(fbeaufort): D
please use gerrit instead 2013/03/20 16:29:13 https://chromiumcodereview.appspot.com/12294022/ h
+ // https://chromiumcodereview.appspot.com/12294022/ is committed.
+ string16 printable_selection_text = PrintableSelectionText();
+ // Escape "&" as "&&".
+ for (size_t position = printable_selection_text.find('&');
+ position != string16::npos;
+ position = printable_selection_text.find('&', position + 2))
+ printable_selection_text.insert(position, 1, '&');
+
+ // Get a list of extension id's that have context menu items, and sort by the
+ // top level context menu title of the extension.
std::set<std::string> ids = menu_manager->ExtensionIds();
std::vector<std::pair<std::string, std::string> > sorted_ids;
for (std::set<std::string>::iterator i = ids.begin(); i != ids.end(); ++i) {
const Extension* extension = service->GetExtensionById(*i, false);
// Platform apps have their context menus created directly in
// AppendPlatformAppItems.
- if (extension && !extension->is_platform_app())
+ if (extension && !extension->is_platform_app()) {
+ std::string menu_title = extension_items_.GetTopLevelContextMenuTitle(
+ *i, printable_selection_text);
sorted_ids.push_back(
- std::pair<std::string, std::string>(extension->name(), *i));
+ std::pair<std::string, std::string>(menu_title, *i));
+ }
}
// TODO(asargent) - See if this works properly for i18n names (bug 32363).
std::sort(sorted_ids.begin(), sorted_ids.end());
@@ -385,18 +397,9 @@
int index = 0;
base::TimeTicks begin = base::TimeTicks::Now();
std::vector<std::pair<std::string, std::string> >::const_iterator i;
- for (i = sorted_ids.begin();
- i != sorted_ids.end(); ++i) {
- string16 printable_selection_text = PrintableSelectionText();
- // Escape "&" as "&&".
- for (size_t position = printable_selection_text.find('&');
- position != string16::npos;
- position = printable_selection_text.find('&', position + 2))
- printable_selection_text.insert(position, 1, '&');
-
+ for (i = sorted_ids.begin(); i != sorted_ids.end(); ++i)
extension_items_.AppendExtensionItems(i->second, printable_selection_text,
&index);
- }
UMA_HISTOGRAM_TIMES("Extensions.ContextMenus_BuildTime",
base::TimeTicks::Now() - begin);
UMA_HISTOGRAM_COUNTS("Extensions.ContextMenus_ItemCount", index);
« no previous file with comments | « chrome/browser/extensions/context_menu_matcher.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698