From bcc372568f33bd4dbc43c770263169bdd9d96147 Mon Sep 17 00:00:00 2001
From: Kevin Sawicki <kevinsawicki@gmail.com>
Date: Mon, 8 Aug 2016 10:09:45 -0700
Subject: [PATCH] Add zoom menu item roles

---
 atom/browser/ui/cocoa/atom_menu_controller.mm |  6 +--
 default_app/main.js                           | 28 ++-----------
 docs/api/menu-item.md                         |  3 ++
 lib/browser/api/menu-item-roles.js            | 41 +++++++++++++++++--
 4 files changed, 47 insertions(+), 31 deletions(-)

diff --git a/atom/browser/ui/cocoa/atom_menu_controller.mm b/atom/browser/ui/cocoa/atom_menu_controller.mm
index 525fde28b193..365317f653c3 100644
--- a/atom/browser/ui/cocoa/atom_menu_controller.mm
+++ b/atom/browser/ui/cocoa/atom_menu_controller.mm
@@ -181,11 +181,11 @@ Role kRolesMap[] = {
 
     // Set menu item's role.
     base::string16 role = model->GetRoleAt(index);
-    if (role.empty()) {
-      [item setTarget:self];
-    } else {
+    [item setTarget:self];
+    if (!role.empty()) {
       for (const Role& pair : kRolesMap) {
         if (role == base::ASCIIToUTF16(pair.role)) {
+          [item setTarget:nil];
           [item setAction:pair.selector];
           break;
         }
diff --git a/default_app/main.js b/default_app/main.js
index ead6adff6775..a3b8c9bc77e4 100644
--- a/default_app/main.js
+++ b/default_app/main.js
@@ -101,35 +101,13 @@ app.once('ready', () => {
           type: 'separator'
         },
         {
-          label: 'Actual Size',
-          accelerator: 'CmdOrCtrl+0',
-          click (item, focusedWindow) {
-            if (focusedWindow) focusedWindow.webContents.setZoomLevel(0)
-          }
+          role: 'resetzoom'
         },
         {
-          label: 'Zoom In',
-          accelerator: 'CmdOrCtrl+Plus',
-          click (item, focusedWindow) {
-            if (focusedWindow) {
-              const {webContents} = focusedWindow
-              webContents.getZoomLevel((zoomLevel) => {
-                webContents.setZoomLevel(zoomLevel + 0.5)
-              })
-            }
-          }
+          role: 'zoomin'
         },
         {
-          label: 'Zoom Out',
-          accelerator: 'CmdOrCtrl+-',
-          click (item, focusedWindow) {
-            if (focusedWindow) {
-              const {webContents} = focusedWindow
-              webContents.getZoomLevel((zoomLevel) => {
-                webContents.setZoomLevel(zoomLevel - 0.5)
-              })
-            }
-          }
+          role: 'zoomout'
         },
         {
           type: 'separator'
diff --git a/docs/api/menu-item.md b/docs/api/menu-item.md
index a40cad49f434..836a91632357 100644
--- a/docs/api/menu-item.md
+++ b/docs/api/menu-item.md
@@ -56,6 +56,9 @@ The `role` property can have following values:
 * `close` - Close current window
 * `quit`- Quit the application
 * `togglefullscreen`- Toggle full screen mode on the current window
+* `resetzoom` - Reset the focused page's zoom level to the original size
+* `zoomin` - Zoom in the focused page by 10%
+* `zoomout` - Zoom out the focused page by 10%
 
 On macOS `role` can also have following additional values:
 
diff --git a/lib/browser/api/menu-item-roles.js b/lib/browser/api/menu-item-roles.js
index 526b60fcc15d..e9a63b897623 100644
--- a/lib/browser/api/menu-item-roles.js
+++ b/lib/browser/api/menu-item-roles.js
@@ -72,6 +72,13 @@ const roles = {
     accelerator: 'Shift+CommandOrControl+Z',
     webContentsMethod: 'redo'
   },
+  resetzoom: {
+    label: 'Actual Size',
+    accelerator: 'CommandOrControl+0',
+    webContentsMethod: (webContents) => {
+      webContents.setZoomLevel(0)
+    }
+  },
   selectall: {
     label: 'Select All',
     accelerator: 'CommandOrControl+A',
@@ -106,9 +113,34 @@ const roles = {
   },
   zoom: {
     label: 'Zoom'
+  },
+  zoomin: {
+    label: 'Zoom In',
+    accelerator: 'CommandOrControl+Plus',
+    webContentsMethod: (webContents) => {
+      webContents.getZoomLevel((zoomLevel) => {
+        webContents.setZoomLevel(zoomLevel + 0.5)
+      })
+    }
+  },
+  zoomout: {
+    label: 'Zoom Out',
+    accelerator: 'CommandOrControl+-',
+    webContentsMethod: (webContents) => {
+      webContents.getZoomLevel((zoomLevel) => {
+        webContents.setZoomLevel(zoomLevel - 0.5)
+      })
+    }
   }
 }
 
+const canExecuteRole = (role) => {
+  if (!roles.hasOwnProperty(role)) return false
+  if (process.platform !== 'darwin') return true
+  // macOS handles all roles natively except the ones listed below
+  return ['zoomin', 'zoomout', 'resetzoom'].includes(role)
+}
+
 exports.getDefaultLabel = (role) => {
   if (roles.hasOwnProperty(role)) {
     return roles[role].label
@@ -122,8 +154,7 @@ exports.getDefaultAccelerator = (role) => {
 }
 
 exports.execute = (role, focusedWindow, focusedWebContents) => {
-  if (!roles.hasOwnProperty(role)) return false
-  if (process.platform === 'darwin') return false
+  if (!canExecuteRole(role)) return false
 
   const {appMethod, webContentsMethod, windowMethod} = roles[role]
 
@@ -142,7 +173,11 @@ exports.execute = (role, focusedWindow, focusedWebContents) => {
   }
 
   if (webContentsMethod && focusedWebContents != null) {
-    focusedWebContents[webContentsMethod]()
+    if (typeof webContentsMethod === 'function') {
+      webContentsMethod(focusedWebContents)
+    } else {
+      focusedWebContents[webContentsMethod]()
+    }
     return true
   }