From 6381f44f26d6ca05c6d58aaec19d40fd58092298 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sat, 2 Jul 2016 11:47:40 +0900 Subject: [PATCH] mac: Pass useDefaultAccelerator to getAcceleratorForCommandId --- atom/browser/api/atom_api_menu.cc | 9 +++-- atom/browser/api/atom_api_menu.h | 8 ++-- atom/browser/api/atom_api_menu_mac.mm | 6 ++- atom/browser/browser.h | 7 +--- atom/browser/browser_mac.mm | 2 +- atom/browser/mac/atom_application_delegate.h | 4 +- atom/browser/mac/atom_application_delegate.mm | 16 ++++---- atom/browser/ui/atom_menu_model.cc | 11 +++++ atom/browser/ui/atom_menu_model.h | 16 ++++++++ atom/browser/ui/cocoa/atom_menu_controller.h | 18 ++++----- atom/browser/ui/cocoa/atom_menu_controller.mm | 40 +++++++++---------- atom/browser/ui/tray_icon.cc | 2 +- atom/browser/ui/tray_icon.h | 6 +-- atom/browser/ui/tray_icon_cocoa.h | 5 +-- atom/browser/ui/tray_icon_cocoa.mm | 14 ++++--- lib/browser/api/menu-item.js | 6 ++- lib/browser/api/menu.js | 8 ++-- 17 files changed, 102 insertions(+), 76 deletions(-) diff --git a/atom/browser/api/atom_api_menu.cc b/atom/browser/api/atom_api_menu.cc index c9cd37522b8..5e66850c754 100644 --- a/atom/browser/api/atom_api_menu.cc +++ b/atom/browser/api/atom_api_menu.cc @@ -53,11 +53,14 @@ bool Menu::IsCommandIdVisible(int command_id) const { return is_visible_.Run(command_id); } -bool Menu::GetAcceleratorForCommandId(int command_id, - ui::Accelerator* accelerator) { +bool Menu::GetAcceleratorForCommandIdWithParams( + int command_id, + bool use_default_accelerator, + ui::Accelerator* accelerator) const { v8::Locker locker(isolate()); v8::HandleScope handle_scope(isolate()); - v8::Local val = get_accelerator_.Run(command_id); + v8::Local val = get_accelerator_.Run( + command_id, use_default_accelerator); return mate::ConvertFromV8(isolate(), val, accelerator); } diff --git a/atom/browser/api/atom_api_menu.h b/atom/browser/api/atom_api_menu.h index 53c6bdaf4ec..9f24f758489 100644 --- a/atom/browser/api/atom_api_menu.h +++ b/atom/browser/api/atom_api_menu.h @@ -46,8 +46,10 @@ class Menu : public mate::TrackableObject, bool IsCommandIdChecked(int command_id) const override; bool IsCommandIdEnabled(int command_id) const override; bool IsCommandIdVisible(int command_id) const override; - bool GetAcceleratorForCommandId(int command_id, - ui::Accelerator* accelerator) override; + bool GetAcceleratorForCommandIdWithParams( + int command_id, + bool use_default_accelerator, + ui::Accelerator* accelerator) const override; void ExecuteCommand(int command_id, int event_flags) override; void MenuWillShow(ui::SimpleMenuModel* source) override; @@ -89,7 +91,7 @@ class Menu : public mate::TrackableObject, base::Callback is_checked_; base::Callback is_enabled_; base::Callback is_visible_; - base::Callback(int)> get_accelerator_; + base::Callback(int, bool)> get_accelerator_; base::Callback, int)> execute_command_; base::Callback menu_will_show_; diff --git a/atom/browser/api/atom_api_menu_mac.mm b/atom/browser/api/atom_api_menu_mac.mm index 3b91dac62a7..22a624988b7 100644 --- a/atom/browser/api/atom_api_menu_mac.mm +++ b/atom/browser/api/atom_api_menu_mac.mm @@ -30,7 +30,8 @@ void MenuMac::PopupAt(Window* window, int x, int y, int positioning_item) { return; base::scoped_nsobject menu_controller( - [[AtomMenuController alloc] initWithModel:model_.get()]); + [[AtomMenuController alloc] initWithModel:model_.get() + useDefaultAccelerator:NO]); NSMenu* menu = [menu_controller menu]; NSView* view = web_contents->GetView()->GetNativeView(); @@ -74,7 +75,8 @@ void MenuMac::PopupAt(Window* window, int x, int y, int positioning_item) { void Menu::SetApplicationMenu(Menu* base_menu) { MenuMac* menu = static_cast(base_menu); base::scoped_nsobject menu_controller( - [[AtomMenuController alloc] initWithModel:menu->model_.get()]); + [[AtomMenuController alloc] initWithModel:menu->model_.get() + useDefaultAccelerator:YES]); [NSApp setMainMenu:[menu_controller menu]]; // Ensure the menu_controller_ is destroyed after main menu is set. diff --git a/atom/browser/browser.h b/atom/browser/browser.h index 37d2e11e679..f91d8b5f5ce 100644 --- a/atom/browser/browser.h +++ b/atom/browser/browser.h @@ -25,16 +25,13 @@ class DictionaryValue; class FilePath; } -namespace ui { -class MenuModel; -} - namespace gfx { class Image; } namespace atom { +class AtomMenuModel; class LoginHandler; // This class is used for control application-wide operations. @@ -130,7 +127,7 @@ class Browser : public WindowListObserver { void DockShow(); // Set docks' menu. - void DockSetMenu(ui::MenuModel* model); + void DockSetMenu(AtomMenuModel* model); // Set docks' icon. void DockSetIcon(const gfx::Image& image); diff --git a/atom/browser/browser_mac.mm b/atom/browser/browser_mac.mm index bb789365ffb..c3cd15a6512 100644 --- a/atom/browser/browser_mac.mm +++ b/atom/browser/browser_mac.mm @@ -216,7 +216,7 @@ void Browser::DockShow() { } } -void Browser::DockSetMenu(ui::MenuModel* model) { +void Browser::DockSetMenu(AtomMenuModel* model) { AtomApplicationDelegate* delegate = (AtomApplicationDelegate*)[NSApp delegate]; [delegate setApplicationDockMenu:model]; } diff --git a/atom/browser/mac/atom_application_delegate.h b/atom/browser/mac/atom_application_delegate.h index 3e5c59c3ff3..777475213ec 100644 --- a/atom/browser/mac/atom_application_delegate.h +++ b/atom/browser/mac/atom_application_delegate.h @@ -11,9 +11,7 @@ base::scoped_nsobject menu_controller_; } -- (id)init; - // Sets the menu that will be returned in "applicationDockMenu:". -- (void)setApplicationDockMenu:(ui::MenuModel*)model; +- (void)setApplicationDockMenu:(atom::AtomMenuModel*)model; @end diff --git a/atom/browser/mac/atom_application_delegate.mm b/atom/browser/mac/atom_application_delegate.mm index 84caae9d5ee..e77bd125201 100644 --- a/atom/browser/mac/atom_application_delegate.mm +++ b/atom/browser/mac/atom_application_delegate.mm @@ -12,14 +12,9 @@ @implementation AtomApplicationDelegate -- (id)init { - self = [super init]; - menu_controller_.reset([[AtomMenuController alloc] init]); - return self; -} - -- (void)setApplicationDockMenu:(ui::MenuModel*)model { - [menu_controller_ populateWithModel:model]; +- (void)setApplicationDockMenu:(atom::AtomMenuModel*)model { + menu_controller_.reset([[AtomMenuController alloc] initWithModel:model + useDefaultAccelerator:NO]); } - (void)applicationWillFinishLaunching:(NSNotification*)notify { @@ -34,7 +29,10 @@ } - (NSMenu*)applicationDockMenu:(NSApplication*)sender { - return [menu_controller_ menu]; + if (menu_controller_) + return [menu_controller_ menu]; + else + return nil; } - (BOOL)application:(NSApplication*)sender diff --git a/atom/browser/ui/atom_menu_model.cc b/atom/browser/ui/atom_menu_model.cc index 369684dd3f2..ad5384f07b7 100644 --- a/atom/browser/ui/atom_menu_model.cc +++ b/atom/browser/ui/atom_menu_model.cc @@ -29,6 +29,17 @@ base::string16 AtomMenuModel::GetRoleAt(int index) { return base::string16(); } +bool AtomMenuModel::GetAcceleratorAtWithParams( + int index, + bool use_default_accelerator, + ui::Accelerator* accelerator) const { + if (delegate_) { + return delegate_->GetAcceleratorForCommandIdWithParams( + GetCommandIdAt(index), use_default_accelerator, accelerator); + } + return false; +} + void AtomMenuModel::MenuClosed() { ui::SimpleMenuModel::MenuClosed(); FOR_EACH_OBSERVER(Observer, observers_, MenuClosed()); diff --git a/atom/browser/ui/atom_menu_model.h b/atom/browser/ui/atom_menu_model.h index 1112949e7ef..99246d74d67 100644 --- a/atom/browser/ui/atom_menu_model.h +++ b/atom/browser/ui/atom_menu_model.h @@ -17,6 +17,19 @@ class AtomMenuModel : public ui::SimpleMenuModel { class Delegate : public ui::SimpleMenuModel::Delegate { public: virtual ~Delegate() {} + + virtual bool GetAcceleratorForCommandIdWithParams( + int command_id, + bool use_default_accelerator, + ui::Accelerator* accelerator) const = 0; + + private: + // ui::SimpleMenuModel::Delegate: + bool GetAcceleratorForCommandId(int command_id, + ui::Accelerator* accelerator) { + return GetAcceleratorForCommandIdWithParams( + command_id, true, accelerator); + } }; class Observer { @@ -35,6 +48,9 @@ class AtomMenuModel : public ui::SimpleMenuModel { void SetRole(int index, const base::string16& role); base::string16 GetRoleAt(int index); + bool GetAcceleratorAtWithParams(int index, + bool use_default_accelerator, + ui::Accelerator* accelerator) const; // ui::SimpleMenuModel: void MenuClosed() override; diff --git a/atom/browser/ui/cocoa/atom_menu_controller.h b/atom/browser/ui/cocoa/atom_menu_controller.h index f8c48aa5dcb..af0b2769618 100644 --- a/atom/browser/ui/cocoa/atom_menu_controller.h +++ b/atom/browser/ui/cocoa/atom_menu_controller.h @@ -11,8 +11,8 @@ #include "base/mac/scoped_nsobject.h" #include "base/strings/string16.h" -namespace ui { -class MenuModel; +namespace atom { +class AtomMenuModel; } // A controller for the cross-platform menu model. The menu that's created @@ -23,24 +23,20 @@ class MenuModel; // as it only maintains weak references. @interface AtomMenuController : NSObject { @protected - ui::MenuModel* model_; // weak + atom::AtomMenuModel* model_; // weak base::scoped_nsobject menu_; BOOL isMenuOpen_; + BOOL useDefaultAccelerator_; } -@property(nonatomic, assign) ui::MenuModel* model; - -// NIB-based initializer. This does not create a menu. Clients can set the -// properties of the object and the menu will be created upon the first call to -// |-menu|. Note that the menu will be immutable after creation. -- (id)init; +@property(nonatomic, assign) atom::AtomMenuModel* model; // Builds a NSMenu from the pre-built model (must not be nil). Changes made // to the contents of the model after calling this will not be noticed. -- (id)initWithModel:(ui::MenuModel*)model; +- (id)initWithModel:(atom::AtomMenuModel*)model useDefaultAccelerator:(BOOL)use; // Populate current NSMenu with |model|. -- (void)populateWithModel:(ui::MenuModel*)model; +- (void)populateWithModel:(atom::AtomMenuModel*)model; // Programmatically close the constructed menu. - (void)cancel; diff --git a/atom/browser/ui/cocoa/atom_menu_controller.mm b/atom/browser/ui/cocoa/atom_menu_controller.mm index 536f4876058..3efd1611516 100644 --- a/atom/browser/ui/cocoa/atom_menu_controller.mm +++ b/atom/browser/ui/cocoa/atom_menu_controller.mm @@ -48,15 +48,11 @@ Role kRolesMap[] = { @synthesize model = model_; -- (id)init { - if ((self = [super init])) - [self menu]; - return self; -} - -- (id)initWithModel:(ui::MenuModel*)model { +- (id)initWithModel:(atom::AtomMenuModel*)model useDefaultAccelerator:(BOOL)use { if ((self = [super init])) { model_ = model; + isMenuOpen_ = NO; + useDefaultAccelerator_ = use; [self menu]; } return self; @@ -73,7 +69,7 @@ Role kRolesMap[] = { [super dealloc]; } -- (void)populateWithModel:(ui::MenuModel*)model { +- (void)populateWithModel:(atom::AtomMenuModel*)model { if (!menu_) return; @@ -82,7 +78,7 @@ Role kRolesMap[] = { const int count = model->GetItemCount(); for (int index = 0; index < count; index++) { - if (model->GetTypeAt(index) == ui::MenuModel::TYPE_SEPARATOR) + if (model->GetTypeAt(index) == atom::AtomMenuModel::TYPE_SEPARATOR) [self addSeparatorToMenu:menu_ atIndex:index]; else [self addItemToMenu:menu_ atIndex:index fromModel:model]; @@ -99,12 +95,12 @@ Role kRolesMap[] = { // Creates a NSMenu from the given model. If the model has submenus, this can // be invoked recursively. -- (NSMenu*)menuFromModel:(ui::MenuModel*)model { +- (NSMenu*)menuFromModel:(atom::AtomMenuModel*)model { NSMenu* menu = [[[NSMenu alloc] initWithTitle:@""] autorelease]; const int count = model->GetItemCount(); for (int index = 0; index < count; index++) { - if (model->GetTypeAt(index) == ui::MenuModel::TYPE_SEPARATOR) + if (model->GetTypeAt(index) == atom::AtomMenuModel::TYPE_SEPARATOR) [self addSeparatorToMenu:menu atIndex:index]; else [self addItemToMenu:menu atIndex:index fromModel:model]; @@ -126,9 +122,7 @@ Role kRolesMap[] = { // associated with the entry in the model identified by |modelIndex|. - (void)addItemToMenu:(NSMenu*)menu atIndex:(NSInteger)index - fromModel:(ui::MenuModel*)ui_model { - atom::AtomMenuModel* model = static_cast(ui_model); - + fromModel:(atom::AtomMenuModel*)model { base::string16 label16 = model->GetLabelAt(index); NSString* label = l10n_util::FixUpWindowsStyleLabel(label16); base::scoped_nsobject item( @@ -141,12 +135,13 @@ Role kRolesMap[] = { if (model->GetIconAt(index, &icon) && !icon.IsEmpty()) [item setImage:icon.ToNSImage()]; - ui::MenuModel::ItemType type = model->GetTypeAt(index); - if (type == ui::MenuModel::TYPE_SUBMENU) { + atom::AtomMenuModel::ItemType type = model->GetTypeAt(index); + if (type == atom::AtomMenuModel::TYPE_SUBMENU) { // Recursively build a submenu from the sub-model at this index. [item setTarget:nil]; [item setAction:nil]; - ui::MenuModel* submenuModel = model->GetSubmenuModelAt(index); + atom::AtomMenuModel* submenuModel = static_cast( + model->GetSubmenuModelAt(index)); NSMenu* submenu = [self menuFromModel:submenuModel]; [submenu setTitle:[item title]]; [item setSubmenu:submenu]; @@ -170,7 +165,8 @@ Role kRolesMap[] = { NSValue* modelObject = [NSValue valueWithPointer:model]; [item setRepresentedObject:modelObject]; // Retains |modelObject|. ui::Accelerator accelerator; - if (model->GetAcceleratorAt(index, &accelerator)) { + if (model->GetAcceleratorAtWithParams( + index, useDefaultAccelerator_, &accelerator)) { const ui::PlatformAcceleratorCocoa* platformAccelerator = static_cast( accelerator.platform_accelerator()); @@ -206,8 +202,8 @@ Role kRolesMap[] = { return NO; NSInteger modelIndex = [item tag]; - ui::MenuModel* model = - static_cast( + atom::AtomMenuModel* model = + static_cast( [[(id)item representedObject] pointerValue]); DCHECK(model); if (model) { @@ -234,8 +230,8 @@ Role kRolesMap[] = { // item chosen. - (void)itemSelected:(id)sender { NSInteger modelIndex = [sender tag]; - ui::MenuModel* model = - static_cast( + atom::AtomMenuModel* model = + static_cast( [[sender representedObject] pointerValue]); DCHECK(model); if (model) { diff --git a/atom/browser/ui/tray_icon.cc b/atom/browser/ui/tray_icon.cc index fda68b09cd1..88c2f9139a8 100644 --- a/atom/browser/ui/tray_icon.cc +++ b/atom/browser/ui/tray_icon.cc @@ -27,7 +27,7 @@ void TrayIcon::DisplayBalloon(ImageType icon, } void TrayIcon::PopUpContextMenu(const gfx::Point& pos, - ui::SimpleMenuModel* menu_model) { + AtomMenuModel* menu_model) { } gfx::Rect TrayIcon::GetBounds() { diff --git a/atom/browser/ui/tray_icon.h b/atom/browser/ui/tray_icon.h index 2763e50941b..617755dc363 100644 --- a/atom/browser/ui/tray_icon.h +++ b/atom/browser/ui/tray_icon.h @@ -8,9 +8,9 @@ #include #include +#include "atom/browser/ui/atom_menu_model.h" #include "atom/browser/ui/tray_icon_observer.h" #include "base/observer_list.h" -#include "ui/base/models/simple_menu_model.h" #include "ui/gfx/geometry/rect.h" namespace atom { @@ -55,10 +55,10 @@ class TrayIcon { // Popups the menu. virtual void PopUpContextMenu(const gfx::Point& pos, - ui::SimpleMenuModel* menu_model); + AtomMenuModel* menu_model); // Set the context menu for this icon. - virtual void SetContextMenu(ui::SimpleMenuModel* menu_model) = 0; + virtual void SetContextMenu(AtomMenuModel* menu_model) = 0; // Returns the bounds of tray icon. virtual gfx::Rect GetBounds(); diff --git a/atom/browser/ui/tray_icon_cocoa.h b/atom/browser/ui/tray_icon_cocoa.h index cb972d54a9a..e9abaa5590e 100644 --- a/atom/browser/ui/tray_icon_cocoa.h +++ b/atom/browser/ui/tray_icon_cocoa.h @@ -9,7 +9,6 @@ #include -#include "atom/browser/ui/atom_menu_model.h" #include "atom/browser/ui/tray_icon.h" #include "base/mac/scoped_nsobject.h" @@ -30,8 +29,8 @@ class TrayIconCocoa : public TrayIcon, void SetTitle(const std::string& title) override; void SetHighlightMode(bool highlight) override; void PopUpContextMenu(const gfx::Point& pos, - ui::SimpleMenuModel* menu_model) override; - void SetContextMenu(ui::SimpleMenuModel* menu_model) override; + AtomMenuModel* menu_model) override; + void SetContextMenu(AtomMenuModel* menu_model) override; gfx::Rect GetBounds() override; protected: diff --git a/atom/browser/ui/tray_icon_cocoa.mm b/atom/browser/ui/tray_icon_cocoa.mm index 2510d3160cc..7c2f12f0504 100644 --- a/atom/browser/ui/tray_icon_cocoa.mm +++ b/atom/browser/ui/tray_icon_cocoa.mm @@ -249,11 +249,12 @@ const CGFloat kVerticalTitleMargin = 2; [self setNeedsDisplay:YES]; } -- (void)popUpContextMenu:(ui::SimpleMenuModel*)menu_model { +- (void)popUpContextMenu:(atom::AtomMenuModel*)menu_model { // Show a custom menu. if (menu_model) { base::scoped_nsobject menuController( - [[AtomMenuController alloc] initWithModel:menu_model]); + [[AtomMenuController alloc] initWithModel:menu_model + useDefaultAccelerator:NO]); forceHighlight_ = YES; // Should highlight when showing menu. [self setNeedsDisplay:YES]; [statusItem_ popUpStatusItemMenu:[menuController menu]]; @@ -365,18 +366,19 @@ void TrayIconCocoa::SetHighlightMode(bool highlight) { } void TrayIconCocoa::PopUpContextMenu(const gfx::Point& pos, - ui::SimpleMenuModel* menu_model) { + AtomMenuModel* menu_model) { [status_item_view_ popUpContextMenu:menu_model]; } -void TrayIconCocoa::SetContextMenu(ui::SimpleMenuModel* menu_model) { +void TrayIconCocoa::SetContextMenu(AtomMenuModel* menu_model) { // Substribe to MenuClosed event. if (menu_model_) menu_model_->RemoveObserver(this); - static_cast(menu_model)->AddObserver(this); + menu_model->AddObserver(this); // Create native menu. - menu_.reset([[AtomMenuController alloc] initWithModel:menu_model]); + menu_.reset([[AtomMenuController alloc] initWithModel:menu_model + useDefaultAccelerator:NO]); [status_item_view_ setMenuController:menu_.get()]; } diff --git a/lib/browser/api/menu-item.js b/lib/browser/api/menu-item.js index 8fb1b51d1e6..d34cf3e3d10 100644 --- a/lib/browser/api/menu-item.js +++ b/lib/browser/api/menu-item.js @@ -31,7 +31,7 @@ const MenuItem = function (options) { this.overrideReadOnlyProperty('type', 'normal') this.overrideReadOnlyProperty('role') - this.overrideReadOnlyProperty('accelerator', roles.getDefaultAccelerator(this.role)) + this.overrideReadOnlyProperty('accelerator') this.overrideReadOnlyProperty('icon') this.overrideReadOnlyProperty('submenu') @@ -66,6 +66,10 @@ const MenuItem = function (options) { MenuItem.types = ['normal', 'separator', 'submenu', 'checkbox', 'radio'] +MenuItem.prototype.getDefaultRoleAccelerator = function () { + return roles.getDefaultAccelerator(this.role) +} + MenuItem.prototype.overrideProperty = function (name, defaultValue) { if (defaultValue == null) { defaultValue = null diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index 3b108219363..467cca6d09e 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -106,9 +106,11 @@ Menu.prototype._init = function () { var command = this.commandsMap[commandId] return command != null ? command.visible : undefined }, - getAcceleratorForCommandId: (commandId) => { - var command = this.commandsMap[commandId] - return command != null ? command.accelerator : undefined + getAcceleratorForCommandId: (commandId, useDefaultAccelerator) => { + const command = this.commandsMap[commandId] + if (command == null) return + if (command.accelerator != null) return command.accelerator + if (useDefaultAccelerator) return command.getDefaultRoleAccelerator() }, getIconForCommandId: (commandId) => { var command = this.commandsMap[commandId]