Merge pull request #11385 from electron/async-menu

fix: Fix `menu.popup()` bugs

lib/browser/api/menu.js

@@ -45,30 +45,49 @@ Menu.prototype._init = function () {
 
 Menu.prototype.popup = function (window, x, y, positioningItem) {
   let [newX, newY, newPosition, newWindow] = [x, y, positioningItem, window]
+  let opts
 
   // menu.popup(x, y, positioningItem)
-  if (!window) {
+  if (window != null && !(window instanceof BrowserWindow)) {
-    // shift over values
-    if (typeof window !== 'object' || window.constructor !== BrowserWindow) {
     [newPosition, newY, newX, newWindow] = [y, x, window, null]
   }
+
+  // menu.popup({})
+  if (window != null && window.constructor === Object) {
+    opts = window
+  // menu.popup(window, {})
+  } else if (x && typeof x === 'object') {
+    opts = x
   }
 
-  // menu.popup(window, {})
+  if (opts) {
-  if (x && typeof x === 'object') {
-    const opts = x
     newX = opts.x
     newY = opts.y
     newPosition = opts.positioningItem
   }
 
   // set defaults
-  if (typeof x !== 'number') newX = -1
+  if (typeof newX !== 'number') newX = -1
-  if (typeof y !== 'number') newY = -1
+  if (typeof newY !== 'number') newY = -1
-  if (typeof positioningItem !== 'number') newPosition = -1
+  if (typeof newPosition !== 'number') newPosition = -1
-  if (!window) newWindow = BrowserWindow.getFocusedWindow()
+  if (!newWindow || (newWindow && newWindow.constructor !== BrowserWindow)) {
+    newWindow = BrowserWindow.getFocusedWindow()
+
+    // No window focused?
+    if (!newWindow) {
+      const browserWindows = BrowserWindow.getAllWindows()
+
+      if (browserWindows && browserWindows.length > 0) {
+        newWindow = browserWindows[0]
+      } else {
+        throw new Error(`Cannot open Menu without a BrowserWindow present`)
+      }
+    }
+  }
 
   this.popupAt(newWindow, newX, newY, newPosition)
+
+  return { browserWindow: newWindow, x: newX, y: newY, position: newPosition }
 }
 
 Menu.prototype.closePopup = function (window) {

spec/api-menu-spec.js

@@ -283,7 +283,52 @@ describe('Menu module', () => {
     })
 
     it('returns immediately', () => {
-      menu.popup(w, {x: 100, y: 100, async: true})
+      const { browserWindow, x, y } = menu.popup(w, {x: 100, y: 101})
+
+      assert.equal(browserWindow, w)
+      assert.equal(x, 100)
+      assert.equal(y, 101)
+
+      menu.closePopup(w)
+    })
+
+    it('works without a given BrowserWindow and options', () => {
+      const { browserWindow, x, y } = menu.popup({x: 100, y: 101})
+
+      assert.equal(browserWindow.constructor.name, 'BrowserWindow')
+      assert.equal(x, 100)
+      assert.equal(y, 101)
+
+      menu.closePopup()
+    })
+
+    it('works without a given BrowserWindow', () => {
+      const { browserWindow, x, y } = menu.popup(100, 101)
+
+      assert.equal(browserWindow.constructor.name, 'BrowserWindow')
+      assert.equal(x, 100)
+      assert.equal(y, 101)
+
+      menu.closePopup()
+    })
+
+    it('works without a given BrowserWindow and 0 options', () => {
+      const { browserWindow, x, y } = menu.popup(0, 1)
+
+      assert.equal(browserWindow.constructor.name, 'BrowserWindow')
+      assert.equal(x, 0)
+      assert.equal(y, 1)
+
+      menu.closePopup()
+    })
+
+    it('works with a given BrowserWindow and no options', () => {
+      const { browserWindow, x, y } = menu.popup(w, 100, 101)
+
+      assert.equal(browserWindow, w)
+      assert.equal(x, 100)
+      assert.equal(y, 101)
+
       menu.closePopup(w)
     })
   })