From a08ca9defbc688a5cefa06b87d993bccc7532b60 Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Mon, 27 Aug 2018 18:58:47 +0200 Subject: [PATCH] fix: don't crash on tray.setContextMenu(null) (#14322) --- atom/browser/api/atom_api_tray.cc | 2 +- atom/browser/ui/tray_icon_cocoa.mm | 15 +++++++++++---- docs/api/tray.md | 2 +- spec/api-tray-spec.js | 25 +++++++++++++++++++++++++ 4 files changed, 38 insertions(+), 6 deletions(-) create mode 100644 spec/api-tray-spec.js diff --git a/atom/browser/api/atom_api_tray.cc b/atom/browser/api/atom_api_tray.cc index 552563bacfcd..a750a575316e 100644 --- a/atom/browser/api/atom_api_tray.cc +++ b/atom/browser/api/atom_api_tray.cc @@ -210,7 +210,7 @@ void Tray::PopUpContextMenu(mate::Arguments* args) { void Tray::SetContextMenu(v8::Isolate* isolate, mate::Handle menu) { menu_.Reset(isolate, menu.ToV8()); - tray_icon_->SetContextMenu(menu->model()); + tray_icon_->SetContextMenu(menu.IsEmpty() ? nullptr : menu->model()); } gfx::Rect Tray::GetBounds() { diff --git a/atom/browser/ui/tray_icon_cocoa.mm b/atom/browser/ui/tray_icon_cocoa.mm index 2c434d7a9e31..d1f0b00a7894 100644 --- a/atom/browser/ui/tray_icon_cocoa.mm +++ b/atom/browser/ui/tray_icon_cocoa.mm @@ -474,11 +474,18 @@ void TrayIconCocoa::SetContextMenu(AtomMenuModel* menu_model) { // Substribe to MenuClosed event. if (menu_model_) menu_model_->RemoveObserver(this); - menu_model->AddObserver(this); - // Create native menu. - menu_.reset([[AtomMenuController alloc] initWithModel:menu_model - useDefaultAccelerator:NO]); + menu_model_ = menu_model; + + if (menu_model) { + menu_model->AddObserver(this); + // Create native menu. + menu_.reset([[AtomMenuController alloc] initWithModel:menu_model + useDefaultAccelerator:NO]); + } else { + menu_.reset(); + } + [status_item_view_ setMenuController:menu_.get()]; } diff --git a/docs/api/tray.md b/docs/api/tray.md index c60f7ba4c9e9..552511e3b3d3 100644 --- a/docs/api/tray.md +++ b/docs/api/tray.md @@ -275,7 +275,7 @@ The `position` is only available on Windows, and it is (0, 0) by default. #### `tray.setContextMenu(menu)` -* `menu` Menu +* `menu` Menu | null Sets the context menu for this icon. diff --git a/spec/api-tray-spec.js b/spec/api-tray-spec.js new file mode 100644 index 000000000000..0e54479c1481 --- /dev/null +++ b/spec/api-tray-spec.js @@ -0,0 +1,25 @@ +const {remote} = require('electron') +const {Menu, Tray, nativeImage} = remote + +describe('tray module', () => { + describe('tray.setContextMenu', () => { + let tray + + beforeEach(() => { + tray = new Tray(nativeImage.createEmpty()) + }) + + afterEach(() => { + tray.destroy() + tray = null + }) + + it('accepts menu instance', () => { + tray.setContextMenu(new Menu()) + }) + + it('accepts null', () => { + tray.setContextMenu(null) + }) + }) +})