MacViews: Gets wrench menu working
WrenchMenuModel was set up differently between Mac and Views at compile
time. This change unifies the model so that the wrench menu can work with
MacViews.
Use ButtonMenuItemModel as submodels for Cut/Copy/Paste and
Zoom/Fullscreen menu buttons. Update Views menu to work with this model
change.
BUG=425229
Review URL: https://codereview.chromium.org/1003853004
Cr-Commit-Position: refs/heads/master@{#321016}
diff --git a/chrome/app/chrome_command_ids.h b/chrome/app/chrome_command_ids.h
index f7b0604..cd11b91b 100644
--- a/chrome/app/chrome_command_ids.h
+++ b/chrome/app/chrome_command_ids.h
@@ -214,6 +214,7 @@
#define IDC_SHOW_SRT_BUBBLE 40246
#define IDC_ELEVATED_RECOVERY_DIALOG 40247
#define IDC_TAKE_SCREENSHOT 40248
+#define IDC_MORE_TOOLS_MENU 40249
// Spell-check
// Insert any additional suggestions before _LAST; these have to be consecutive.
diff --git a/chrome/browser/ui/toolbar/wrench_menu_model.cc b/chrome/browser/ui/toolbar/wrench_menu_model.cc
index ad3717f..c7a0991 100644
--- a/chrome/browser/ui/toolbar/wrench_menu_model.cc
+++ b/chrome/browser/ui/toolbar/wrench_menu_model.cc
@@ -955,7 +955,7 @@
AddSeparator(ui::NORMAL_SEPARATOR);
AddSubMenuWithStringId(
- IDC_ZOOM_MENU, IDS_MORE_TOOLS_MENU, tools_menu_model_.get());
+ IDC_MORE_TOOLS_MENU, IDS_MORE_TOOLS_MENU, tools_menu_model_.get());
bool show_exit_menu = browser_defaults::kShowExitMenuItem;
#if defined(OS_WIN)
@@ -1027,22 +1027,14 @@
void WrenchMenuModel::CreateCutCopyPasteMenu() {
AddSeparator(ui::LOWER_SEPARATOR);
-#if defined(OS_MACOSX)
// WARNING: Mac does not use the ButtonMenuItemModel, but instead defines the
- // layout for this menu item in Toolbar.xib. It does, however, use the
+ // layout for this menu item in WrenchMenu.xib. It does, however, use the
// command_id value from AddButtonItem() to identify this special item.
edit_menu_item_model_.reset(new ui::ButtonMenuItemModel(IDS_EDIT, this));
edit_menu_item_model_->AddGroupItemWithStringId(IDC_CUT, IDS_CUT);
edit_menu_item_model_->AddGroupItemWithStringId(IDC_COPY, IDS_COPY);
edit_menu_item_model_->AddGroupItemWithStringId(IDC_PASTE, IDS_PASTE);
AddButtonItem(IDC_EDIT_MENU, edit_menu_item_model_.get());
-#else
- // WARNING: views/wrench_menu assumes these items are added in this order. If
- // you change the order you'll need to update wrench_menu as well.
- AddItemWithStringId(IDC_CUT, IDS_CUT);
- AddItemWithStringId(IDC_COPY, IDS_COPY);
- AddItemWithStringId(IDC_PASTE, IDS_PASTE);
-#endif
AddSeparator(ui::UPPER_SEPARATOR);
}
@@ -1051,29 +1043,18 @@
// This menu needs to be enclosed by separators.
AddSeparator(ui::LOWER_SEPARATOR);
-#if defined(OS_MACOSX)
// WARNING: Mac does not use the ButtonMenuItemModel, but instead defines the
- // layout for this menu item in Toolbar.xib. It does, however, use the
+ // layout for this menu item in WrenchMenu.xib. It does, however, use the
// command_id value from AddButtonItem() to identify this special item.
zoom_menu_item_model_.reset(
new ui::ButtonMenuItemModel(IDS_ZOOM_MENU, this));
- zoom_menu_item_model_->AddGroupItemWithStringId(
- IDC_ZOOM_MINUS, IDS_ZOOM_MINUS2);
- zoom_menu_item_model_->AddButtonLabel(IDC_ZOOM_PERCENT_DISPLAY,
- IDS_ZOOM_PLUS2);
- zoom_menu_item_model_->AddGroupItemWithStringId(
- IDC_ZOOM_PLUS, IDS_ZOOM_PLUS2);
- zoom_menu_item_model_->AddSpace();
- zoom_menu_item_model_->AddItemWithImage(
- IDC_FULLSCREEN, IDR_FULLSCREEN_MENU_BUTTON);
+ zoom_menu_item_model_->AddGroupItemWithStringId(IDC_ZOOM_MINUS,
+ IDS_ZOOM_MINUS2);
+ zoom_menu_item_model_->AddGroupItemWithStringId(IDC_ZOOM_PLUS,
+ IDS_ZOOM_PLUS2);
+ zoom_menu_item_model_->AddItemWithImage(IDC_FULLSCREEN,
+ IDR_FULLSCREEN_MENU_BUTTON);
AddButtonItem(IDC_ZOOM_MENU, zoom_menu_item_model_.get());
-#else
- // WARNING: views/wrench_menu assumes these items are added in this order. If
- // you change the order you'll need to update wrench_menu as well.
- AddItemWithStringId(IDC_ZOOM_MINUS, IDS_ZOOM_MINUS);
- AddItemWithStringId(IDC_ZOOM_PLUS, IDS_ZOOM_PLUS);
- AddItemWithStringId(IDC_FULLSCREEN, IDS_FULLSCREEN);
-#endif
AddSeparator(ui::UPPER_SEPARATOR);
}
diff --git a/chrome/browser/ui/views/toolbar/wrench_menu.cc b/chrome/browser/ui/views/toolbar/wrench_menu.cc
index 8b96fd82c..b206686 100644
--- a/chrome/browser/ui/views/toolbar/wrench_menu.cc
+++ b/chrome/browser/ui/views/toolbar/wrench_menu.cc
@@ -64,6 +64,7 @@
using base::UserMetricsAction;
using bookmarks::BookmarkModel;
using content::WebContents;
+using ui::ButtonMenuItemModel;
using ui::MenuModel;
using views::CustomButton;
using views::ImageButton;
@@ -238,7 +239,7 @@
};
base::string16 GetAccessibleNameForWrenchMenuItem(
- MenuModel* model, int item_index, int accessible_string_id) {
+ ButtonMenuItemModel* model, int item_index, int accessible_string_id) {
base::string16 accessible_name =
l10n_util::GetStringUTF16(accessible_string_id);
base::string16 accelerator_text;
@@ -314,7 +315,7 @@
public views::ButtonListener,
public WrenchMenuObserver {
public:
- WrenchMenuView(WrenchMenu* menu, MenuModel* menu_model)
+ WrenchMenuView(WrenchMenu* menu, ButtonMenuItemModel* menu_model)
: menu_(menu),
menu_model_(menu_model) {
menu_->AddObserver(this);
@@ -373,7 +374,7 @@
protected:
WrenchMenu* menu() { return menu_; }
- MenuModel* menu_model() { return menu_model_; }
+ ButtonMenuItemModel* menu_model() { return menu_model_; }
private:
// Hosting WrenchMenu.
@@ -382,7 +383,7 @@
// The menu model containing the increment/decrement/reset items.
// WARNING: this may be NULL during shutdown.
- MenuModel* menu_model_;
+ ButtonMenuItemModel* menu_model_;
DISALLOW_COPY_AND_ASSIGN(WrenchMenuView);
};
@@ -430,7 +431,7 @@
class WrenchMenu::CutCopyPasteView : public WrenchMenuView {
public:
CutCopyPasteView(WrenchMenu* menu,
- MenuModel* menu_model,
+ ButtonMenuItemModel* menu_model,
int cut_index,
int copy_index,
int paste_index)
@@ -484,7 +485,7 @@
class WrenchMenu::ZoomView : public WrenchMenuView {
public:
ZoomView(WrenchMenu* menu,
- MenuModel* menu_model,
+ ButtonMenuItemModel* menu_model,
int decrement_index,
int increment_index,
int fullscreen_index)
@@ -851,8 +852,9 @@
if (model)
model->RemoveObserver(this);
}
- if (selected_menu_model_)
+ if (selected_menu_model_) {
selected_menu_model_->ActivatedAt(selected_index_);
+ }
}
void WrenchMenu::CloseMenu() {
@@ -995,7 +997,8 @@
// The items representing the cut menu (cut/copy/paste), zoom menu
// (increment/decrement/reset) and extension toolbar view are always enabled.
// The child views of these items enabled state updates appropriately.
- if (command_id == IDC_CUT || command_id == IDC_ZOOM_MINUS ||
+ if (command_id == IDC_EDIT_MENU || command_id == IDC_ZOOM_MENU ||
+ command_id == IDC_MORE_TOOLS_MENU ||
command_id == IDC_EXTENSIONS_OVERFLOW_MENU)
return true;
@@ -1013,7 +1016,8 @@
return;
}
- if (command_id == IDC_CUT || command_id == IDC_ZOOM_MINUS ||
+ if (command_id == IDC_EDIT_MENU || command_id == IDC_ZOOM_MENU ||
+ command_id == IDC_MORE_TOOLS_MENU ||
command_id == IDC_EXTENSIONS_OVERFLOW_MENU) {
// These items are represented by child views. If ExecuteCommand is invoked
// it means the user clicked on the area around the buttons and we should
@@ -1030,7 +1034,8 @@
if (IsBookmarkCommand(command_id))
return false;
- if (command_id == IDC_CUT || command_id == IDC_ZOOM_MINUS ||
+ if (command_id == IDC_EDIT_MENU || command_id == IDC_ZOOM_MENU ||
+ command_id == IDC_MORE_TOOLS_MENU ||
command_id == IDC_EXTENSIONS_OVERFLOW_MENU) {
// These have special child views; don't show the accelerator for them.
return false;
@@ -1101,8 +1106,9 @@
MenuItemView* item =
AddMenuItem(parent, menu_index, model, i, model->GetTypeAt(i));
- if (model->GetCommandIdAt(i) == IDC_CUT ||
- model->GetCommandIdAt(i) == IDC_ZOOM_MINUS) {
+ if (model->GetCommandIdAt(i) == IDC_EDIT_MENU ||
+ model->GetCommandIdAt(i) == IDC_ZOOM_MENU ||
+ model->GetCommandIdAt(i) == IDC_MORE_TOOLS_MENU) {
const MenuConfig& config = item->GetMenuConfig();
int top_margin = config.item_top_margin + config.separator_height / 2;
int bottom_margin =
@@ -1131,25 +1137,25 @@
break;
}
- case IDC_CUT:
- DCHECK_EQ(MenuModel::TYPE_COMMAND, model->GetTypeAt(i));
- DCHECK_LT(i + 2, max);
- DCHECK_EQ(IDC_COPY, model->GetCommandIdAt(i + 1));
- DCHECK_EQ(IDC_PASTE, model->GetCommandIdAt(i + 2));
+ case IDC_EDIT_MENU: {
+ ui::ButtonMenuItemModel* submodel = model->GetButtonMenuItemAt(i);
+ DCHECK_EQ(IDC_CUT, submodel->GetCommandIdAt(0));
+ DCHECK_EQ(IDC_COPY, submodel->GetCommandIdAt(1));
+ DCHECK_EQ(IDC_PASTE, submodel->GetCommandIdAt(2));
item->SetTitle(l10n_util::GetStringUTF16(IDS_EDIT2));
- item->AddChildView(new CutCopyPasteView(this, model,
- i, i + 1, i + 2));
- i += 2;
+ item->AddChildView(new CutCopyPasteView(this, submodel, 0, 1, 2));
break;
+ }
- case IDC_ZOOM_MINUS:
- DCHECK_EQ(MenuModel::TYPE_COMMAND, model->GetTypeAt(i));
- DCHECK_EQ(IDC_ZOOM_PLUS, model->GetCommandIdAt(i + 1));
- DCHECK_EQ(IDC_FULLSCREEN, model->GetCommandIdAt(i + 2));
+ case IDC_ZOOM_MENU: {
+ ui::ButtonMenuItemModel* submodel = model->GetButtonMenuItemAt(i);
+ DCHECK_EQ(IDC_ZOOM_MINUS, submodel->GetCommandIdAt(0));
+ DCHECK_EQ(IDC_ZOOM_PLUS, submodel->GetCommandIdAt(1));
+ DCHECK_EQ(IDC_FULLSCREEN, submodel->GetCommandIdAt(2));
item->SetTitle(l10n_util::GetStringUTF16(IDS_ZOOM_MENU2));
- item->AddChildView(new ZoomView(this, model, i, i + 1, i + 2));
- i += 2;
+ item->AddChildView(new ZoomView(this, submodel, 0, 1, 2));
break;
+ }
case IDC_BOOKMARKS_MENU:
DCHECK(!bookmark_menu_);
@@ -1223,7 +1229,7 @@
return menu_item;
}
-void WrenchMenu::CancelAndEvaluate(MenuModel* model, int index) {
+void WrenchMenu::CancelAndEvaluate(ButtonMenuItemModel* model, int index) {
selected_menu_model_ = model;
selected_index_ = index;
root_->Cancel();
diff --git a/chrome/browser/ui/views/toolbar/wrench_menu.h b/chrome/browser/ui/views/toolbar/wrench_menu.h
index 22fb8973..d9f9fc69 100644
--- a/chrome/browser/ui/views/toolbar/wrench_menu.h
+++ b/chrome/browser/ui/views/toolbar/wrench_menu.h
@@ -138,7 +138,7 @@
// Invoked from the cut/copy/paste menus. Cancels the current active menu and
// activates the menu item in |model| at |index|.
- void CancelAndEvaluate(ui::MenuModel* model, int index);
+ void CancelAndEvaluate(ui::ButtonMenuItemModel* model, int index);
// Creates the bookmark menu if necessary. Does nothing if already created or
// the bookmark model isn't loaded.
@@ -164,7 +164,7 @@
// If |selected_menu_model_| is non-null after the menu completes
// ActivatedAt is invoked. This is done so that ActivatedAt isn't invoked
// while the message loop is nested.
- ui::MenuModel* selected_menu_model_;
+ ui::ButtonMenuItemModel* selected_menu_model_;
int selected_index_;
// Used for managing the bookmark menu items.
diff --git a/ui/base/models/button_menu_item_model.cc b/ui/base/models/button_menu_item_model.cc
index 83bd9737..dd05fe2 100644
--- a/ui/base/models/button_menu_item_model.cc
+++ b/ui/base/models/button_menu_item_model.cc
@@ -88,6 +88,15 @@
return false;
}
+bool ButtonMenuItemModel::GetAcceleratorAt(int index,
+ ui::Accelerator* accelerator) const {
+ if (delegate_) {
+ return delegate_->GetAcceleratorForCommandId(GetCommandIdAt(index),
+ accelerator);
+ }
+ return false;
+}
+
base::string16 ButtonMenuItemModel::GetLabelAt(int index) const {
if (IsItemDynamicAt(index))
return delegate_->GetLabelForCommandId(GetCommandIdAt(index));
@@ -106,9 +115,9 @@
return items_[index].part_of_group;
}
-void ButtonMenuItemModel::ActivatedCommand(int command_id) {
+void ButtonMenuItemModel::ActivatedAt(int index) {
if (delegate_)
- delegate_->ExecuteCommand(command_id, 0);
+ delegate_->ExecuteCommand(GetCommandIdAt(index), 0);
}
bool ButtonMenuItemModel::IsEnabledAt(int index) const {
diff --git a/ui/base/models/button_menu_item_model.h b/ui/base/models/button_menu_item_model.h
index b0bc95bd..01666fe 100644
--- a/ui/base/models/button_menu_item_model.h
+++ b/ui/base/models/button_menu_item_model.h
@@ -12,6 +12,8 @@
namespace ui {
+class Accelerator;
+
// A model representing the rows of buttons that should be inserted in a button
// containing menu item.
class UI_BASE_EXPORT ButtonMenuItemModel {
@@ -34,6 +36,11 @@
virtual bool IsCommandIdEnabled(int command_id) const;
virtual bool DoesCommandIdDismissMenu(int command_id) const;
+ // Gets the accelerator for the specified command id. Returns true if the
+ // command id has a valid accelerator, false otherwise.
+ virtual bool GetAcceleratorForCommandId(int command_id,
+ ui::Accelerator* accelerator) = 0;
+
protected:
virtual ~Delegate() {}
};
@@ -68,6 +75,10 @@
// Whether the label for item |index| changes.
bool IsItemDynamicAt(int index) const;
+ // Gets the accelerator information for the specified index, returning true if
+ // there is a shortcut accelerator for the item, false otherwise.
+ bool GetAcceleratorAt(int index, ui::Accelerator* accelerator) const;
+
// Returns the current label value for the button at |index|.
base::string16 GetLabelAt(int index) const;
@@ -79,8 +90,8 @@
// other items that have their PartOfGroup bit set.
bool PartOfGroup(int index) const;
- // Called from implementations.
- void ActivatedCommand(int command_id);
+ // Called when the item at the specified index has been activated.
+ void ActivatedAt(int index);
// Returns the enabled state of the button at |index|.
bool IsEnabledAt(int index) const;
diff --git a/ui/views/controls/menu/menu_model_adapter.cc b/ui/views/controls/menu/menu_model_adapter.cc
index 195658f9..b04fd0b 100644
--- a/ui/views/controls/menu/menu_model_adapter.cc
+++ b/ui/views/controls/menu/menu_model_adapter.cc
@@ -66,6 +66,7 @@
switch (menu_type) {
case ui::MenuModel::TYPE_COMMAND:
+ case ui::MenuModel::TYPE_BUTTON_ITEM:
type = MenuItemView::NORMAL;
label = model->GetLabelAt(model_index);
sublabel = model->GetSublabelAt(model_index);
