From 360266ba5bdf7a66bfd18f878d6eba03a0738613 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 22 Jan 2016 10:51:51 -0700 Subject: [PATCH 1/7] positioningItem => positioning_item --- atom/browser/api/atom_api_menu.h | 5 +++-- atom/browser/api/atom_api_menu_mac.h | 2 +- atom/browser/api/atom_api_menu_mac.mm | 12 ++++++------ atom/browser/api/atom_api_menu_views.cc | 2 +- atom/browser/api/atom_api_menu_views.h | 2 +- 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/atom/browser/api/atom_api_menu.h b/atom/browser/api/atom_api_menu.h index d3452628100..1ea83653e78 100644 --- a/atom/browser/api/atom_api_menu.h +++ b/atom/browser/api/atom_api_menu.h @@ -52,8 +52,9 @@ class Menu : public mate::TrackableObject, void MenuWillShow(ui::SimpleMenuModel* source) override; virtual void Popup(Window* window) = 0; - virtual void PopupAt(Window* window, int x, int y, - int positioningItem = 0) = 0; + virtual void PopupAt(Window* window, + int x = -1, int y = -1, + int positioning_item = 0) = 0; scoped_ptr model_; Menu* parent_; diff --git a/atom/browser/api/atom_api_menu_mac.h b/atom/browser/api/atom_api_menu_mac.h index e71198315b6..34450bc8b58 100644 --- a/atom/browser/api/atom_api_menu_mac.h +++ b/atom/browser/api/atom_api_menu_mac.h @@ -20,7 +20,7 @@ class MenuMac : public Menu { MenuMac(); void Popup(Window* window) override; - void PopupAt(Window* window, int x, int y, int positioningItem = 0) override; + void PopupAt(Window* window, int x, int y, int positioning_item = 0) override; base::scoped_nsobject menu_controller_; diff --git a/atom/browser/api/atom_api_menu_mac.mm b/atom/browser/api/atom_api_menu_mac.mm index 7e505ccd9d3..d87617a5fc6 100644 --- a/atom/browser/api/atom_api_menu_mac.mm +++ b/atom/browser/api/atom_api_menu_mac.mm @@ -50,7 +50,7 @@ void MenuMac::Popup(Window* window) { forView:web_contents->GetContentNativeView()]; } -void MenuMac::PopupAt(Window* window, int x, int y, int positioningItem) { +void MenuMac::PopupAt(Window* window, int x, int y, int positioning_item) { NativeWindow* native_window = window->window(); if (!native_window) return; @@ -64,13 +64,13 @@ void MenuMac::PopupAt(Window* window, int x, int y, int positioningItem) { NSView* view = web_contents->GetContentNativeView(); // Show the menu. - if (positioningItem >= [menu numberOfItems]) { - positioningItem = [menu numberOfItems] - 1; + if (positioning_item >= [menu numberOfItems]) { + positioning_item = [menu numberOfItems] - 1; } - if (positioningItem < 0) { - positioningItem = 0; + if (positioning_item < 0) { + positioning_item = 0; } - [menu popUpMenuPositioningItem:[menu itemAtIndex:positioningItem] + [menu popUpMenupositioning_item:[menu itemAtIndex:positioning_item] atLocation:NSMakePoint(x, [view frame].size.height - y) inView:view]; } diff --git a/atom/browser/api/atom_api_menu_views.cc b/atom/browser/api/atom_api_menu_views.cc index 67b55d5e988..93869df87f0 100644 --- a/atom/browser/api/atom_api_menu_views.cc +++ b/atom/browser/api/atom_api_menu_views.cc @@ -20,7 +20,7 @@ void MenuViews::Popup(Window* window) { PopupAtPoint(window, gfx::Screen::GetNativeScreen()->GetCursorScreenPoint()); } -void MenuViews::PopupAt(Window* window, int x, int y, int positioningItem) { +void MenuViews::PopupAt(Window* window, int x, int y, int positioning_item) { NativeWindow* native_window = static_cast(window->window()); if (!native_window) return; diff --git a/atom/browser/api/atom_api_menu_views.h b/atom/browser/api/atom_api_menu_views.h index 9e306b4e72b..9f1b116a004 100644 --- a/atom/browser/api/atom_api_menu_views.h +++ b/atom/browser/api/atom_api_menu_views.h @@ -18,7 +18,7 @@ class MenuViews : public Menu { protected: void Popup(Window* window) override; - void PopupAt(Window* window, int x, int y, int positioningItem = 0) override; + void PopupAt(Window* window, int x, int y, int positioning_item = 0) override; private: void PopupAtPoint(Window* window, const gfx::Point& point); From 5f195a789ae326fb8a33188ef97686f8b44298f4 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 22 Jan 2016 11:17:12 -0700 Subject: [PATCH 2/7] mac: Remove duplicate code of Popup --- atom/browser/api/atom_api_menu_mac.mm | 55 +++++++++------------------ 1 file changed, 17 insertions(+), 38 deletions(-) diff --git a/atom/browser/api/atom_api_menu_mac.mm b/atom/browser/api/atom_api_menu_mac.mm index d87617a5fc6..cfd3f0a89c3 100644 --- a/atom/browser/api/atom_api_menu_mac.mm +++ b/atom/browser/api/atom_api_menu_mac.mm @@ -19,35 +19,7 @@ MenuMac::MenuMac() { } void MenuMac::Popup(Window* window) { - NativeWindow* native_window = window->window(); - if (!native_window) - return; - content::WebContents* web_contents = native_window->web_contents(); - if (!web_contents) - return; - - NSWindow* nswindow = native_window->GetNativeWindow(); - base::scoped_nsobject menu_controller( - [[AtomMenuController alloc] initWithModel:model_.get()]); - - // Fake out a context menu event. - NSEvent* currentEvent = [NSApp currentEvent]; - NSPoint position = [nswindow mouseLocationOutsideOfEventStream]; - NSTimeInterval eventTime = [currentEvent timestamp]; - NSEvent* clickEvent = [NSEvent mouseEventWithType:NSRightMouseDown - location:position - modifierFlags:NSRightMouseDownMask - timestamp:eventTime - windowNumber:[nswindow windowNumber] - context:nil - eventNumber:0 - clickCount:1 - pressure:1.0]; - - // Show the menu. - [NSMenu popUpContextMenu:[menu_controller menu] - withEvent:clickEvent - forView:web_contents->GetContentNativeView()]; + PopupAt(window, -1, -1, 0); } void MenuMac::PopupAt(Window* window, int x, int y, int positioning_item) { @@ -63,16 +35,23 @@ void MenuMac::PopupAt(Window* window, int x, int y, int positioning_item) { NSMenu* menu = [menu_controller menu]; NSView* view = web_contents->GetContentNativeView(); + // Which menu item to show. + NSMenuItem* item = nil; + if (positioning_item < [menu numberOfItems] && positioning_item >= 0) + item = [menu itemAtIndex:positioning_item]; + + // (-1, -1) means showing on mouse location. + NSPoint position; + if (x == -1 || y == -1) { + NSWindow* nswindow = native_window->GetNativeWindow(); + position = [view convertPoint:[nswindow mouseLocationOutsideOfEventStream] + fromView:nil]; + } else { + position = NSMakePoint(x, [view frame].size.height - y); + } + // Show the menu. - if (positioning_item >= [menu numberOfItems]) { - positioning_item = [menu numberOfItems] - 1; - } - if (positioning_item < 0) { - positioning_item = 0; - } - [menu popUpMenupositioning_item:[menu itemAtIndex:positioning_item] - atLocation:NSMakePoint(x, [view frame].size.height - y) - inView:view]; + [menu popUpMenuPositioningItem:item atLocation:position inView:view]; } // static From 0e3a3d07484653bb50632cc8e2f7832b365dd6fc Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 22 Jan 2016 11:25:16 -0700 Subject: [PATCH 3/7] views: Remove PopupAtPoint --- atom/browser/api/atom_api_menu_views.cc | 17 +++++++++++------ atom/browser/api/atom_api_menu_views.h | 2 -- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/atom/browser/api/atom_api_menu_views.cc b/atom/browser/api/atom_api_menu_views.cc index 93869df87f0..248523c4537 100644 --- a/atom/browser/api/atom_api_menu_views.cc +++ b/atom/browser/api/atom_api_menu_views.cc @@ -17,7 +17,7 @@ MenuViews::MenuViews() { } void MenuViews::Popup(Window* window) { - PopupAtPoint(window, gfx::Screen::GetNativeScreen()->GetCursorScreenPoint()); + PopupAt(window, -1, -1, 0); } void MenuViews::PopupAt(Window* window, int x, int y, int positioning_item) { @@ -31,18 +31,23 @@ void MenuViews::PopupAt(Window* window, int x, int y, int positioning_item) { if (!view) return; - gfx::Point origin = view->GetViewBounds().origin(); - PopupAtPoint(window, gfx::Point(origin.x() + x, origin.y() + y)); -} + // (-1, -1) means showing on mouse location. + gfx::Point location; + if (x == -1 || y == -1) { + location = gfx::Screen::GetNativeScreen()->GetCursorScreenPoint(); + } else { + gfx::Point origin = view->GetViewBounds().origin(); + location = gfx::Point(origin.x() + x, origin.y() + y); + } -void MenuViews::PopupAtPoint(Window* window, const gfx::Point& point) { + // Show the menu. views::MenuRunner menu_runner( model(), views::MenuRunner::CONTEXT_MENU | views::MenuRunner::HAS_MNEMONICS); ignore_result(menu_runner.RunMenuAt( static_cast(window->window())->widget(), NULL, - gfx::Rect(point, gfx::Size()), + gfx::Rect(location, gfx::Size()), views::MENU_ANCHOR_TOPLEFT, ui::MENU_SOURCE_MOUSE)); } diff --git a/atom/browser/api/atom_api_menu_views.h b/atom/browser/api/atom_api_menu_views.h index 9f1b116a004..802e1405fd5 100644 --- a/atom/browser/api/atom_api_menu_views.h +++ b/atom/browser/api/atom_api_menu_views.h @@ -21,8 +21,6 @@ class MenuViews : public Menu { void PopupAt(Window* window, int x, int y, int positioning_item = 0) override; private: - void PopupAtPoint(Window* window, const gfx::Point& point); - DISALLOW_COPY_AND_ASSIGN(MenuViews); }; From ca77c95c6d5e5fe959f7b406acfeee8f558944b3 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 22 Jan 2016 11:27:17 -0700 Subject: [PATCH 4/7] No more need to override Menu::Popup --- atom/browser/api/atom_api_menu.cc | 4 ++++ atom/browser/api/atom_api_menu.h | 2 +- atom/browser/api/atom_api_menu_mac.h | 1 - atom/browser/api/atom_api_menu_mac.mm | 4 ---- atom/browser/api/atom_api_menu_views.cc | 4 ---- atom/browser/api/atom_api_menu_views.h | 1 - 6 files changed, 5 insertions(+), 11 deletions(-) diff --git a/atom/browser/api/atom_api_menu.cc b/atom/browser/api/atom_api_menu.cc index 96cba3fe991..ec565073900 100644 --- a/atom/browser/api/atom_api_menu.cc +++ b/atom/browser/api/atom_api_menu.cc @@ -69,6 +69,10 @@ void Menu::MenuWillShow(ui::SimpleMenuModel* source) { menu_will_show_.Run(); } +void Menu::Popup(Window* window) { + PopupAt(window); +} + void Menu::InsertItemAt( int index, int command_id, const base::string16& label) { model_->InsertItemAt(index, command_id, label); diff --git a/atom/browser/api/atom_api_menu.h b/atom/browser/api/atom_api_menu.h index 1ea83653e78..18cfae6a348 100644 --- a/atom/browser/api/atom_api_menu.h +++ b/atom/browser/api/atom_api_menu.h @@ -51,7 +51,7 @@ class Menu : public mate::TrackableObject, void ExecuteCommand(int command_id, int event_flags) override; void MenuWillShow(ui::SimpleMenuModel* source) override; - virtual void Popup(Window* window) = 0; + void Popup(Window* window); virtual void PopupAt(Window* window, int x = -1, int y = -1, int positioning_item = 0) = 0; diff --git a/atom/browser/api/atom_api_menu_mac.h b/atom/browser/api/atom_api_menu_mac.h index 34450bc8b58..85227fa2a9d 100644 --- a/atom/browser/api/atom_api_menu_mac.h +++ b/atom/browser/api/atom_api_menu_mac.h @@ -19,7 +19,6 @@ class MenuMac : public Menu { protected: MenuMac(); - void Popup(Window* window) override; void PopupAt(Window* window, int x, int y, int positioning_item = 0) override; base::scoped_nsobject menu_controller_; diff --git a/atom/browser/api/atom_api_menu_mac.mm b/atom/browser/api/atom_api_menu_mac.mm index cfd3f0a89c3..71c677b0476 100644 --- a/atom/browser/api/atom_api_menu_mac.mm +++ b/atom/browser/api/atom_api_menu_mac.mm @@ -18,10 +18,6 @@ namespace api { MenuMac::MenuMac() { } -void MenuMac::Popup(Window* window) { - PopupAt(window, -1, -1, 0); -} - void MenuMac::PopupAt(Window* window, int x, int y, int positioning_item) { NativeWindow* native_window = window->window(); if (!native_window) diff --git a/atom/browser/api/atom_api_menu_views.cc b/atom/browser/api/atom_api_menu_views.cc index 248523c4537..4a3a97dd906 100644 --- a/atom/browser/api/atom_api_menu_views.cc +++ b/atom/browser/api/atom_api_menu_views.cc @@ -16,10 +16,6 @@ namespace api { MenuViews::MenuViews() { } -void MenuViews::Popup(Window* window) { - PopupAt(window, -1, -1, 0); -} - void MenuViews::PopupAt(Window* window, int x, int y, int positioning_item) { NativeWindow* native_window = static_cast(window->window()); if (!native_window) diff --git a/atom/browser/api/atom_api_menu_views.h b/atom/browser/api/atom_api_menu_views.h index 802e1405fd5..e4d17c77ca6 100644 --- a/atom/browser/api/atom_api_menu_views.h +++ b/atom/browser/api/atom_api_menu_views.h @@ -17,7 +17,6 @@ class MenuViews : public Menu { MenuViews(); protected: - void Popup(Window* window) override; void PopupAt(Window* window, int x, int y, int positioning_item = 0) override; private: From 2d8e5e3a16cb624c4f69501ac3c70aa28bc1735a Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 22 Jan 2016 11:29:26 -0700 Subject: [PATCH 5/7] docs: Default parameter for Menu.popup --- docs/api/menu.md | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/docs/api/menu.md b/docs/api/menu.md index 7b563d57801..ca35d4bb42a 100644 --- a/docs/api/menu.md +++ b/docs/api/menu.md @@ -239,16 +239,15 @@ will become properties of the constructed menu items. ### `Menu.popup([browserWindow, x, y, positioningItem])` -* `browserWindow` BrowserWindow (optional) -* `x` Number (optional) -* `y` Number (**required** if `x` is used) -* `positioningItem` Number (optional) _OS X_ +* `browserWindow` BrowserWindow (optional) - Default is `null`. +* `x` Number (optional) - Default is -1. +* `y` Number (**required** if `x` is used) - Default is -1. +* `positioningItem` Number (optional) _OS X_ - the index of the menu item to + be positioned under the mouse cursor at the specified coordinates. -Pops up this menu as a context menu in the `browserWindow`. You -can optionally provide a `x,y` coordinate to place the menu at, otherwise it -will be placed at the current mouse cursor position. `positioningItem` is the -index of the menu item to be positioned under the mouse cursor at the specified -coordinates (only supported on OS X). +Pops up this menu as a context menu in the `browserWindow`. You can optionally +provide a `x, y` coordinate to place the menu at, otherwise it will be placed +at the current mouse cursor position. ### `Menu.append(menuItem)` From d1051b55cc9d970fd64bd8ebdf767f294c1e400a Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 22 Jan 2016 11:32:14 -0700 Subject: [PATCH 6/7] docs: Default value of positioningItem --- docs/api/menu.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/api/menu.md b/docs/api/menu.md index ca35d4bb42a..0e66fc216d5 100644 --- a/docs/api/menu.md +++ b/docs/api/menu.md @@ -242,8 +242,9 @@ will become properties of the constructed menu items. * `browserWindow` BrowserWindow (optional) - Default is `null`. * `x` Number (optional) - Default is -1. * `y` Number (**required** if `x` is used) - Default is -1. -* `positioningItem` Number (optional) _OS X_ - the index of the menu item to - be positioned under the mouse cursor at the specified coordinates. +* `positioningItem` Number (optional) _OS X_ - The index of the menu item to + be positioned under the mouse cursor at the specified coordinates. Default is + -1. Pops up this menu as a context menu in the `browserWindow`. You can optionally provide a `x, y` coordinate to place the menu at, otherwise it will be placed From 984462be447593a1a7fe64964105b51a3154b344 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 22 Jan 2016 11:59:08 -0700 Subject: [PATCH 7/7] Remove Menu::Popup --- atom/browser/api/atom_api_menu.cc | 7 +------ atom/browser/api/atom_api_menu.h | 1 - atom/browser/api/lib/menu.js | 15 +++++++++------ 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/atom/browser/api/atom_api_menu.cc b/atom/browser/api/atom_api_menu.cc index ec565073900..e40ba17f464 100644 --- a/atom/browser/api/atom_api_menu.cc +++ b/atom/browser/api/atom_api_menu.cc @@ -69,10 +69,6 @@ void Menu::MenuWillShow(ui::SimpleMenuModel* source) { menu_will_show_.Run(); } -void Menu::Popup(Window* window) { - PopupAt(window); -} - void Menu::InsertItemAt( int index, int command_id, const base::string16& label) { model_->InsertItemAt(index, command_id, label); @@ -173,8 +169,7 @@ void Menu::BuildPrototype(v8::Isolate* isolate, .SetMethod("isItemCheckedAt", &Menu::IsItemCheckedAt) .SetMethod("isEnabledAt", &Menu::IsEnabledAt) .SetMethod("isVisibleAt", &Menu::IsVisibleAt) - .SetMethod("_popup", &Menu::Popup) - .SetMethod("_popupAt", &Menu::PopupAt); + .SetMethod("popupAt", &Menu::PopupAt); } } // namespace api diff --git a/atom/browser/api/atom_api_menu.h b/atom/browser/api/atom_api_menu.h index 18cfae6a348..1ae708863a7 100644 --- a/atom/browser/api/atom_api_menu.h +++ b/atom/browser/api/atom_api_menu.h @@ -51,7 +51,6 @@ class Menu : public mate::TrackableObject, void ExecuteCommand(int command_id, int event_flags) override; void MenuWillShow(ui::SimpleMenuModel* source) override; - void Popup(Window* window); virtual void PopupAt(Window* window, int x = -1, int y = -1, int positioning_item = 0) = 0; diff --git a/atom/browser/api/lib/menu.js b/atom/browser/api/lib/menu.js index 506922fd4a8..62c771af6c6 100644 --- a/atom/browser/api/lib/menu.js +++ b/atom/browser/api/lib/menu.js @@ -159,17 +159,20 @@ Menu.prototype._init = function() { }; Menu.prototype.popup = function(window, x, y, positioningItem) { - if ((window != null ? window.constructor : void 0) !== BrowserWindow) { + if (typeof window != 'object' || window.constructor !== BrowserWindow) { // Shift. + positioningItem = y; y = x; x = window; window = BrowserWindow.getFocusedWindow(); } - if ((x != null) && (y != null)) { - return this._popupAt(window, x, y, positioningItem || 0); - } else { - return this._popup(window); - } + + // Default parameters. + if (typeof x !== 'number') x = -1; + if (typeof y !== 'number') y = -1; + if (typeof positioningItem !== 'number') positioningItem = 0; + + this.popupAt(window, x, y, positioningItem); }; Menu.prototype.append = function(item) {