diff --git a/DEPS b/DEPS index f6031d46b5..b348c190d5 100644 --- a/DEPS +++ b/DEPS @@ -2,7 +2,7 @@ use_relative_paths = True deps = { "vendor/node": "https://github.com/brave/node.git@f51b9ab8ff446ca7b13be0de1bc12b854b23938d", - "vendor/native_mate": "https://github.com/zcbenz/native-mate.git@b5e5de626c6a57e44c7e6448d8bbaaac475d493c", + "vendor/native_mate": "https://github.com/zcbenz/native-mate.git@ad0fd825663932ee3fa29ff935dfec99933bdd8c", "vendor/requests": "https://github.com/kennethreitz/requests@e4d59bedfd3c7f4f254f4f5d036587bcd8152458", "vendor/boto": "https://github.com/boto/boto@f7574aa6cc2c819430c1f05e9a1a1a666ef8169b", "vendor/python-patch": "https://github.com/svn2github/python-patch@a336a458016ced89aba90dfc3f4c8222ae3b1403", diff --git a/atom/browser/api/atom_api_menu.cc b/atom/browser/api/atom_api_menu.cc index 88cb0ef181..09d5cc8f2d 100644 --- a/atom/browser/api/atom_api_menu.cc +++ b/atom/browser/api/atom_api_menu.cc @@ -190,7 +190,8 @@ void Menu::BuildPrototype(v8::Isolate* isolate, .SetMethod("isItemCheckedAt", &Menu::IsItemCheckedAt) .SetMethod("isEnabledAt", &Menu::IsEnabledAt) .SetMethod("isVisibleAt", &Menu::IsVisibleAt) - .SetMethod("popupAt", &Menu::PopupAt); + .SetMethod("popupAt", &Menu::PopupAt) + .SetMethod("closePopupAt", &Menu::ClosePopupAt); } } // namespace api diff --git a/atom/browser/api/atom_api_menu.h b/atom/browser/api/atom_api_menu.h index a918e23fed..b661082b70 100644 --- a/atom/browser/api/atom_api_menu.h +++ b/atom/browser/api/atom_api_menu.h @@ -62,9 +62,9 @@ class Menu : public mate::TrackableObject, void ExecuteCommand(int command_id, int event_flags) override; void MenuWillShow(ui::SimpleMenuModel* source) override; - virtual void PopupAt(Window* window, - int x = -1, int y = -1, - int positioning_item = 0) = 0; + virtual void PopupAt( + Window* window, int x, int y, int positioning_item) = 0; + virtual void ClosePopupAt(int32_t window_id) = 0; std::unique_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 d318b7bdc0..887e998b89 100644 --- a/atom/browser/api/atom_api_menu_mac.h +++ b/atom/browser/api/atom_api_menu_mac.h @@ -7,10 +7,13 @@ #include "atom/browser/api/atom_api_menu.h" +#include #include #import "atom/browser/ui/cocoa/atom_menu_controller.h" +using base::scoped_nsobject; + namespace atom { namespace api { @@ -19,15 +22,24 @@ class MenuMac : public Menu { protected: MenuMac(v8::Isolate* isolate, v8::Local wrapper); - void PopupAt(Window* window, int x, int y, int positioning_item) override; - - base::scoped_nsobject menu_controller_; + void PopupAt( + Window* window, int x, int y, int positioning_item) override; + void PopupOnUI(const base::WeakPtr& native_window, + int32_t window_id, int x, int y, int positioning_item); + void ClosePopupAt(int32_t window_id) override; private: friend class Menu; static void SendActionToFirstResponder(const std::string& action); + scoped_nsobject menu_controller_; + + // window ID -> open context menu + std::map> popup_controllers_; + + base::WeakPtrFactory weak_factory_; + DISALLOW_COPY_AND_ASSIGN(MenuMac); }; diff --git a/atom/browser/api/atom_api_menu_mac.mm b/atom/browser/api/atom_api_menu_mac.mm index 45956d5ccb..ddb2688a85 100644 --- a/atom/browser/api/atom_api_menu_mac.mm +++ b/atom/browser/api/atom_api_menu_mac.mm @@ -6,35 +6,55 @@ #include "atom/browser/native_window.h" #include "atom/browser/unresponsive_suppressor.h" +#include "base/mac/scoped_sending_event.h" #include "base/message_loop/message_loop.h" #include "base/strings/sys_string_conversions.h" #include "brightray/browser/inspectable_web_contents.h" #include "brightray/browser/inspectable_web_contents_view.h" +#include "content/public/browser/browser_thread.h" #include "content/public/browser/web_contents.h" #include "atom/common/node_includes.h" +using content::BrowserThread; + namespace atom { namespace api { MenuMac::MenuMac(v8::Isolate* isolate, v8::Local wrapper) - : Menu(isolate, wrapper) { + : Menu(isolate, wrapper), + weak_factory_(this) { } -void MenuMac::PopupAt(Window* window, int x, int y, int positioning_item) { +void MenuMac::PopupAt( + Window* window, int x, int y, int positioning_item) { NativeWindow* native_window = window->window(); if (!native_window) return; + + auto popup = base::Bind(&MenuMac::PopupOnUI, weak_factory_.GetWeakPtr(), + native_window->GetWeakPtr(), window->ID(), x, y, + positioning_item); + + BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, popup); +} + +void MenuMac::PopupOnUI(const base::WeakPtr& native_window, + int32_t window_id, int x, int y, int positioning_item) { + if (!native_window) + return; brightray::InspectableWebContents* web_contents = native_window->inspectable_web_contents(); if (!web_contents || !web_contents->GetWebContents()) return; - base::scoped_nsobject menu_controller( - [[AtomMenuController alloc] initWithModel:model_.get() + auto close_callback = base::Bind(&MenuMac::ClosePopupAt, + weak_factory_.GetWeakPtr(), window_id); + popup_controllers_[window_id] = base::scoped_nsobject( + [[AtomMenuController alloc] initWithModel:model() useDefaultAccelerator:NO]); - NSMenu* menu = [menu_controller menu]; + NSMenu* menu = [popup_controllers_[window_id] menu]; NSView* view = web_contents->GetWebContents()->GetNativeView(); // Which menu item to show. @@ -69,13 +89,27 @@ if (rightmostMenuPoint > screenRight) position.x = position.x - [menu size].width; + [popup_controllers_[window_id] setCloseCallback:close_callback]; + // Make sure events can be pumped while the menu is up. + base::MessageLoop::ScopedNestableTaskAllower allow( + base::MessageLoop::current()); + + // One of the events that could be pumped is |window.close()|. + // User-initiated event-tracking loops protect against this by + // setting flags in -[CrApplication sendEvent:], but since + // web-content menus are initiated by IPC message the setup has to + // be done manually. + base::mac::ScopedSendingEvent sendingEventScoper; + // Don't emit unresponsive event when showing menu. atom::UnresponsiveSuppressor suppressor; - - // Show the menu. [menu popUpMenuPositioningItem:item atLocation:position inView:view]; } +void MenuMac::ClosePopupAt(int32_t window_id) { + popup_controllers_.erase(window_id); +} + // static void Menu::SetApplicationMenu(Menu* base_menu) { MenuMac* menu = static_cast(base_menu); diff --git a/atom/browser/api/atom_api_menu_views.cc b/atom/browser/api/atom_api_menu_views.cc index 8a72247c9d..94a4e4134a 100644 --- a/atom/browser/api/atom_api_menu_views.cc +++ b/atom/browser/api/atom_api_menu_views.cc @@ -2,23 +2,28 @@ // Use of this source code is governed by the MIT license that can be // found in the LICENSE file. +#include + #include "atom/browser/api/atom_api_menu_views.h" #include "atom/browser/native_window_views.h" #include "atom/browser/unresponsive_suppressor.h" #include "content/public/browser/render_widget_host_view.h" #include "ui/display/screen.h" -#include "ui/views/controls/menu/menu_runner.h" + +using views::MenuRunner; namespace atom { namespace api { MenuViews::MenuViews(v8::Isolate* isolate, v8::Local wrapper) - : Menu(isolate, wrapper) { + : Menu(isolate, wrapper), + weak_factory_(this) { } -void MenuViews::PopupAt(Window* window, int x, int y, int positioning_item) { +void MenuViews::PopupAt( + Window* window, int x, int y, int positioning_item) { NativeWindow* native_window = static_cast(window->window()); if (!native_window) return; @@ -38,14 +43,20 @@ void MenuViews::PopupAt(Window* window, int x, int y, int positioning_item) { location = gfx::Point(origin.x() + x, origin.y() + y); } + int flags = MenuRunner::CONTEXT_MENU | + MenuRunner::HAS_MNEMONICS | + MenuRunner::ASYNC; + // Don't emit unresponsive event when showing menu. atom::UnresponsiveSuppressor suppressor; // Show the menu. - views::MenuRunner menu_runner( - model(), - views::MenuRunner::CONTEXT_MENU | views::MenuRunner::HAS_MNEMONICS); - ignore_result(menu_runner.RunMenuAt( + int32_t window_id = window->ID(); + auto close_callback = base::Bind( + &MenuViews::ClosePopupAt, weak_factory_.GetWeakPtr(), window_id); + menu_runners_[window_id] = std::unique_ptr(new MenuRunner( + model(), flags, close_callback)); + ignore_result(menu_runners_[window_id]->RunMenuAt( static_cast(window->window())->widget(), NULL, gfx::Rect(location, gfx::Size()), @@ -53,6 +64,10 @@ void MenuViews::PopupAt(Window* window, int x, int y, int positioning_item) { ui::MENU_SOURCE_MOUSE)); } +void MenuViews::ClosePopupAt(int32_t window_id) { + menu_runners_.erase(window_id); +} + // static mate::WrappableBase* Menu::New(mate::Arguments* args) { return new MenuViews(args->isolate(), args->GetThis()); diff --git a/atom/browser/api/atom_api_menu_views.h b/atom/browser/api/atom_api_menu_views.h index 1e7abd1372..81dd754f99 100644 --- a/atom/browser/api/atom_api_menu_views.h +++ b/atom/browser/api/atom_api_menu_views.h @@ -5,8 +5,13 @@ #ifndef ATOM_BROWSER_API_ATOM_API_MENU_VIEWS_H_ #define ATOM_BROWSER_API_ATOM_API_MENU_VIEWS_H_ +#include +#include + #include "atom/browser/api/atom_api_menu.h" +#include "base/memory/weak_ptr.h" #include "ui/display/screen.h" +#include "ui/views/controls/menu/menu_runner.h" namespace atom { @@ -17,9 +22,16 @@ class MenuViews : public Menu { MenuViews(v8::Isolate* isolate, v8::Local wrapper); protected: - void PopupAt(Window* window, int x, int y, int positioning_item) override; + void PopupAt( + Window* window, int x, int y, int positioning_item) override; + void ClosePopupAt(int32_t window_id) override; private: + // window ID -> open context menu + std::map> menu_runners_; + + base::WeakPtrFactory weak_factory_; + DISALLOW_COPY_AND_ASSIGN(MenuViews); }; diff --git a/atom/browser/api/atom_api_window.h b/atom/browser/api/atom_api_window.h index f78678acd1..e174ffdc29 100644 --- a/atom/browser/api/atom_api_window.h +++ b/atom/browser/api/atom_api_window.h @@ -55,6 +55,8 @@ class Window : public mate::TrackableObject, NativeWindow* window() const { return window_.get(); } + int32_t ID() const; + protected: Window(v8::Isolate* isolate, v8::Local wrapper, const mate::Dictionary& options); @@ -198,7 +200,6 @@ class Window : public mate::TrackableObject, void SetVisibleOnAllWorkspaces(bool visible); bool IsVisibleOnAllWorkspaces(); - int32_t ID() const; v8::Local WebContents(v8::Isolate* isolate); // Remove this window from parent window's |child_windows_|. diff --git a/atom/browser/ui/cocoa/atom_menu_controller.h b/atom/browser/ui/cocoa/atom_menu_controller.h index af0b276961..a230437f53 100644 --- a/atom/browser/ui/cocoa/atom_menu_controller.h +++ b/atom/browser/ui/cocoa/atom_menu_controller.h @@ -8,6 +8,7 @@ #import +#include "base/callback.h" #include "base/mac/scoped_nsobject.h" #include "base/strings/string16.h" @@ -27,6 +28,7 @@ class AtomMenuModel; base::scoped_nsobject menu_; BOOL isMenuOpen_; BOOL useDefaultAccelerator_; + base::Callback closeCallback; } @property(nonatomic, assign) atom::AtomMenuModel* model; @@ -35,6 +37,8 @@ class AtomMenuModel; // to the contents of the model after calling this will not be noticed. - (id)initWithModel:(atom::AtomMenuModel*)model useDefaultAccelerator:(BOOL)use; +- (void)setCloseCallback:(const base::Callback&)callback; + // Populate current NSMenu with |model|. - (void)populateWithModel:(atom::AtomMenuModel*)model; diff --git a/atom/browser/ui/cocoa/atom_menu_controller.mm b/atom/browser/ui/cocoa/atom_menu_controller.mm index 6bdaa4c780..b3e293153f 100644 --- a/atom/browser/ui/cocoa/atom_menu_controller.mm +++ b/atom/browser/ui/cocoa/atom_menu_controller.mm @@ -12,9 +12,12 @@ #include "ui/base/accelerators/accelerator.h" #include "ui/base/accelerators/platform_accelerator_cocoa.h" #include "ui/base/l10n/l10n_util_mac.h" +#include "content/public/browser/browser_thread.h" #include "ui/events/cocoa/cocoa_event_utils.h" #include "ui/gfx/image/image.h" +using content::BrowserThread; + namespace { struct Role { @@ -71,6 +74,10 @@ - (void)dealloc { [super dealloc]; } +- (void)setCloseCallback:(const base::Callback&)callback { + closeCallback = callback; +} + - (void)populateWithModel:(atom::AtomMenuModel*)model { if (!menu_) return; @@ -265,8 +272,13 @@ - (void)menuWillOpen:(NSMenu*)menu { - (void)menuDidClose:(NSMenu*)menu { if (isMenuOpen_) { - model_->MenuWillClose(); isMenuOpen_ = NO; + model_->MenuWillClose(); + + // Post async task so that itemSelected runs before the close callback + // deletes the controller from the map which deallocates it + if (!closeCallback.is_null()) + BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, closeCallback); } } diff --git a/brave/common/extensions/url_bindings.cc b/brave/common/extensions/url_bindings.cc index d12112551a..1ce70d2a48 100644 --- a/brave/common/extensions/url_bindings.cc +++ b/brave/common/extensions/url_bindings.cc @@ -330,7 +330,8 @@ void URLBindings::Parse( GURL gurl(url_string); gin::Dictionary dict = gin::Dictionary::CreateEmpty(isolate); if (gurl.has_username()) - dict.Set("auth", gurl.username() + (gurl.has_password() ? ":" + gurl.password() : "")); + dict.Set("auth", gurl.username() + (gurl.has_password() + ? ":" + gurl.password() : "")); dict.Set("hash", gurl.ref()); dict.Set("hostname", gurl.host()); dict.Set("host", gurl.host() + ":" + gurl.port()); diff --git a/docs/api/menu.md b/docs/api/menu.md index 4e039c6b18..797093f232 100644 --- a/docs/api/menu.md +++ b/docs/api/menu.md @@ -264,17 +264,25 @@ will become properties of the constructed menu items. The `menu` object has the following instance methods: -#### `menu.popup([browserWindow, x, y, positioningItem])` +#### `menu.popup([browserWindow, options])` -* `browserWindow` BrowserWindow (optional) - Default is `BrowserWindow.getFocusedWindow()`. -* `x` Number (optional) - Default is the current mouse cursor position. -* `y` Number (**required** if `x` is used) - Default is the current mouse cursor position. -* `positioningItem` Number (optional) _macOS_ - The index of the menu item to - be positioned under the mouse cursor at the specified coordinates. Default is - -1. +* `browserWindow` BrowserWindow (optional) - Default is the focused window. +* `options` Object (optional) + * `x` Number (optional) - Default is the current mouse cursor position. + * `y` Number (**required** if `x` is used) - Default is the current mouse + cursor position. + * `positioningItem` Number (optional) _macOS_ - 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`. +#### `menu.closePopup([browserWindow])` + +* `browserWindow` BrowserWindow (optional) - Default is the focused window. + +Closes the context menu in the `browserWindow`. + #### `menu.append(menuItem)` * `menuItem` MenuItem diff --git a/docs/tutorial/planned-breaking-changes.md b/docs/tutorial/planned-breaking-changes.md index 896fd07000..250c14020b 100644 --- a/docs/tutorial/planned-breaking-changes.md +++ b/docs/tutorial/planned-breaking-changes.md @@ -40,6 +40,15 @@ clipboard.writeHtml() clipboard.writeHTML() ``` +## `menu` + +```js +// Deprecated +menu.popup(browserWindow, 100, 200, 2) +// Replace with +menu.popup(browserWindow, {x: 100, y: 200, positioningItem: 2}) +``` + ## `nativeImage` ```js diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index a68383cfd6..121ed0f923 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -144,6 +144,7 @@ Menu.prototype._init = function () { } Menu.prototype.popup = function (window, x, y, positioningItem) { + // menu.popup(x, y, positioningItem) if (typeof window !== 'object' || window.constructor !== BrowserWindow) { // Shift. positioningItem = y @@ -152,6 +153,14 @@ Menu.prototype.popup = function (window, x, y, positioningItem) { window = BrowserWindow.getFocusedWindow() } + // menu.popup(window, {}) + if (x != null && typeof x === 'object') { + const options = x + x = options.x + y = options.y + positioningItem = options.positioningItem + } + // Default to showing under mouse location. if (typeof x !== 'number') x = -1 if (typeof y !== 'number') y = -1 @@ -162,6 +171,15 @@ Menu.prototype.popup = function (window, x, y, positioningItem) { this.popupAt(window, x, y, positioningItem) } +Menu.prototype.closePopup = function (window) { + if (window == null || window.constructor !== BrowserWindow) { + window = BrowserWindow.getFocusedWindow() + } + if (window != null) { + this.closePopupAt(window.id) + } +} + Menu.prototype.append = function (item) { return this.insert(this.getItemCount(), item) } diff --git a/spec/api-menu-spec.js b/spec/api-menu-spec.js index 4ea6d71cdb..1973898728 100644 --- a/spec/api-menu-spec.js +++ b/spec/api-menu-spec.js @@ -1,7 +1,8 @@ const assert = require('assert') const {ipcRenderer, remote} = require('electron') -const {Menu, MenuItem} = remote +const {BrowserWindow, Menu, MenuItem} = remote +const {closeWindow} = require('./window-helpers') describe('menu module', function () { describe('Menu.buildFromTemplate', function () { @@ -216,6 +217,29 @@ describe('menu module', function () { }) }) + describe('Menu.popup', function () { + let w = null + + afterEach(function () { + return closeWindow(w).then(function () { w = null }) + }) + + it('returns immediately (default async)', function () { + w = new BrowserWindow({show: false, width: 200, height: 200}) + const menu = Menu.buildFromTemplate([ + { + label: '1' + }, { + label: '2' + }, { + label: '3' + } + ]) + menu.popup(w, {x: 100, y: 100}) + menu.closePopup(w) + }) + }) + describe('MenuItem.click', function () { it('should be called with the item object passed', function (done) { var menu = Menu.buildFromTemplate([