fix activateIfOpen

fix #9964
brave · Jul 11, 2017 · 008fdd4 · 008fdd4
1 parent 2bd7726
commit 008fdd4
Show file tree

Hide file tree

Showing 8 changed files with 66 additions and 144 deletions.
diff --git a/app/browser/reducers/shareReducer.js b/app/browser/reducers/shareReducer.js
@@ -7,12 +7,20 @@
 const appConstants = require('../../../js/constants/appConstants')
 const {makeImmutable} = require('../../common/state/immutableUtil')
 const {simpleShareActiveTab} = require('../share')
+const BrowserWindow = require('electron').BrowserWindow
 
 const shareReducer = (state, action, immutableAction) => {
   action = immutableAction || makeImmutable(action)
   switch (action.get('actionType')) {
     case appConstants.APP_SIMPLE_SHARE_ACTIVE_TAB_REQUESTED:
-      state = simpleShareActiveTab(state, action.get('windowId'), action.get('shareType'))
+      let windowId = action.get('senderWindowId')
+      if (windowId == null) {
+        if (BrowserWindow.getActiveWindow()) {
+          windowId = BrowserWindow.getActiveWindow().id
+        }
+      }
+
+      state = simpleShareActiveTab(state, windowId, action.get('shareType'))
       break
   }
   return state

diff --git a/app/browser/reducers/tabsReducer.js b/app/browser/reducers/tabsReducer.js
@@ -26,6 +26,7 @@ const getSetting = require('../../../js/settings').getSetting
 const settings = require('../../../js/constants/settings')
 const {tabCloseAction} = require('../../common/constants/settingsEnums')
 const {frameOptsFromFrame} = require('../../../js/state/frameStateUtil')
+const {isSourceAboutUrl, isTargetAboutUrl, isIntermediateAboutPage} = require('../../../js/lib/appUrlUtil')
 
 const updateActiveTab = (state, closeTabId) => {
   if (!tabState.getByTabId(state, closeTabId)) {
@@ -136,16 +137,20 @@ const tabsReducer = (state, action, immutableAction) => {
       break
     }
     case appConstants.APP_CREATE_TAB_REQUESTED:
-      if (!action.getIn(['createProperties', 'windowId'])) {
+      if (action.getIn(['createProperties', 'windowId']) == null) {
         const senderWindowId = action.getIn(['senderWindowId'])
-        if (senderWindowId) {
+        if (senderWindowId != null) {
           action = action.setIn(['createProperties', 'windowId'], senderWindowId)
+        } else if (BrowserWindow.getActiveWindow()) {
+          action = action.setIn(['createProperties', 'windowId'], BrowserWindow.getActiveWindow().id)
         }
       }
 
+      const url = action.getIn(['createProperties', 'url'])
       setImmediate(() => {
-        if (action.get('activateIfOpen')) {
-          tabs.maybeCreateTab(state, action, action.get('createProperties'))
+        if (action.get('activateIfOpen') ||
+            ((isSourceAboutUrl(url) || isTargetAboutUrl(url)) && !isIntermediateAboutPage(url))) {
+          tabs.maybeCreateTab(state, action.get('createProperties'))
         } else {
           tabs.create(action.get('createProperties'), null, action.get('isRestore'))
         }

diff --git a/app/common/commonMenu.js b/app/common/commonMenu.js
@@ -22,22 +22,14 @@ if (process.type === 'browser') {
   BrowserWindow = electron.remote.BrowserWindow
 }
 
-const ensureAtLeastOneWindow = (frameOpts) => {
-  if (BrowserWindow.getAllWindows().length === 0) {
-    appActions.newWindow(frameOpts)
-    return false
-  }
-  return true
-}
-
-const getCurrentWindowId = () => {
+const ensureAtLeastOneWindow = (frameOpts = {}) => {
   if (process.type === 'browser') {
-    const activeWindow = BrowserWindow.getActiveWindow()
-    return activeWindow && activeWindow.id
+    if (BrowserWindow.getAllWindows().length === 0) {
+      appActions.newWindow(Immutable.fromJS({location: frameOpts.url}))
+      return
+    }
   }
-
-  const currentWindow = require('../renderer/currentWindow')
-  return currentWindow.getCurrentWindowId()
+  appActions.createTabRequested(frameOpts)
 }
 
 /**
@@ -76,12 +68,7 @@ module.exports.newTabMenuItem = (openerTabId) => {
     label: locale.translation('newTab'),
     accelerator: 'CmdOrCtrl+T',
     click: function (item, focusedWindow) {
-      if (ensureAtLeastOneWindow(Immutable.fromJS({}))) {
-        appActions.createTabRequested({
-          windowId: getCurrentWindowId(),
-          openerTabId
-        })
-      }
+      ensureAtLeastOneWindow({ openerTabId })
     }
   }
 }
@@ -91,13 +78,10 @@ module.exports.newPrivateTabMenuItem = () => {
     label: locale.translation('newPrivateTab'),
     accelerator: 'Shift+CmdOrCtrl+P',
     click: function (item, focusedWindow) {
-      if (ensureAtLeastOneWindow(Immutable.fromJS({ isPrivate: true }))) {
-        appActions.createTabRequested({
-          url: 'about:newtab',
-          windowId: getCurrentWindowId(),
-          isPrivate: true
-        })
-      }
+      ensureAtLeastOneWindow({
+        url: 'about:newtab',
+        isPrivate: true
+      })
     }
   }
 }
@@ -106,11 +90,9 @@ module.exports.newPartitionedTabMenuItem = () => {
   const newPartitionedMenuItem = (partitionNumber) => ({
     label: `${locale.translation('newSessionTab')} ${partitionNumber}`,
     click: (item, focusedWindow) => {
-      if (ensureAtLeastOneWindow(Immutable.fromJS({ partitionNumber }))) {
-        appActions.createTabRequested({
-          partitionNumber
-        })
-      }
+      ensureAtLeastOneWindow({
+        partitionNumber
+      })
     }
   })
 
@@ -160,7 +142,7 @@ module.exports.simpleShareActiveTabMenuItem = (l10nId, type, accelerator) => {
     label: locale.translation(l10nId),
     accelerator,
     click: function (item, focusedWindow) {
-      appActions.simpleShareActiveTabRequested(getCurrentWindowId(), type)
+      appActions.simpleShareActiveTabRequested(type)
     }
   }
 }
@@ -194,17 +176,9 @@ module.exports.preferencesMenuItem = () => {
     label: locale.translation(isDarwin ? 'preferences' : 'settings'),
     accelerator: 'CmdOrCtrl+,',
     click: (item, focusedWindow) => {
-      if (BrowserWindow.getAllWindows().length === 0) {
-        appActions.newWindow(Immutable.fromJS({
-          location: 'about:preferences'
-        }))
-      } else {
-        appActions.createTabRequested({
-          activateIfOpen: true,
-          url: 'about:preferences',
-          windowId: getCurrentWindowId()
-        })
-      }
+      ensureAtLeastOneWindow({
+        url: 'about:preferences'
+      })
     }
   }
 }
@@ -214,17 +188,9 @@ module.exports.bookmarksManagerMenuItem = () => {
     label: locale.translation('bookmarksManager'),
     accelerator: isDarwin ? 'CmdOrCtrl+Alt+B' : 'Ctrl+Shift+O',
     click: (item, focusedWindow) => {
-      if (BrowserWindow.getAllWindows().length === 0) {
-        appActions.newWindow(Immutable.fromJS({
-          location: 'about:bookmarks'
-        }))
-      } else {
-        appActions.createTabRequested({
-          activateIfOpen: true,
-          url: 'about:bookmarks',
-          windowId: getCurrentWindowId()
-        })
-      }
+      ensureAtLeastOneWindow({
+        url: 'about:bookmarks'
+      })
     }
   }
 }
@@ -234,17 +200,9 @@ module.exports.historyMenuItem = () => {
     label: locale.translation('showAllHistory'),
     accelerator: 'CmdOrCtrl+Y',
     click: function (item, focusedWindow) {
-      if (BrowserWindow.getAllWindows().length === 0) {
-        appActions.newWindow(Immutable.fromJS({
-          location: 'about:history'
-        }))
-      } else {
-        appActions.createTabRequested({
-          activateIfOpen: true,
-          url: 'about:history',
-          windowId: getCurrentWindowId()
-        })
-      }
+      ensureAtLeastOneWindow({
+        url: 'about:history'
+      })
     }
   }
 }
@@ -254,18 +212,10 @@ module.exports.downloadsMenuItem = () => {
     label: locale.translation('downloadsManager'),
     accelerator: isDarwin ? 'CmdOrCtrl+Shift+J' : 'Ctrl+J',
     click: (item, focusedWindow) => {
-      if (BrowserWindow.getAllWindows().length === 0) {
-        appActions.newWindow(Immutable.fromJS({
-          location: 'about:downloads'
-        }))
-      } else {
-        module.exports.sendToFocusedWindow(focusedWindow, [messages.HIDE_DOWNLOADS_TOOLBAR])
-        appActions.createTabRequested({
-          activateIfOpen: true,
-          url: 'about:downloads',
-          windowId: getCurrentWindowId()
-        })
-      }
+      module.exports.sendToFocusedWindow(focusedWindow, [messages.HIDE_DOWNLOADS_TOOLBAR])
+      ensureAtLeastOneWindow({
+        url: 'about:downloads'
+      })
     }
   }
 }
@@ -274,17 +224,9 @@ module.exports.extensionsMenuItem = () => {
   return {
     label: locale.translation('extensionsManager'),
     click: (item, focusedWindow) => {
-      if (BrowserWindow.getAllWindows().length === 0) {
-        appActions.newWindow(Immutable.fromJS({
-          location: 'about:preferences#extensions'
-        }))
-      } else {
-        appActions.createTabRequested({
-          activateIfOpen: true,
-          url: 'about:preferences#extensions',
-          windowId: getCurrentWindowId()
-        })
-      }
+      ensureAtLeastOneWindow({
+        url: 'about:preferences#extensions'
+      })
     }
   }
 }
@@ -293,17 +235,9 @@ module.exports.passwordsMenuItem = () => {
   return {
     label: locale.translation('passwordsManager'),
     click: (item, focusedWindow) => {
-      if (BrowserWindow.getAllWindows().length === 0) {
-        appActions.newWindow(Immutable.fromJS({
-          location: 'about:passwords'
-        }))
-      } else {
-        appActions.createTabRequested({
-          activateIfOpen: true,
-          url: 'about:passwords',
-          windowId: getCurrentWindowId()
-        })
-      }
+      ensureAtLeastOneWindow({
+        url: 'about:passwords'
+      })
     }
   }
 }
@@ -338,11 +272,9 @@ module.exports.submitFeedbackMenuItem = () => {
   return {
     label: locale.translation('submitFeedback'),
     click: function (item, focusedWindow) {
-      appActions.createTabRequested({
-        activateIfOpen: true,
-        url: communityURL,
-        windowId: getCurrentWindowId()
-      })
+      ensureAtLeastOneWindow({
+        url: communityURL
+      }, true)
     }
   }
 }
@@ -374,10 +306,8 @@ module.exports.aboutBraveMenuItem = () => {
   return {
     label: locale.translation('aboutApp'),
     click: (item, focusedWindow) => {
-      appActions.createTabRequested({
-        activateIfOpen: true,
-        url: 'about:brave',
-        windowId: getCurrentWindowId()
+      ensureAtLeastOneWindow({
+        url: 'about:brave'
       })
     }
   }
@@ -396,17 +326,9 @@ module.exports.braveryGlobalMenuItem = () => {
   return {
     label: locale.translation('braveryGlobal'),
     click: (item, focusedWindow) => {
-      if (BrowserWindow.getAllWindows().length === 0) {
-        appActions.newWindow(Immutable.fromJS({
-          location: 'about:preferences#shields'
-        }))
-      } else {
-        appActions.createTabRequested({
-          activateIfOpen: true,
-          url: 'about:preferences#shields',
-          windowId: getCurrentWindowId()
-        })
-      }
+      ensureAtLeastOneWindow({
+        url: 'about:preferences#shields'
+      })
     }
   }
 }
@@ -419,17 +341,9 @@ module.exports.braveryPaymentsMenuItem = () => {
   return {
     label: label,
     click: (item, focusedWindow) => {
-      if (BrowserWindow.getAllWindows().length === 0) {
-        appActions.newWindow(Immutable.fromJS({
-          location: 'about:preferences#payments'
-        }))
-      } else {
-        appActions.createTabRequested({
-          activateIfOpen: true,
-          url: 'about:preferences#payments',
-          windowId: getCurrentWindowId()
-        })
-      }
+      ensureAtLeastOneWindow({
+        url: 'about:preferences#payments'
+      })
     }
   }
 }

diff --git a/app/common/state/tabState.js b/app/common/state/tabState.js
@@ -262,7 +262,7 @@ const tabState = {
     return state.get('tabs').find(
       (tab) => tab.get('windowId') === windowId &&
         tab.get('url') === createProperties.get('url') &&
-        (tab.get('partition') || 'default') === (createProperties.get('partition') || 'default'))
+        (tab.get('partition') || 'persist:default') === (createProperties.get('partition') || 'persist:default'))
   },
 
   getTabsByWindow: (state, windowValue) => {

diff --git a/app/ledger.js b/app/ledger.js
@@ -566,7 +566,6 @@ if (ipc) {
       } else if (buttonIndex === 2 && win) {
         // Add funds: Open payments panel
         appActions.createTabRequested({
-          activateIfOpen: true,
           url: 'about:preferences#payments',
           windowId: win.id
         })
@@ -578,7 +577,6 @@ if (ipc) {
         appActions.changeSetting(settings.PAYMENTS_NOTIFICATIONS, false)
       } else if (buttonIndex === 2 && win) {
         appActions.createTabRequested({
-          activateIfOpen: true,
           url: 'about:preferences#payments',
           windowId: win.id
         })
@@ -592,7 +590,6 @@ if (ipc) {
       appActions.hideNotification(message)
       if (buttonIndex === 1 && win) {
         appActions.createTabRequested({
-          activateIfOpen: true,
           url: 'about:preferences#payments',
           windowId: win.id
         })

diff --git a/app/renderer/components/download/downloadsBar.js b/app/renderer/components/download/downloadsBar.js
@@ -36,7 +36,6 @@ class DownloadsBar extends React.Component {
 
   onShowDownloads () {
     appActions.createTabRequested({
-      activateIfOpen: true,
       url: 'about:downloads'
     })
     windowActions.setDownloadsToolbarVisible(false)

diff --git a/js/actions/appActions.js b/js/actions/appActions.js
@@ -155,10 +155,9 @@ const appActions = {
    * @param {number} windowId - the window ID to use for the active tab
    * @param {string} shareType - The type of share to do, must be one of: "email", "facebook", "pinterest", "twitter", "googlePlus", "linkedIn", "buffer", "reddit", or "digg"
    */
-  simpleShareActiveTabRequested: function (windowId, shareType) {
+  simpleShareActiveTabRequested: function (shareType) {
     dispatch({
       actionType: appConstants.APP_SIMPLE_SHARE_ACTIVE_TAB_REQUESTED,
-      windowId,
       shareType
     })
   },

diff --git a/test/unit/app/browser/reducers/shareReducerTest.js b/test/unit/app/browser/reducers/shareReducerTest.js
@@ -33,7 +33,7 @@ describe('shareReducer', function () {
       this.state = Immutable.Map()
       this.windowId = 2
       this.shareType = 'email'
-      this.newState = shareReducer(this.state, {actionType: appConstants.APP_SIMPLE_SHARE_ACTIVE_TAB_REQUESTED, windowId: this.windowId, shareType: this.shareType})
+      this.newState = shareReducer(this.state, {actionType: appConstants.APP_SIMPLE_SHARE_ACTIVE_TAB_REQUESTED, senderWindowId: this.windowId, shareType: this.shareType})
     })
     it('calls simpleShareActiveTab once with the correct args', function () {
       const callCount = this.shareStub.simpleShareActiveTab.withArgs(this.state, this.windowId, this.shareType).callCount