From 66f2fb2fe4ae6c0b39991db4078097067a3ae048 Mon Sep 17 00:00:00 2001
From: Kevin Sawicki <kevinsawicki@gmail.com>
Date: Wed, 22 Jun 2016 13:48:26 -0700
Subject: [PATCH] Add execute helper to roles file

---
 lib/browser/api/menu-item-roles.js | 57 +++++++++++++++++++++++-------
 lib/browser/api/menu-item.js       | 49 +++----------------------
 2 files changed, 50 insertions(+), 56 deletions(-)

diff --git a/lib/browser/api/menu-item-roles.js b/lib/browser/api/menu-item-roles.js
index 6d6b0371e87c..947bf4d28c1f 100644
--- a/lib/browser/api/menu-item-roles.js
+++ b/lib/browser/api/menu-item-roles.js
@@ -8,22 +8,22 @@ const roles = {
   close: {
     label: 'Close',
     accelerator: 'CmdOrCtrl+W',
-    method: 'close'
+    windowMethod: 'close'
   },
   copy: {
     label: 'Copy',
     accelerator: 'CmdOrCtrl+C',
-    method: 'copy'
+    webContentsMethod: 'copy'
   },
   cut: {
     label: 'Cut',
     accelerator: 'CmdOrCtrl+X',
-    method: 'cut'
+    webContentsMethod: 'cut'
   },
   delete: {
     label: 'Delete',
     accelerator: 'Delete',
-    method: 'delete'
+    webContentsMethod: 'delete'
   },
   front: {
     label: 'Bring All to Front'
@@ -45,17 +45,17 @@ const roles = {
   minimize: {
     label: 'Minimize',
     accelerator: 'CmdOrCtrl+M',
-    method: 'minimize'
+    windowMethod: 'minimize'
   },
   paste: {
     label: 'Paste',
     accelerator: 'CmdOrCtrl+V',
-    method: 'paste'
+    webContentsMethod: 'paste'
   },
   pasteandmatchstyle: {
     label: 'Paste and Match Style',
     accelerator: 'Shift+Command+V',
-    method: 'pasteAndMatchStyle'
+    webContentsMethod: 'pasteAndMatchStyle'
   },
   quit: {
     get label () {
@@ -63,17 +63,17 @@ const roles = {
       return process.platform === 'win32' ? 'Exit' : `Quit ${app.getName()}`
     },
     accelerator: process.platform === 'win32' ? null : 'Command+Q',
-    method: 'quit'
+    appMethod: 'quit'
   },
   redo: {
     label: 'Redo',
     accelerator: 'Shift+CmdOrCtrl+Z',
-    method: 'redo'
+    webContentsMethod: 'redo'
   },
   selectall: {
     label: 'Select All',
     accelerator: 'CmdOrCtrl+A',
-    method: 'selectAll'
+    webContentsMethod: 'selectAll'
   },
   services: {
     label: 'Services'
@@ -81,12 +81,14 @@ const roles = {
   togglefullscreen: {
     label: 'Toggle Full Screen',
     accelerator: process.platform === 'darwin' ? 'Ctrl+Command+F' : 'F11',
-    method: 'toggleFullScreen'
+    windowMethod: function (window) {
+      window.setFullScreen(!window.isFullScreen())
+    }
   },
   undo: {
     label: 'Undo',
     accelerator: 'CmdOrCtrl+Z',
-    method: 'undo'
+    webContentsMethod: 'undo'
   },
   unhide: {
     label: 'Show All'
@@ -110,3 +112,34 @@ exports.getDefaultLabel = function (role) {
 exports.getDefaultAccelerator = function (role) {
   if (roles.hasOwnProperty(role)) return roles[role].accelerator
 }
+
+exports.execute = function (role, focusedWindow) {
+  if (!roles.hasOwnProperty(role)) return false
+  if (process.platform === 'darwin') return false
+
+  const {appMethod, webContentsMethod, windowMethod} = roles[role]
+
+  if (appMethod) {
+    app[appMethod]()
+    return true
+  }
+
+  if (focusedWindow != null) {
+    if (windowMethod) {
+      if (typeof windowMethod === 'function') {
+        windowMethod(focusedWindow)
+      } else {
+        focusedWindow[windowMethod]()
+      }
+      return true
+    } else if (webContentsMethod) {
+      const {webContents} = focusedWindow
+      if (webContents) {
+        webContents[webContentsMethod]()
+      }
+      return true
+    }
+  }
+
+  return false
+}
diff --git a/lib/browser/api/menu-item.js b/lib/browser/api/menu-item.js
index ba980960b559..d2d8e651980c 100644
--- a/lib/browser/api/menu-item.js
+++ b/lib/browser/api/menu-item.js
@@ -4,35 +4,6 @@ const roles = require('./menu-item-roles')
 
 let nextCommandId = 0
 
-// Maps role to methods of webContents
-const rolesMap = {
-  undo: 'undo',
-  redo: 'redo',
-  cut: 'cut',
-  copy: 'copy',
-  paste: 'paste',
-  pasteandmatchstyle: 'pasteAndMatchStyle',
-  selectall: 'selectAll',
-  minimize: 'minimize',
-  close: 'close',
-  delete: 'delete',
-  quit: 'quit',
-  togglefullscreen: 'toggleFullScreen'
-}
-
-// Maps methods that should be called directly on the BrowserWindow instance
-const methodInBrowserWindow = {
-  minimize: true,
-  close: true,
-  toggleFullScreen: function (window) {
-    window.setFullScreen(!window.isFullScreen())
-  }
-}
-
-const methodInApp = {
-  quit: true
-}
-
 const MenuItem = function (options) {
   const {app, Menu} = require('electron')
 
@@ -83,22 +54,12 @@ const MenuItem = function (options) {
       this.checked = !this.checked
     }
 
-    if (this.role && rolesMap[this.role] && process.platform !== 'darwin' && focusedWindow != null) {
-      const methodName = rolesMap[this.role]
-      if (methodInApp[methodName]) {
-        return app[methodName]()
-      } else if (typeof methodInBrowserWindow[methodName] === 'function') {
-        return methodInBrowserWindow[methodName](focusedWindow)
-      } else if (methodInBrowserWindow[methodName]) {
-        return focusedWindow[methodName]()
-      } else {
-        const {webContents} = focusedWindow
-        return webContents != null ? webContents[methodName]() : void 0
+    if (!roles.execute(this.role)) {
+      if (typeof click === 'function') {
+        click(this, focusedWindow, event)
+      } else if (typeof this.selector === 'string' && process.platform === 'darwin') {
+        Menu.sendActionToFirstResponder(this.selector)
       }
-    } else if (typeof click === 'function') {
-      return click(this, focusedWindow, event)
-    } else if (typeof this.selector === 'string' && process.platform === 'darwin') {
-      return Menu.sendActionToFirstResponder(this.selector)
     }
   }
 }