Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Commit

Permalink
Merge pull request #9965 from brave/issue-9964
Browse files Browse the repository at this point in the history 
fix activateIfOpen
  • Loading branch information
@bridiver
bridiver authored Jul 18, 2017
2 parents 5e37a1d + 008fdd4 commit 3b5de18
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 144 deletions.
10 changes: 9 additions & 1 deletion app/browser/reducers/shareReducer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 9 additions & 4 deletions app/browser/reducers/tabsReducer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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'))
}
Expand Down
176 changes: 45 additions & 131 deletions app/common/commonMenu.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

/**
Expand Down Expand Up @@ -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 })
}
}
}
Expand All @@ -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
})
}
}
}
Expand All @@ -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
})
}
})

Expand Down Expand Up @@ -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)
}
}
}
Expand Down Expand Up @@ -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'
})
}
}
}
Expand All @@ -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'
})
}
}
}
Expand All @@ -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'
})
}
}
}
Expand All @@ -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'
})
}
}
}
Expand All @@ -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'
})
}
}
}
Expand All @@ -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'
})
}
}
}
Expand Down Expand Up @@ -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)
}
}
}
Expand Down Expand Up @@ -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'
})
}
}
Expand All @@ -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'
})
}
}
}
Expand All @@ -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'
})
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion app/common/state/tabState.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
3 changes: 0 additions & 3 deletions app/ledger.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
Expand All @@ -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
})
Expand All @@ -592,7 +590,6 @@ if (ipc) {
appActions.hideNotification(message)
if (buttonIndex === 1 && win) {
appActions.createTabRequested({
activateIfOpen: true,
url: 'about:preferences#payments',
windowId: win.id
})
Expand Down
1 change: 0 additions & 1 deletion app/renderer/components/download/downloadsBar.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ class DownloadsBar extends React.Component {

onShowDownloads () {
appActions.createTabRequested({
activateIfOpen: true,
url: 'about:downloads'
})
windowActions.setDownloadsToolbarVisible(false)
Expand Down
3 changes: 1 addition & 2 deletions js/actions/appActions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,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
})
},
Expand Down
2 changes: 1 addition & 1 deletion test/unit/app/browser/reducers/shareReducerTest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 3b5de18

Please sign in to comment.