Merge pull request #13933 from ryanml/file-share-menu-fix

Ensuring File > Share context menu items are disabled when no windows are open

app/browser/menu.js

@@ -39,6 +39,8 @@ const bookmarkUtil = require('../common/lib/bookmarkUtil')
 const isDarwin = platformUtil.isDarwin()
 const isLinux = platformUtil.isLinux()
 const isWindows = platformUtil.isWindows()
+const {templateUrls} = require('./share')
+const {getAllRendererWindows} = require('./windows')
 
 let appMenu = null
 let closedFrames = new Immutable.OrderedMap()
@@ -596,21 +598,29 @@ const createMenu = (state) => {
   }
 }
 
-const setMenuItemChecked = (state, label, checked) => {
-  // Update electron menu (Mac / Linux)
+const setMenuItemAttribute = (state, label, key, value) => {
   const systemMenuItem = menuUtil.getMenuItem(appMenu, label)
-  systemMenuItem.checked = checked
+  systemMenuItem[key] = value
 
   // Update in-memory menu template (Windows)
   if (isWindows) {
     const oldTemplate = state.getIn(['menu', 'template'])
-    const newTemplate = menuUtil.setTemplateItemChecked(oldTemplate, label, checked)
+    const newTemplate = menuUtil.setTemplateItemAttribute(oldTemplate, label, key, value)
     if (newTemplate) {
       appActions.setMenubarTemplate(newTemplate)
     }
   }
 }
 
+const updateShareMenuItems = (state, enabled) => {
+  for (let key of Object.keys(templateUrls)) {
+    const siteName = menuUtil.extractSiteName(key)
+    const l10nId = key === 'email' ? 'emailPageLink' : 'sharePageLink'
+    const label = locale.translation(l10nId, {siteName: siteName})
+    setMenuItemAttribute(state, label, 'enabled', enabled)
+  }
+}
+
 const doAction = (state, action) => {
   switch (action.actionType) {
     case appConstants.APP_SET_STATE:
@@ -622,17 +632,28 @@ const doAction = (state, action) => {
         const frame = frameStateUtil.getFrameByTabId(state, action.tabId)
         if (frame) {
           currentLocation = frame.location
-          setMenuItemChecked(state, locale.translation('bookmarkPage'), isCurrentLocationBookmarked(state))
+          setMenuItemAttribute(state, locale.translation('bookmarkPage'), 'checked', isCurrentLocationBookmarked(state))
+        }
+        break
+      }
+    case appConstants.APP_WINDOW_CLOSED:
+    case appConstants.APP_WINDOW_CREATED:
+      {
+        const windowCount = getAllRendererWindows().length
+        if (action.actionType === appConstants.APP_WINDOW_CLOSED && windowCount === 0) {
+          updateShareMenuItems(state, false)
+        } else if (action.actionType === appConstants.APP_WINDOW_CREATED && windowCount === 1) {
+          updateShareMenuItems(state, true)
         }
         break
       }
     case appConstants.APP_CHANGE_SETTING:
       if (action.key === settings.SHOW_BOOKMARKS_TOOLBAR) {
         // Update the checkbox next to "Bookmarks Toolbar" (Bookmarks menu)
-        setMenuItemChecked(state, locale.translation('bookmarksToolbar'), action.value)
+        setMenuItemAttribute(state, locale.translation('bookmarksToolbar'), 'checked', action.value)
       }
       if (action.key === settings.DEBUG_ALLOW_MANUAL_TAB_DISCARD) {
-        setMenuItemChecked(state, 'Allow manual tab discarding', action.value)
+        setMenuItemAttribute(state, 'Allow manual tab discarding', 'checked', action.value)
       }
       break
     case windowConstants.WINDOW_UNDO_CLOSED_FRAME:

app/browser/share.js

@@ -15,7 +15,6 @@ const templateUrls = {
   googlePlus: 'https://plus.google.com/share?url={url}',
   linkedIn: 'https://www.linkedin.com/shareArticle?url={url}&title={title}',
   buffer: 'https://buffer.com/add?text={title}&url={url}',
-  digg: 'https://digg.com/submit?url={url}&title={title}',
   reddit: 'https://reddit.com/submit?url={url}&title={title}'
 }
 

app/common/commonMenu.js

@@ -13,6 +13,7 @@ const getSetting = require('../../js/settings').getSetting
 const communityURL = 'https://community.brave.com/'
 const isDarwin = process.platform === 'darwin'
 const electron = require('electron')
+const menuUtil = require('./lib/menuUtil')
 
 const ensureAtLeastOneWindow = (frameOpts) => {
   // Handle no new tab requested, but need a window
@@ -136,7 +137,7 @@ module.exports.printMenuItem = () => {
 }
 
 module.exports.simpleShareActiveTabMenuItem = (l10nId, type, accelerator) => {
-  const siteName = type.charAt(0).toUpperCase() + type.slice(1)
+  const siteName = menuUtil.extractSiteName(type)
 
   return {
     label: locale.translation(l10nId, {siteName: siteName}),

app/common/lib/menuUtil.js

@@ -67,16 +67,20 @@ const getTemplateItem = (template, label) => {
   return null
 }
 
+module.exports.extractSiteName = (type) => {
+  return type.charAt(0).toUpperCase() + type.slice(1)
+}
+
 /**
- * Search a menu template and update the checked status
+ * Searches a menu template and updates a passed item key
  *
  * @return the new template OR null if no change was made (no update needed)
  */
-module.exports.setTemplateItemChecked = (template, label, checked) => {
+module.exports.setTemplateItemAttribute = (template, label, key, value) => {
   const menu = template.toJS()
   const menuItem = getTemplateItem(menu, label)
-  if (menuItem.checked !== checked) {
-    menuItem.checked = checked
+  if (menuItem[key] !== value) {
+    menuItem[key] = value
     return makeImmutable(menu)
   }
   return null

test/unit/app/common/lib/menuUtilTest.js

@@ -86,7 +86,7 @@ describe('menuUtil tests', function () {
     })
   })
 
-  describe('setTemplateItemChecked', function () {
+  describe('setTemplateItemAttribute', function () {
     const defaultTemplate = Immutable.fromJS([
       {
         'label': 'Bookmarks',
@@ -113,11 +113,11 @@ describe('menuUtil tests', function () {
           ]
         }
       ])
-      const newTemplate = menuUtil.setTemplateItemChecked(defaultTemplate, 'Bookmarks Toolbar', true)
+      const newTemplate = menuUtil.setTemplateItemAttribute(defaultTemplate, 'Bookmarks Toolbar', 'checked', true)
       assert.deepEqual(newTemplate.toJS(), expectedTemplate.toJS())
     })
     it('returns null when no change is made', function () {
-      const newTemplate = menuUtil.setTemplateItemChecked(defaultTemplate, 'Bookmarks Toolbar', false)
+      const newTemplate = menuUtil.setTemplateItemAttribute(defaultTemplate, 'Bookmarks Toolbar', 'checked', false)
       assert.equal(newTemplate, null)
     })
   })