diff --git a/app/browser/menu.js b/app/browser/menu.js index d22e9fd1c51..1067dae7908 100644 --- a/app/browser/menu.js +++ b/app/browser/menu.js @@ -75,7 +75,7 @@ const createFileSubmenu = () => { label: locale.translation('closeTab'), accelerator: 'CmdOrCtrl+W', click: function (item, focusedWindow) { - appActions.activeWebContentsClosed() + appActions.tabCloseRequested(tabState.TAB_ID_ACTIVE) } }, { // This should be disabled when no windows are active. @@ -629,10 +629,10 @@ const doAction = (action) => { createMenu() }) break - case appConstants.APP_TAB_CLOSED: + case appConstants.APP_TAB_CLOSE_REQUESTED: appDispatcher.waitFor([appStore.dispatchToken], () => { action = makeImmutable(action) - const tab = getByTabId(appStore.getState(), action.getIn(['tabValue', 'tabId'])) + const tab = getByTabId(appStore.getState(), action.get('tabId')) if (tab && !tab.get('incognito') && tab.get('url') !== 'about:newtab') { if (tab.get('frame')) { lastClosedUrl = tab.get('url') diff --git a/app/browser/reducers/sitesReducer.js b/app/browser/reducers/sitesReducer.js index 0cd8736c81d..54149a55032 100644 --- a/app/browser/reducers/sitesReducer.js +++ b/app/browser/reducers/sitesReducer.js @@ -11,7 +11,6 @@ const siteUtil = require('../../../js/state/siteUtil') const syncActions = require('../../../js/actions/syncActions') const syncUtil = require('../../../js/state/syncUtil') const Immutable = require('immutable') -const {makeImmutable} = require('../../common/state/immutableUtil') const settings = require('../../../js/constants/settings') const {getSetting} = require('../../../js/settings') const writeActions = require('../../../js/constants/sync/proto').actions @@ -20,11 +19,10 @@ const syncEnabled = () => { return getSetting(settings.SYNC_ENABLED) === true } -const sitesReducer = (state, action, emitChanges) => { +const sitesReducer = (state, action, immutableAction) => { switch (action.actionType) { case appConstants.APP_ON_CLEAR_BROWSING_DATA: - action = makeImmutable(action) - if (action.getIn(['clearDataDetail', 'browserHistory'])) { + if (immutableAction.getIn(['clearDataDetail', 'browserHistory'])) { state = state.set('sites', siteUtil.clearHistory(state.get('sites'))) filtering.clearHistory() } @@ -99,28 +97,29 @@ const sitesReducer = (state, action, emitChanges) => { state = syncUtil.updateSiteCache(state, siteDetail) }) break - case appConstants.APP_TAB_PINNED: - const tab = state.get('tabs').find((tab) => tab.get('tabId') === action.tabId) - if (!tab) { - console.warn('Trying to pin a tabId which does not exist:', action.tabId, 'tabs: ', state.get('tabs').toJS()) - break - } - const sites = state.get('sites') - const siteDetail = siteUtil.getDetailFromTab(tab, siteTags.PINNED, sites) - if (action.pinned) { - state = state.set('sites', siteUtil.addSite(sites, siteDetail, siteTags.PINNED)) - } else { - state = state.set('sites', siteUtil.removeSite(sites, siteDetail, siteTags.PINNED)) - } - if (syncEnabled()) { - state = syncUtil.updateSiteCache(state, siteDetail) + case appConstants.APP_TAB_UPDATED: + if (immutableAction.getIn(['changeInfo', 'pinned']) != null) { + const pinned = immutableAction.getIn(['changeInfo', 'pinned']) + const tabId = immutableAction.getIn(['tabValue', 'tabId']) + const tab = state.get('tabs').find((tab) => tab.get('tabId') === tabId) + if (!tab) { + console.warn('Trying to pin a tabId which does not exist:', tabId, 'tabs: ', state.get('tabs').toJS()) + break + } + const sites = state.get('sites') + const siteDetail = siteUtil.getDetailFromTab(tab, siteTags.PINNED, sites) + if (pinned) { + state = state.set('sites', siteUtil.addSite(sites, siteDetail, siteTags.PINNED)) + } else { + state = state.set('sites', siteUtil.removeSite(sites, siteDetail, siteTags.PINNED)) + } + if (syncEnabled()) { + state = syncUtil.updateSiteCache(state, siteDetail) + } } break - - case appConstants.APP_MAYBE_CREATE_TAB_REQUESTED: case appConstants.APP_CREATE_TAB_REQUESTED: { - action = makeImmutable(action) - const createProperties = action.get('createProperties') + const createProperties = immutableAction.get('createProperties') if (createProperties.get('pinned')) { state = state.set('sites', siteUtil.addSite(state.get('sites'), siteUtil.getDetailFromCreateProperties(createProperties), siteTags.PINNED)) diff --git a/app/browser/reducers/tabsReducer.js b/app/browser/reducers/tabsReducer.js index 42eb4ca0286..bdcd5bf2cb8 100644 --- a/app/browser/reducers/tabsReducer.js +++ b/app/browser/reducers/tabsReducer.js @@ -13,11 +13,9 @@ const windowAction = require('../../../js/actions/windowActions.js') const {makeImmutable} = require('../../common/state/immutableUtil') const {getFlashResourceId} = require('../../../js/flash') const {l10nErrorText} = require('../../common/lib/httpUtil') -const windows = require('../windows') const Immutable = require('immutable') const dragTypes = require('../../../js/constants/dragTypes') const {frameOptsFromFrame} = require('../../../js/state/frameStateUtil') -const {BrowserWindow} = require('electron') const tabsReducer = (state, action, immutableAction) => { action = immutableAction || makeImmutable(action) @@ -29,91 +27,132 @@ const tabsReducer = (state, action, immutableAction) => { state = tabState.maybeCreateTab(state, action) break case appConstants.APP_TAB_MOVED: { - const tabId = action.get('tabId') - const frameOpts = action.get('frameOpts') - const browserOpts = action.get('browserOpts') || new Immutable.Map() - const windowId = action.get('windowId') || -1 - state = tabs.moveTo(state, tabId, frameOpts, browserOpts, windowId) + setImmediate(() => { + const tabId = action.get('tabId') + const frameOpts = action.get('frameOpts') + const browserOpts = action.get('browserOpts') || new Immutable.Map() + const windowId = action.get('windowId') || -1 + tabs.moveTo(state, tabId, frameOpts, browserOpts, windowId) + }) break } case appConstants.APP_CREATE_TAB_REQUESTED: - tabs.createTab(action) - break - case appConstants.APP_MAYBE_CREATE_TAB_REQUESTED: - state = tabs.maybeCreateTab(state, action) + if (!action.getIn(['createProperties', 'windowId'])) { + const senderWindowId = action.getIn(['senderWindowId']) + if (senderWindowId) { + action = action.setIn(['createProperties', 'windowId'], senderWindowId) + } + } + + setImmediate(() => { + if (action.get('activateIfOpen')) { + tabs.maybeCreateTab(state, action) + } else { + tabs.createTab(action) + } + }) break case appConstants.APP_TAB_UPDATED: state = tabState.maybeCreateTab(state, action) break - case appConstants.APP_ACTIVE_WEB_CONTENTS_CLOSED: { - const tabValue = tabState.getActiveTabValue(state, BrowserWindow.getActiveWindow().id) - if (tabValue) { - const tabId = tabValue.get('tabId') - if (tabs.isDevToolsFocused(tabId)) { - state = tabs.toggleDevTools(state, tabId) - } else { - state = tabs.closeTab(state, tabId, false) + case appConstants.APP_TAB_CLOSE_REQUESTED: + { + const tabId = tabState.resolveTabId(state, action.get('tabId')) + if (tabId === tabState.TAB_ID_NONE) { + break } + + setImmediate(() => { + if (tabId) { + if (tabs.isDevToolsFocused(tabId)) { + tabs.toggleDevTools(tabId) + } else { + tabs.closeTab(tabId, action.get('forceClosePinned')) + } + } + }) } break - } - case appConstants.APP_TAB_CLOSED: { - const tabId = action.getIn(['tabValue', 'tabId']) - const forceClose = action.get('forceClose') - if (tabId) { - state = tabs.closeTab(state, tabId, forceClose) + case appConstants.APP_TAB_CLOSED: + { + const tabId = action.get('tabId') + if (tabId === tabState.TAB_ID_NONE) { + break + } + state = tabState.removeTabByTabId(state, tabId) } break - } case appConstants.APP_ALLOW_FLASH_ONCE: case appConstants.APP_ALLOW_FLASH_ALWAYS: - { + setImmediate(() => { const webContents = getWebContents(action.get('tabId')) if (webContents && !webContents.isDestroyed() && webContents.getURL() === action.get('url')) { webContents.authorizePlugin(getFlashResourceId()) } - break - } + }) + break case appConstants.APP_TAB_CLONED: - state = tabs.clone(state, action) + setImmediate(() => { + tabs.clone(action) + }) break case appConstants.APP_TAB_PINNED: - state = tabs.pin(state, action) + setImmediate(() => { + tabs.pin(state, action.get('tabId'), action.get('pinned')) + }) break case windowConstants.WINDOW_SET_AUDIO_MUTED: - state = tabs.setAudioMuted(state, action) + setImmediate(() => { + tabs.setAudioMuted(action) + }) break case windowConstants.WINDOW_SET_ALL_AUDIO_MUTED: action.get('frameList').forEach((frameProp) => { - state = tabs.setAudioMuted(state, frameProp) + setImmediate(() => { + tabs.setAudioMuted(frameProp) + }) }) break - case windowConstants.WINDOW_SET_ACTIVE_FRAME: - tabs.setActive(action.getIn(['frameProps', 'tabId'])) + case appConstants.APP_TAB_ACTIVATE_REQUESTED: + setImmediate(() => { + tabs.setActive(action.get('tabId')) + }) break case appConstants.APP_TAB_TOGGLE_DEV_TOOLS: - state = tabs.toggleDevTools(state, action.get('tabId')) + setImmediate(() => { + tabs.toggleDevTools(action.get('tabId')) + }) break case appConstants.APP_LOAD_URL_REQUESTED: - state = tabs.loadURL(state, action) + setImmediate(() => { + tabs.loadURL(action) + }) break case appConstants.APP_LOAD_URL_IN_ACTIVE_TAB_REQUESTED: - state = tabs.loadURLInActiveTab(state, action.get('windowId'), action.get('url')) + setImmediate(() => { + tabs.loadURLInActiveTab(state, action.get('windowId'), action.get('url')) + }) break case appConstants.APP_ON_GO_BACK: - state = tabs.goBack(state, action) + setImmediate(() => { + tabs.goBack(action.get('tabId')) + }) break case appConstants.APP_ON_GO_FORWARD: - state = tabs.goForward(state, action) + setImmediate(() => { + tabs.goForward(action.get('tabId')) + }) break case appConstants.APP_ON_GO_TO_INDEX: - state = tabs.goToIndex(state, action) + setImmediate(() => { + tabs.goToIndex(action.get('tabId'), action.get('index')) + }) break case appConstants.APP_ON_GO_BACK_LONG: { const history = tabs.getHistoryEntries(state, action) const tabValue = tabState.getByTabId(state, action.get('tabId')) - const windowId = windows.getActiveWindowId() + const windowId = tabValue.get('windowId') if (history !== null) { windowAction.onLongBackHistory( @@ -131,7 +170,7 @@ const tabsReducer = (state, action, immutableAction) => { { const history = tabs.getHistoryEntries(state, action) const tabValue = tabState.getByTabId(state, action.get('tabId')) - const windowId = windows.getActiveWindowId() + const windowId = tabValue.get('windowId') if (history !== null) { windowAction.onLongForwardHistory( diff --git a/app/browser/reducers/windowsReducer.js b/app/browser/reducers/windowsReducer.js index 9cc55ac54ce..2674d8525b5 100644 --- a/app/browser/reducers/windowsReducer.js +++ b/app/browser/reducers/windowsReducer.js @@ -19,11 +19,12 @@ const windowsReducer = (state, action, immutableAction) => { case appConstants.APP_WINDOW_READY: windows.windowReady(action.get('windowId')) break - case appConstants.APP_TAB_PINNED: - setImmediate(() => { - // check after the state has been updated - windows.pinnedTabsChanged() - }) + case appConstants.APP_TAB_UPDATED: + if (immutableAction.getIn(['changeInfo', 'pinned']) != null) { + setImmediate(() => { + windows.pinnedTabsChanged() + }) + } break case appConstants.APP_CLOSE_WINDOW: state = windows.closeWindow(state, action) diff --git a/app/browser/tabMessageBox.js b/app/browser/tabMessageBox.js index 1b0d78323ba..ff420fc73ef 100644 --- a/app/browser/tabMessageBox.js +++ b/app/browser/tabMessageBox.js @@ -90,7 +90,7 @@ const tabMessageBox = { onTabClosed: (state, action) => { action = makeImmutable(action) - const tabId = action.getIn(['tabValue', 'tabId']) + const tabId = action.get('tabId') if (tabId) { // remove callback; call w/ defaults const cb = messageBoxCallbacks[tabId] diff --git a/app/browser/tabs.js b/app/browser/tabs.js index ce1a100c102..c368b526049 100644 --- a/app/browser/tabs.js +++ b/app/browser/tabs.js @@ -9,7 +9,7 @@ const tabState = require('../common/state/tabState') const {app, BrowserWindow, extensions, session, ipcMain} = require('electron') const {makeImmutable} = require('../common/state/immutableUtil') const {getTargetAboutUrl, getSourceAboutUrl, isSourceAboutUrl, newFrameUrl, isTargetAboutUrl} = require('../../js/lib/appUrlUtil') -const {isURL, getUrlFromInput, toPDFJSLocation, getDefaultFaviconUrl} = require('../../js/lib/urlutil') +const {isURL, getUrlFromInput, toPDFJSLocation, getDefaultFaviconUrl, isHttpOrHttps} = require('../../js/lib/urlutil') const {isSessionPartition} = require('../../js/state/frameStateUtil') const {getOrigin} = require('../../js/state/siteUtil') const {getSetting} = require('../../js/settings') @@ -54,10 +54,10 @@ const getTabValue = function (tabId) { } } -const updateTab = (tabId) => { +const updateTab = (tabId, changeInfo = {}) => { let tabValue = getTabValue(tabId) if (tabValue) { - appActions.tabUpdated(tabValue) + appActions.tabUpdated(tabValue, makeImmutable(changeInfo)) } } @@ -93,6 +93,14 @@ const needsPartitionAssigned = (createProperties) => { createProperties.partition } +const isAutoDiscardable = (createProperties) => { + if (createProperties.pinned) { + return false + } + + return isHttpOrHttps(createProperties.url) +} + // TODO(bridiver) - refactor this into an action ipcMain.on(messages.ABOUT_COMPONENT_INITIALIZED, (e) => { const tab = e.sender @@ -285,7 +293,7 @@ const api = { let newTabValue = getTabValue(newTab.getId()) let index - if (newTabValue && parseInt(newTabValue.get('index')) > -1) { + if (parseInt(newTabValue.get('index')) > -1) { index = newTabValue.get('index') } @@ -300,12 +308,13 @@ const api = { const frameOpts = { location, partition: newTab.session.partition, - openInForeground: newTab.active, + openInForeground: !!newTabValue.get('active'), guestInstanceId: newTab.guestInstanceId, isPinned: !!newTabValue.get('pinned'), openerTabId, disposition, - index + index, + unloaded: !!newTabValue.get('discarded') } if (disposition === 'new-window' || disposition === 'new-popup') { @@ -320,11 +329,12 @@ const api = { updateTab(tabId) }) - process.on('chrome-tabs-updated', (tabId) => { - updateTab(tabId) + process.on('chrome-tabs-updated', (tabId, changeInfo) => { + updateTab(tabId, changeInfo) }) - process.on('chrome-tabs-removed', (tabId) => { + process.on('chrome-tabs-removed', (tabId, windowId) => { + appActions.tabClosed(tabId, windowId) cleanupWebContents(tabId) }) @@ -334,18 +344,6 @@ const api = { } let tabId = tab.getId() - tab.once('destroyed', cleanupWebContents.bind(null, tabId)) - tab.once('crashed', cleanupWebContents.bind(null, tabId)) - tab.once('close', cleanupWebContents.bind(null, tabId)) - - tab.on('page-favicon-updated', function (e, favicons) { - if (favicons && favicons.length > 0) { - // tab.setTabValues({ - // faviconUrl: favicons[0] - // }) - // updateTabDebounce(tabId) - } - }) tab.on('unresponsive', () => { console.log('unresponsive') }) @@ -365,10 +363,6 @@ const api = { return } - if (options.index !== undefined) { - tab.setTabIndex(options.index) - } - tab.once('did-attach', () => { if (options.back) { tab.goBack() @@ -394,7 +388,7 @@ const api = { } }, - toggleDevTools: (state, tabId) => { + toggleDevTools: (tabId) => { const tab = getWebContents(tabId) if (tab && !tab.isDestroyed()) { if (tab.isDevToolsOpened()) { @@ -403,7 +397,6 @@ const api = { tab.openDevTools() } } - return state }, setActive: (tabId) => { @@ -415,7 +408,7 @@ const api = { }) }, - loadURL: (state, action) => { + loadURL: (action) => { action = makeImmutable(action) const tabId = action.get('tabId') const tab = getWebContents(tabId) @@ -429,12 +422,11 @@ const api = { url, partition: tab.session.partition }) - return state + return } tab.loadURL(url) } - return state }, loadURLInActiveTab: (state, windowId, url) => { @@ -442,7 +434,6 @@ const api = { if (tabValue) { api.loadURLInTab(state, tabValue.get('tabId'), url) } - return state }, loadURLInTab: (state, tabId, url) => { @@ -454,20 +445,17 @@ const api = { return state }, - setAudioMuted: (state, action) => { + setAudioMuted: (action) => { action = makeImmutable(action) const muted = action.get('muted') const tabId = action.get('tabId') const tab = getWebContents(tabId) if (tab && !tab.isDestroyed()) { tab.setAudioMuted(muted) - const tabValue = getTabValue(tabId) - return tabState.updateTab(state, { tabValue }) } - return state }, - clone: (state, action) => { + clone: (action) => { action = makeImmutable(action) const tabId = action.get('tabId') let options = action.get('options') || Immutable.Map() @@ -480,27 +468,21 @@ const api = { tab.clone(options.toJS(), (newTab) => { }) } - return state }, - pin: (state, action) => { - action = makeImmutable(action) - const tabId = action.get('tabId') - const pinned = action.get('pinned') + pin: (state, tabId, pinned) => { const tab = getWebContents(tabId) if (tab && !tab.isDestroyed()) { const url = tab.getURL() // For now we only support 1 tab pin per URL const alreadyPinnedTab = tabState.queryTab(state, { url, pinned: true, partition: tab.session.partition }) if (pinned && alreadyPinnedTab) { - tab.close(tab) - return tabState.removeTabByTabId(state, tabId) + api.closeTab(tabId) + return } tab.setPinned(pinned) - state = tabState.updateTabValue(state, getTabValue(tabId)) } - return state }, isDevToolsFocused: (tabId) => { @@ -511,31 +493,32 @@ const api = { tab.isDevToolsFocused() }, - closeTab: (state, tabId, forceClose) => { + closeTab: (tabId, forceClosePinned = false) => { const tabValue = getTabValue(tabId) if (!tabValue) { - return state + return false } const tab = getWebContents(tabId) try { if (tab && !tab.isDestroyed()) { - if (forceClose) { - tab.forceClose() + if (tabValue.get('pinned') && !forceClosePinned) { + tab.hostWebContents.send(messages.SHORTCUT_NEXT_TAB) + return false } else { - tab.close(tab) + tab.forceClose() } } } catch (e) { - // ignore + return false } - return tabState.removeTab(state, tabValue) + return true }, create: (createProperties, cb = null) => { setImmediate(() => { createProperties = makeImmutable(createProperties).toJS() const switchToNewTabsImmediately = getSetting(settings.SWITCH_TO_NEW_TABS) === true - if (switchToNewTabsImmediately) { + if (createProperties.active == null && switchToNewTabsImmediately) { createProperties.active = true } if (!createProperties.url) { @@ -548,6 +531,10 @@ const api = { createProperties.parent_partition = '' } } + if (!isAutoDiscardable(createProperties)) { + createProperties.discarded = false + createProperties.autoDiscardable = false + } extensions.createTab(createProperties, (tab) => { cb && cb(tab) }) @@ -588,7 +575,7 @@ const api = { const currentWindowId = tabValue && tabValue.get('windowId') if (windowId != null && currentWindowId === windowId) { - return state + return } if (windowId == null || windowId === -1) { @@ -596,13 +583,13 @@ const api = { // a new window to be created. const windowTabCount = tabState.getTabsByWindowId(state, currentWindowId).size if (windowTabCount === 1) { - return state + return } } if (tabValue.get('pinned')) { // If the current tab is pinned, then don't allow to drag out - return state + return } tab.detach(() => { @@ -613,7 +600,6 @@ const api = { } }) } - return state }, maybeCreateTab: (state, action) => { @@ -623,7 +609,7 @@ const api = { createProperties = createProperties.set('url', url) const windowId = createProperties.get('windowId') if (!windowId) { - return state + return } const tabData = tabState.getMatchingTab(state, createProperties, windowId, url) if (tabData) { @@ -631,34 +617,27 @@ const api = { } else { api.createTab(action) } - return state }, - goBack: (state, action) => { - action = makeImmutable(action) - const tab = getWebContents(action.get('tabId')) + goBack: (tabId) => { + const tab = getWebContents(tabId) if (tab && !tab.isDestroyed()) { tab.goBack() } - return state }, - goForward: (state, action) => { - action = makeImmutable(action) - const tab = getWebContents(action.get('tabId')) + goForward: (tabId) => { + const tab = getWebContents(tabId) if (tab && !tab.isDestroyed()) { tab.goForward() } - return state }, - goToIndex: (state, action) => { - action = makeImmutable(action) - const tab = getWebContents(action.get('tabId')) + goToIndex: (tabId, index) => { + const tab = getWebContents(tabId) if (tab && !tab.isDestroyed()) { - tab.goToIndex(action.get('index')) + tab.goToIndex(index) } - return state }, getHistoryEntries: (state, action) => { diff --git a/app/browser/webContentsCache.js b/app/browser/webContentsCache.js index 2cc7180811f..77f35ab6f31 100644 --- a/app/browser/webContentsCache.js +++ b/app/browser/webContentsCache.js @@ -2,14 +2,11 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at http://mozilla.org/MPL/2.0/. */ -const appActions = require('../../js/actions/appActions') - const currentWebContents = {} const cleanupWebContents = (tabId) => { if (currentWebContents[tabId]) { delete currentWebContents[tabId] - appActions.tabClosed({ tabId }) } } diff --git a/app/browser/windows.js b/app/browser/windows.js index 601fa663087..037bba1e5e0 100644 --- a/app/browser/windows.js +++ b/app/browser/windows.js @@ -104,7 +104,7 @@ const updatePinnedTabs = (win) => { tab.get('url') === site.get('location') && (tab.get('partitionNumber') || 0) === (site.get('partitionNumber') || 0)) if (tab) { - appActions.tabClosed(tab, true) + appActions.tabCloseRequested(tab.get('tabId'), true) } win.__alreadyPinnedSites = win.__alreadyPinnedSites.remove(site) }) diff --git a/app/common/commonMenu.js b/app/common/commonMenu.js index 736bdf58d89..9dfd7fedd49 100644 --- a/app/common/commonMenu.js +++ b/app/common/commonMenu.js @@ -199,7 +199,8 @@ module.exports.preferencesMenuItem = () => { location: 'about:preferences' })) } else { - appActions.maybeCreateTabRequested({ + appActions.createTabRequested({ + activateIfOpen: true, url: 'about:preferences', windowId: getCurrentWindowId() }) @@ -218,7 +219,8 @@ module.exports.bookmarksManagerMenuItem = () => { location: 'about:bookmarks' })) } else { - appActions.maybeCreateTabRequested({ + appActions.createTabRequested({ + activateIfOpen: true, url: 'about:bookmarks', windowId: getCurrentWindowId() }) @@ -237,7 +239,8 @@ module.exports.historyMenuItem = () => { location: 'about:history' })) } else { - appActions.maybeCreateTabRequested({ + appActions.createTabRequested({ + activateIfOpen: true, url: 'about:history', windowId: getCurrentWindowId() }) @@ -257,7 +260,8 @@ module.exports.downloadsMenuItem = () => { })) } else { module.exports.sendToFocusedWindow(focusedWindow, [messages.HIDE_DOWNLOADS_TOOLBAR]) - appActions.maybeCreateTabRequested({ + appActions.createTabRequested({ + activateIfOpen: true, url: 'about:downloads', windowId: getCurrentWindowId() }) @@ -275,7 +279,8 @@ module.exports.extensionsMenuItem = () => { location: 'about:preferences#extensions' })) } else { - appActions.maybeCreateTabRequested({ + appActions.createTabRequested({ + activateIfOpen: true, url: 'about:preferences#extensions', windowId: getCurrentWindowId() }) @@ -293,7 +298,8 @@ module.exports.passwordsMenuItem = () => { location: 'about:passwords' })) } else { - appActions.maybeCreateTabRequested({ + appActions.createTabRequested({ + activateIfOpen: true, url: 'about:passwords', windowId: getCurrentWindowId() }) @@ -332,7 +338,8 @@ module.exports.submitFeedbackMenuItem = () => { return { label: locale.translation('submitFeedback'), click: function (item, focusedWindow) { - appActions.maybeCreateTabRequested({ + appActions.createTabRequested({ + activateIfOpen: true, url: communityURL, windowId: getCurrentWindowId() }) @@ -367,7 +374,8 @@ module.exports.aboutBraveMenuItem = () => { return { label: locale.translation('aboutApp'), click: (item, focusedWindow) => { - appActions.maybeCreateTabRequested({ + appActions.createTabRequested({ + activateIfOpen: true, url: 'about:brave', windowId: getCurrentWindowId() }) @@ -393,7 +401,8 @@ module.exports.braveryGlobalMenuItem = () => { location: 'about:preferences#shields' })) } else { - appActions.maybeCreateTabRequested({ + appActions.createTabRequested({ + activateIfOpen: true, url: 'about:preferences#shields', windowId: getCurrentWindowId() }) @@ -415,7 +424,8 @@ module.exports.braveryPaymentsMenuItem = () => { location: 'about:preferences#payments' })) } else { - appActions.maybeCreateTabRequested({ + appActions.createTabRequested({ + activateIfOpen: true, url: 'about:preferences#payments', windowId: getCurrentWindowId() }) diff --git a/app/common/state/tabState.js b/app/common/state/tabState.js index 93ea8185b84..708817a4997 100644 --- a/app/common/state/tabState.js +++ b/app/common/state/tabState.js @@ -12,7 +12,7 @@ const frameStateUtil = require('../../../js/state/frameStateUtil') const validateId = function (propName, id) { assert.ok(id, `${propName} cannot be null`) id = parseInt(id) - assert.ok(id === -1 || id > 0, `${propName} must be positive`) + assert.ok(id >= -2, `${propName} must be a valid`) return id } @@ -62,6 +62,7 @@ const tabState = { }, TAB_ID_NONE: -1, + TAB_ID_ACTIVE: -2, validateTabId: (tabId) => { validateId('tabId', tabId) @@ -167,7 +168,7 @@ const tabState = { }, getPathByTabId: (state, tabId) => { - tabId = validateId('tabId', tabId) + tabId = tabState.resolveTabId(state, tabId) state = validateState(state) const index = state.get('tabs').findIndex((tab) => tab.get('tabId') === tabId) @@ -178,9 +179,6 @@ const tabState = { }, getByTabId: (state, tabId) => { - tabId = validateId('tabId', tabId) - state = validateState(state) - if (tabId === tabState.TAB_ID_NONE) { return null } @@ -306,9 +304,17 @@ const tabState = { return tabState.getTabPropertyByTabId(state, tabId, 'active') || false }, + // TOOD(bridiver) - make everything work with TAB_ID_ACTIVE + resolveTabId: (state, tabId) => { + if (tabId === tabState.TAB_ID_ACTIVE) { + tabId = tabState.getActiveTabId(state) + } + return validateId('tabId', tabId) + }, + getFramePathByTabId: (state, tabId) => { + tabId = tabState.resolveTabId(state, tabId) state = makeImmutable(state) - tabId = validateId('tabId', tabId) if (tabId === tabState.TAB_ID_NONE) { return null diff --git a/app/ledger.js b/app/ledger.js index 439a7e4bb00..d2b7a44dc20 100644 --- a/app/ledger.js +++ b/app/ledger.js @@ -535,7 +535,8 @@ if (ipc) { appActions.changeSetting(settings.PAYMENTS_NOTIFICATIONS, false) } else if (buttonIndex === 2 && win) { // Add funds: Open payments panel - appActions.maybeCreateTabRequested({ + appActions.createTabRequested({ + activateIfOpen: true, url: 'about:preferences#payments', windowId: win.id }) @@ -546,7 +547,8 @@ if (ipc) { if (buttonIndex === 0) { appActions.changeSetting(settings.PAYMENTS_NOTIFICATIONS, false) } else if (buttonIndex === 2 && win) { - appActions.maybeCreateTabRequested({ + appActions.createTabRequested({ + activateIfOpen: true, url: 'about:preferences#payments', windowId: win.id }) @@ -559,7 +561,8 @@ if (ipc) { } else if (message === notificationTryPaymentsMessage) { appActions.hideNotification(message) if (buttonIndex === 1 && win) { - appActions.maybeCreateTabRequested({ + appActions.createTabRequested({ + activateIfOpen: true, url: 'about:preferences#payments', windowId: win.id }) diff --git a/app/renderer/components/navigation/browserAction.js b/app/renderer/components/navigation/browserAction.js index 16d6aeec0ba..4017e12874c 100644 --- a/app/renderer/components/navigation/browserAction.js +++ b/app/renderer/components/navigation/browserAction.js @@ -6,6 +6,7 @@ const React = require('react') const electron = require('electron') const ipc = electron.ipcRenderer const {StyleSheet, css} = require('aphrodite') +const Immutable = require('immutable') // Components const ReduxComponent = require('../reduxComponent') @@ -15,13 +16,11 @@ const BrowserActionBadge = require('./browserActionBadge') // State const extensionState = require('../../../common/state/extensionState') const tabState = require('../../../common/state/tabState') +const frameStateUtil = require('../../../../js/state/frameStateUtil') // Actions const windowActions = require('../../../../js/actions/windowActions') -// Utils -const {getCurrentWindowId} = require('../../currentWindow') - class BrowserAction extends React.Component { constructor () { super() @@ -54,8 +53,8 @@ class BrowserAction extends React.Component { mergeProps (state, dispatchProps, ownProps) { const currentWindow = state.get('currentWindow') - const activeTab = tabState.getActiveTabValue(state, getCurrentWindowId()) - const activeTabId = activeTab && activeTab.get('tabId') + const activeFrame = frameStateUtil.getActiveFrame(currentWindow) || Immutable.Map() + const activeTabId = activeFrame.get('tabId') || tabState.TAB_ID_NONE const browserActions = extensionState.getBrowserActionByTabId(state, ownProps.extensionId, activeTabId) const props = {} diff --git a/app/renderer/components/navigation/navigationBar.js b/app/renderer/components/navigation/navigationBar.js index d219067454e..6076095822b 100644 --- a/app/renderer/components/navigation/navigationBar.js +++ b/app/renderer/components/navigation/navigationBar.js @@ -24,7 +24,6 @@ const windowStore = require('../../../../js/stores/windowStore') const contextMenus = require('../../../../js/contextMenus') const LongPressButton = require('./../../../../js/components/longPressButton') const PublisherToggle = require('../publisherToggle') -const {getCurrentWindowId} = require('../../currentWindow') // State const tabState = require('../../../common/state/tabState') @@ -114,14 +113,14 @@ class NavigationBar extends React.Component { } mergeProps (state, dispatchProps, ownProps) { - const windowState = state.get('currentWindow') - const activeFrame = frameStateUtil.getActiveFrame(windowState) || Immutable.Map() + const currentWindow = state.get('currentWindow') + const activeFrame = frameStateUtil.getActiveFrame(currentWindow) || Immutable.Map() const activeFrameKey = activeFrame.get('key') - const activeTabId = tabState.getActiveTabId(state, getCurrentWindowId()) + const activeTabId = activeFrame.get('tabId') || tabState.TAB_ID_NONE const activeTabShowingMessageBox = tabState.isShowingMessageBox(state, activeTabId) - const bookmarkDetail = windowState.get('bookmarkDetail') - const mouseInTitlebar = windowState.getIn(['ui', 'mouseInTitlebar']) + const bookmarkDetail = currentWindow.get('bookmarkDetail') + const mouseInTitlebar = currentWindow.getIn(['ui', 'mouseInTitlebar']) const title = activeFrame.get('title') || '' const loading = activeFrame.get('loading') const location = activeFrame.get('location') || '' @@ -153,7 +152,7 @@ class NavigationBar extends React.Component { props.bookmarkDetail = bookmarkDetail props.mouseInTitlebar = mouseInTitlebar props.settings = state.get('settings') - props.menubarVisible = menuBarState.isMenuBarVisible(windowState) + props.menubarVisible = menuBarState.isMenuBarVisible(currentWindow) props.siteSettings = state.get('siteSettings') props.synopsis = state.getIn(['publisherInfo', 'synopsis']) || new Immutable.Map() props.activeTabShowingMessageBox = activeTabShowingMessageBox diff --git a/app/renderer/components/navigation/navigator.js b/app/renderer/components/navigation/navigator.js index 3de974a283e..03ca1be03e7 100644 --- a/app/renderer/components/navigation/navigator.js +++ b/app/renderer/components/navigation/navigator.js @@ -6,6 +6,7 @@ const React = require('react') const {StyleSheet, css} = require('aphrodite') const electron = require('electron') const ipc = electron.ipcRenderer +const Immutable = require('immutable') // Actions const appActions = require('../../../../js/actions/appActions') @@ -147,10 +148,10 @@ class Navigator extends React.Component { mergeProps (state, dispatchProps, ownProps) { const currentWindow = state.get('currentWindow') - const activeTab = tabState.getActiveTabValue(state, getCurrentWindowId()) - const activeTabId = activeTab && activeTab.get('tabId') + const activeFrame = frameStateUtil.getActiveFrame(currentWindow) || Immutable.Map() + const activeTabId = activeFrame.get('tabId') || tabState.TAB_ID_NONE + const activeTab = tabState.getByTabId(state, activeTabId) const activeTabShowingMessageBox = !!(activeTab && tabState.isShowingMessageBox(state, activeTabId)) - const activeFrame = frameStateUtil.getActiveFrame(currentWindow) const allSiteSettings = siteSettingsState.getAllSiteSettings(state, activeFrame) const braverySettings = siteSettings.activeSettings(allSiteSettings, state, appConfig) const enabledExtensions = extensionState.getEnabledExtensions(state) diff --git a/app/renderer/components/navigation/urlBar.js b/app/renderer/components/navigation/urlBar.js index 3a9d38283af..c4efe83d0b3 100644 --- a/app/renderer/components/navigation/urlBar.js +++ b/app/renderer/components/navigation/urlBar.js @@ -409,9 +409,9 @@ class UrlBar extends React.Component { } mergeProps (state, dispatchProps, ownProps) { - const windowState = state.get('currentWindow') - const activeFrame = frameStateUtil.getActiveFrame(windowState) || Immutable.Map() - const activeTabId = tabState.getActiveTabId(state, getCurrentWindowId()) + const currentWindow = state.get('currentWindow') + const activeFrame = frameStateUtil.getActiveFrame(currentWindow) || Immutable.Map() + const activeTabId = activeFrame.get('tabId') || tabState.TAB_ID_NONE const location = activeFrame.get('location') || '' const baseUrl = getBaseUrl(location) @@ -436,7 +436,7 @@ class UrlBar extends React.Component { const activateSearchEngine = urlbar.getIn(['searchDetail', 'activateSearchEngine']) const urlbarSearchDetail = urlbar.get('searchDetail') - let searchURL = windowState.getIn(['searchDetail', 'searchURL']) + let searchURL = currentWindow.getIn(['searchDetail', 'searchURL']) let searchShortcut = '' // remove shortcut from the search terms if (activateSearchEngine && urlbarSearchDetail !== null) { @@ -464,7 +464,7 @@ class UrlBar extends React.Component { props.endLoadTime = activeFrame.get('endLoadTime') props.loading = activeFrame.get('loading') props.enableNoScript = siteSettingsState.isNoScriptEnabled(state, braverySettings) - props.noScriptIsVisible = windowState.getIn(['ui', 'noScriptInfo', 'isVisible']) || false + props.noScriptIsVisible = currentWindow.getIn(['ui', 'noScriptInfo', 'isVisible']) || false props.menubarVisible = ownProps.menubarVisible props.activeTabShowingMessageBox = tabState.isShowingMessageBox(state, activeTabId) props.noBorderRadius = isPublisherButtonEnabled diff --git a/app/renderer/components/tabs/tab.js b/app/renderer/components/tabs/tab.js index 27494ccac99..3371cfdb962 100644 --- a/app/renderer/components/tabs/tab.js +++ b/app/renderer/components/tabs/tab.js @@ -17,6 +17,7 @@ const CloseTabIcon = require('./content/closeTabIcon') const {NotificationBarCaret} = require('../../../../js/components/notificationBar') // Actions +const appActions = require('../../../../js/actions/appActions') const windowActions = require('../../../../js/actions/windowActions') // Store @@ -140,16 +141,11 @@ class Tab extends ImmutableComponent { dnd.onDragOver(dragTypes.TAB, this.tabNode.getBoundingClientRect(), this.props.tab.get('frameKey'), this.draggingOverData, e) } - setActiveFrame (event) { - event.stopPropagation() - windowActions.setActiveFrame(this.frame) - } - onTabClosedWithMouse (event) { event.stopPropagation() if (this.props.onTabClosedWithMouse && this.frame && !this.frame.isEmpty()) { this.props.onTabClosedWithMouse(this.tabNode.parentNode.getBoundingClientRect()) - windowActions.closeFrame(this.frame) + appActions.tabCloseRequested(this.frame.get('tabId')) } } @@ -198,7 +194,8 @@ class Tab extends ImmutableComponent { if (e.button === 1) { this.onTabClosedWithMouse(e) } else { - this.setActiveFrame(e) + e.stopPropagation() + appActions.tabActivateRequested(this.frame.get('tabId')) } } diff --git a/app/renderer/reducers/frameReducer.js b/app/renderer/reducers/frameReducer.js index 1cbfa95318e..9b53f117978 100644 --- a/app/renderer/reducers/frameReducer.js +++ b/app/renderer/reducers/frameReducer.js @@ -4,17 +4,16 @@ 'use strict' -const electron = require('electron') +const Immutable = require('immutable') const windowConstants = require('../../../js/constants/windowConstants') const frameStateUtil = require('../../../js/state/frameStateUtil') -const siteUtil = require('../../../js/state/siteUtil') const appActions = require('../../../js/actions/appActions') const windowActions = require('../../../js/actions/windowActions') const settings = require('../../../js/constants/settings') const getSetting = require('../../../js/settings').getSetting +const config = require('../../../js/constants/config') const {updateTabPageIndex} = require('../lib/tabUtil') const {getCurrentWindowId} = require('../currentWindow') -const messages = require('../../../js/constants/messages') const setFullScreen = (state, action) => { return state.mergeIn(['frames', frameStateUtil.getFramePropsIndex(state.get('frames'), action.frameProps)], { @@ -25,8 +24,11 @@ const setFullScreen = (state, action) => { const closeFrame = (state, action) => { // Use the frameProps we passed in, or default to the active frame - const frameProps = action.frameProps || frameStateUtil.getActiveFrame(state) + const frameProps = action.frameProps const index = frameStateUtil.getFramePropsIndex(state.get('frames'), frameProps) + if (index === -1) { + return state + } const hoverState = state.getIn(['frames', index, 'hoverState']) const activeFrameKey = frameStateUtil.getActiveFrame(state).get('key') state = state.merge(frameStateUtil.removeFrame( @@ -59,79 +61,38 @@ const closeFrame = (state, action) => { const frameReducer = (state, action) => { switch (action.actionType) { case windowConstants.WINDOW_CLOSE_FRAMES: - const activeFrameKey = state.get('activeFrameKey') - const activeFrame = frameStateUtil.findFrameInList(action.framePropsList, activeFrameKey) - - if (activeFrame) { - const origin = siteUtil.getOrigin(activeFrame.get('location')) - if (origin) { - appActions.clearNotifications(origin) - } - - // If the frame was full screen, exit - if (activeFrame && activeFrame.get('isFullScreen')) { - state = setFullScreen(state, { - frameProps: activeFrame, - isFullScreen: false - }) + let closedFrames = new Immutable.List() + action.framePropsList.forEach((frameProps) => { + if (!frameProps.get('isPrivate') && frameProps.get('location') !== 'about:newtab') { + closedFrames = closedFrames.push(frameProps) + if (closedFrames.size > config.maxClosedFrames) { + closedFrames = closedFrames.shift() + } } - } - - state = state.merge(frameStateUtil.removeFrames( - state.get('frames'), - state.get('tabs'), - state.get('closedFrames'), - action.framePropsList, - activeFrame, - state.get('activeFrameKey'), - getSetting(settings.TAB_CLOSE_ACTION) - )) + }) - state = updateTabPageIndex(state, frameStateUtil.getActiveFrame(state)) - state = state.deleteIn(['ui', 'tabs', 'fixTabWidth']) + closedFrames.forEach((frame) => { + appActions.tabCloseRequested(frame.get('tabId')) + }) break - case windowConstants.WINDOW_CLOSE_FRAME: - const ipc = electron.ipcRenderer - const origin = siteUtil.getOrigin(action.frameProps.get('location')) - if (origin) { - appActions.clearNotifications(origin) - } - // If the frame was full screen, exit - if (action.frameProps && action.frameProps.get('isFullScreen')) { - state = setFullScreen(state, { - frameProps: action.frameProps, - isFullScreen: false - }) + if (!action.frameProps) { + break } // Unless a caller explicitly specifies to close a pinned frame, then // ignore the call. const frames = frameStateUtil.getFrames(state) const nonPinnedFrames = frames.filter((frame) => !frame.get('pinnedLocation')) - if (action.frameProps && action.frameProps.get('pinnedLocation')) { - // Check for no frames at all, and if that's the case the user - // only has pinned frames and tried to close, so close the - // whole app. - if (nonPinnedFrames.size === 0) { - appActions.closeWindow(getCurrentWindowId()) - return state - } - - const frameKey = action.frameProps ? action.frameProps.get('key') : null - const activeFrameKey = state.get('activeFrameKey') - - if (!action.forceClosePinned && frameKey === activeFrameKey) { - // Go to next frame if the user tries to close a pinned tab - ipc.emit(messages.SHORTCUT_NEXT_TAB) - return state - } - } - const pinnedFrames = frames.filter((frame) => frame.get('pinnedLocation')) // If there is at least 1 pinned frame don't close the window until subsequent // close attempts if (nonPinnedFrames.size > 1 || pinnedFrames.size > 0) { state = closeFrame(state, action) + + const frame = frameStateUtil.getActiveFrame(state) + if (frame) { + appActions.tabActivateRequested(frame.get('tabId')) + } } else { appActions.closeWindow(getCurrentWindowId()) } diff --git a/app/renderer/suggestionClickHandlers.js b/app/renderer/suggestionClickHandlers.js index 896f65b6439..72c41b6038e 100644 --- a/app/renderer/suggestionClickHandlers.js +++ b/app/renderer/suggestionClickHandlers.js @@ -27,7 +27,7 @@ const navigateSiteClickHandler = (formatUrl) => (site, isForSecondaryAction, shi } const frameClickHandler = (frameProps) => - windowActions.setActiveFrame(frameProps) + appActions.tabActivateRequested(frameProps.get('tabId')) module.exports = { navigateSiteClickHandler, diff --git a/docs/appActions.md b/docs/appActions.md index 00f97f732c6..748941775df 100644 --- a/docs/appActions.md +++ b/docs/appActions.md @@ -115,36 +115,47 @@ A request for a URL email share occurred -### maybeCreateTabRequested(createProperties) +### tabUpdated(tabValue, changeInfo) -A request for a "maybe" new tab has been made with the specified createProperties -If a tab is already opened it will instead set it as active. +A tab has been updated **Parameters** -**createProperties**: `Object`, these are only used if a new tab is being created +**tabValue**: `Object`, A tab has been updated +**changeInfo**: `Object`, from chrome-tabs-updated -### tabUpdated(tabValue) -A tab has been updated +### tabActivateRequested(tabId) + +Dispatches a message to the store to set a new frame as the active frame. **Parameters** -**tabValue**: `Object`, A tab has been updated +**tabId**: `Number`, the tabId to activate -### tabClosed(tabId, force) +### tabCloseRequested(tabId, forceClosePinned) -Closes an open tab +Dispatches a message to close the tabId **Parameters** -**tabId**: `number`, Closes an open tab +**tabId**: `Number`, the tabId to close + +**forceClosePinned**: `Boolean`, force close if pinned + + + +### tabClosed(tabId) + +Notifies that a tab has been closed + +**Parameters** -**force**: `boolean`, closing the tab +**tabId**: `number`, Notifies that a tab has been closed @@ -446,16 +457,6 @@ Hides a message in the notification bar -### clearNotifications(origin) - -Clears all notifications for a given origin. - -**Parameters** - -**origin**: `string`, Clears all notifications for a given origin. - - - ### addWord(word, learn) Adds a word to the dictionary @@ -748,12 +749,6 @@ Dispatches a message to toogle the dev tools on/off for the specified tabId -### activeWebContentsClosed() - -Dispatches a message to toogle the dev tools on/off or close the tab, depending on what's active. - - - ### tabCloned(tabId, options) Dispatches a message when a tab is being cloned diff --git a/docs/windowActions.md b/docs/windowActions.md index 97db8ab50cc..29e830957eb 100644 --- a/docs/windowActions.md +++ b/docs/windowActions.md @@ -179,20 +179,7 @@ Dispatches a message to the store to indicate that the webview entered full scre -### unloadedTabCreated(frameOpts, openInForeground) - -Dispatches a message to the store to create a new unloaded frame - -**Parameters** - -**frameOpts**: `Object`, An object of frame options such as isPrivate, element, and tab features. - These may not all be hooked up in Electron yet. - -**openInForeground**: `boolean`, true if the new frame should become the new active frame - - - -### closeFrame(frames, frameProps, forceClosePinned) +### closeFrame(frames, frameProps) Dispatches a message to close a frame @@ -202,8 +189,6 @@ Dispatches a message to close a frame **frameProps**: `Object`, The properties of the frame to close -**forceClosePinned**: `boolean`, Should we force close pinned tab - ### closeFrames(framePropsList) @@ -229,16 +214,6 @@ Dispatches a message to the store to clear closed frames -### setActiveFrame(frameProps) - -Dispatches a message to the store to set a new frame as the active frame. - -**Parameters** - -**frameProps**: `Object`, the frame properties for the webview in question. - - - ### setFocusedFrame(frameProps) Dispatches a message to the store when the frame is active and the window is focused diff --git a/js/actions/appActions.js b/js/actions/appActions.js index 93b8a96201f..8c5f1040454 100644 --- a/js/actions/appActions.js +++ b/js/actions/appActions.js @@ -115,10 +115,11 @@ const appActions = { * A request for a new tab has been made with the specified createProperties * @param {Object} createProperties */ - createTabRequested: function (createProperties) { + createTabRequested: function (createProperties, activateIfOpen) { AppDispatcher.dispatch({ actionType: appConstants.APP_CREATE_TAB_REQUESTED, - createProperties + createProperties, + activateIfOpen }) }, @@ -162,39 +163,55 @@ const appActions = { }, /** - * A request for a "maybe" new tab has been made with the specified createProperties - * If a tab is already opened it will instead set it as active. + * A tab has been updated + * @param {Object} tabValue + * @param {Object} changeInfo from chrome-tabs-updated + */ + tabUpdated: function (tabValue, changeInfo) { + AppDispatcher.dispatch({ + actionType: appConstants.APP_TAB_UPDATED, + tabValue, + changeInfo + }) + }, + + /** + * Dispatches a message to the store to set a new frame as the active frame. * - * @param {Object} createProperties - these are only used if a new tab is being created + * @param {Number} tabId - the tabId to activate */ - maybeCreateTabRequested: function (createProperties) { + tabActivateRequested: function (tabId) { AppDispatcher.dispatch({ - actionType: appConstants.APP_MAYBE_CREATE_TAB_REQUESTED, - createProperties + actionType: appConstants.APP_TAB_ACTIVATE_REQUESTED, + tabId }) }, /** - * A tab has been updated - * @param {Object} tabValue + * Dispatches a message to close the tabId + * + * @param {Number} tabId - the tabId to close + * @param {Boolean} forceClosePinned - force close if pinned */ - tabUpdated: function (tabValue) { + tabCloseRequested: function (tabId, forceClosePinned = false) { AppDispatcher.dispatch({ - actionType: appConstants.APP_TAB_UPDATED, - tabValue + actionType: appConstants.APP_TAB_CLOSE_REQUESTED, + tabId, + forceClosePinned }) }, /** - * Closes an open tab + * Notifies that a tab has been closed * @param {number} tabId - * @param {boolean} force closing the tab */ - tabClosed: function (tabValue, forceClose = false) { + tabClosed: function (tabId, windowId) { AppDispatcher.dispatch({ actionType: appConstants.APP_TAB_CLOSED, - tabValue, - forceClose + tabId, + queryInfo: { + windowId + } }) }, @@ -536,17 +553,6 @@ const appActions = { }) }, - /** - * Clears all notifications for a given origin. - * @param {string} origin - */ - clearNotifications: function (origin) { - AppDispatcher.dispatch({ - actionType: appConstants.APP_CLEAR_NOTIFICATIONS, - origin - }) - }, - /** * Adds a word to the dictionary * @param {string} word - The word to add @@ -917,15 +923,6 @@ const appActions = { }) }, - /** - * Dispatches a message to toogle the dev tools on/off or close the tab, depending on what's active. - */ - activeWebContentsClosed: function () { - AppDispatcher.dispatch({ - actionType: appConstants.APP_ACTIVE_WEB_CONTENTS_CLOSED - }) - }, - /** * Dispatches a message when a tab is being cloned * @param {number} tabId - The tabId of the tab to clone diff --git a/js/actions/bookmarkActions.js b/js/actions/bookmarkActions.js index 099e509267a..30bb7507c90 100644 --- a/js/actions/bookmarkActions.js +++ b/js/actions/bookmarkActions.js @@ -26,9 +26,10 @@ const bookmarkActions = { active: false }), getSetting(SWITCH_TO_NEW_TABS)) } else { - windowActions.unloadedTabCreated({ + appActions.createTabRequested({ location: bookmark.get('location'), - partitionNumber: bookmark.get('partitionNumber') + partitionNumber: bookmark.get('partitionNumber'), + discarded: true }, false) } }) diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js index abb7ba9600a..d753a0bca42 100644 --- a/js/actions/windowActions.js +++ b/js/actions/windowActions.js @@ -207,33 +207,16 @@ const windowActions = { }) }, - /** - * Dispatches a message to the store to create a new unloaded frame - * - * @param {Object} frameOpts - An object of frame options such as isPrivate, element, and tab features. - * These may not all be hooked up in Electron yet. - * @param {boolean} openInForeground - true if the new frame should become the new active frame - */ - unloadedTabCreated: function (frameOpts, openInForeground) { - dispatch({ - actionType: windowConstants.WINDOW_UNLOADED_TAB_CREATED, - frameOpts: frameOpts, - openInForeground - }) - }, - /** * Dispatches a message to close a frame * * @param {Object[]} frames - Immutable list of of all the frames * @param {Object} frameProps - The properties of the frame to close - * @param {boolean} forceClosePinned - Should we force close pinned tab */ - closeFrame: function (frameProps, forceClosePinned) { + closeFrame: function (frameProps) { dispatch({ actionType: windowConstants.WINDOW_CLOSE_FRAME, - frameProps, - forceClosePinned + frameProps }) }, @@ -267,14 +250,9 @@ const windowActions = { }) }, - /** - * Dispatches a message to the store to set a new frame as the active frame. - * - * @param {Object} frameProps - the frame properties for the webview in question. - */ - setActiveFrame: function (frameProps) { + activeFrameChanged: function (frameProps) { dispatch({ - actionType: windowConstants.WINDOW_SET_ACTIVE_FRAME, + actionType: windowConstants.WINDOW_ACTIVE_FRAME_CHANGED, frameProps: frameProps }) }, diff --git a/js/components/frame.js b/js/components/frame.js index 01eee0a869e..55f7fad9b1b 100644 --- a/js/components/frame.js +++ b/js/components/frame.js @@ -37,7 +37,6 @@ const domUtil = require('../../app/renderer/lib/domUtil') const { aboutUrls, isSourceMagnetUrl, - isSourceAboutUrl, isTargetAboutUrl, getTargetAboutUrl, getBaseUrl, @@ -169,24 +168,15 @@ class Frame extends React.Component { this.expireContentSettings(this.origin) } - updateWebview (cb, prevProps, newSrc) { - // lazy load webview - if (!this.webview && !this.props.isActive && !this.props.isPreview && - // allow force loading of new frames - this.props.unloaded === true && - // don't lazy load about pages - !aboutUrls.get(getBaseUrl(this.props.src)) && - // pinned tabs don't serialize their state so the icon is lost for lazy loading - !this.props.pinnedLocation) { + updateWebview (cb, prevProps = {}) { + if (cb && this.runOnDomReady) { + // there is already a callback waiting for did-attach + // so replace it with this callback because it might be a + // mount callback which is a subset of the update callback + this.runOnDomReady = cb return } - newSrc = newSrc || this.props.src - - if (isSourceAboutUrl(newSrc)) { - newSrc = getTargetAboutUrl(newSrc) - } - // Create the webview dynamically because React doesn't whitelist all // of the attributes we need if (this.shouldCreateWebview()) { @@ -207,7 +197,7 @@ class Frame extends React.Component { if (!this.props.guestInstanceId || !this.webview.attachGuest(this.props.guestInstanceId)) { // The partition is guaranteed to be initialized by now by the browser process this.webview.setAttribute('partition', frameStateUtil.getPartition(this.frame)) - this.webview.setAttribute('src', newSrc) + this.webview.setAttribute('src', this.frame.get('location')) } domUtil.appendChild(this.webviewContainer, this.webview) } else { @@ -216,8 +206,8 @@ class Frame extends React.Component { } onPropsChanged (prevProps = {}) { - if (this.props.isActive && !prevProps.isActive) { - windowActions.setActiveFrame(this.frame) + if (this.props.isTabActive && !prevProps.isTabActive) { + windowActions.activeFrameChanged(this.frame) } if (this.props.tabIndex !== prevProps.tabIndex) { this.webview.setTabIndex(this.props.tabIndex) @@ -228,8 +218,8 @@ class Frame extends React.Component { } componentDidMount () { - if (this.props.isActive) { - windowActions.setActiveFrame(this.frame) + if (this.props.isTabActive) { + windowActions.activeFrameChanged(this.frame) } this.updateWebview(this.onPropsChanged) if (this.props.activeShortcut) { @@ -318,22 +308,7 @@ class Frame extends React.Component { this.expireContentSettings(prevOrigin) } - if (this.props.src !== prevProps.src) { - this.updateWebview(cb, prevProps) - } else if (this.shouldCreateWebview()) { - // plugin/insecure-content allow state has changed. recreate with the current - // location, not the src. - this.updateWebview(cb, prevProps, this.props.location) - } else { - if (this.runOnDomReady) { - // there is already a callback waiting for did-attach - // so replace it with this callback because it might be a - // mount callback which is a subset of the update callback - this.runOnDomReady = cb - } else { - cb(prevProps) - } - } + this.updateWebview(cb, prevProps) } handleShortcut () { @@ -571,18 +546,12 @@ class Frame extends React.Component { this.webview.addEventListener('mouseleave', (e) => { windowActions.onFrameMouseLeave(this.props.tabId) }) - this.webview.addEventListener('destroyed', (e) => { + this.webview.addEventListener('will-destroy', (e) => { if (this.frame.isEmpty()) { return } this.onCloseFrame(this.frame, true) }) - this.webview.addEventListener('close', () => { - if (this.frame.isEmpty()) { - return - } - this.onCloseFrame(this.frame) - }) this.webview.addEventListener('page-favicon-updated', (e) => { if (this.frame.isEmpty()) { return @@ -985,6 +954,7 @@ class Frame extends React.Component { props.isFullScreen = frame.get('isFullScreen') props.isPreview = frame.get('key') === currentWindow.get('previewFrameKey') props.isActive = frameStateUtil.isFrameKeyActive(currentWindow, frame.get('key')) + props.isTabActive = tab ? tabState.isActive(state, tabId) : false props.showFullScreenWarning = frame.get('showFullScreenWarning') props.location = frame.get('location') props.hrefPreview = frame.get('hrefPreview') diff --git a/js/components/main.js b/js/components/main.js index f911843b2c7..dc773e3d2b1 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -377,9 +377,14 @@ class Main extends ImmutableComponent { })) }) - ipc.on(messages.SHORTCUT_CLOSE_FRAME, (e, i) => typeof i !== 'undefined' && i !== null - ? windowActions.closeFrame(frameStateUtil.getFrameByKey(self.props.windowState, i)) - : windowActions.closeFrame(frameStateUtil.getActiveFrame(this.props.windowState))) + ipc.on(messages.SHORTCUT_CLOSE_FRAME, (e, i) => { + const frame = i == null + ? frameStateUtil.getActiveFrame(this.props.windowState) + : frameStateUtil.getFrameByKey(self.props.windowState, i) + if (frame) { + appActions.tabCloseRequested(frame.get('tabId')) + } + }) ipc.on(messages.SHORTCUT_UNDO_CLOSED_FRAME, () => windowActions.undoClosedFrame()) ipc.on(messages.SHORTCUT_CLOSE_OTHER_FRAMES, (e, key, isCloseRight, isCloseLeft) => { @@ -391,7 +396,9 @@ class Main extends ImmutableComponent { frameStateUtil.getFrames(self.props.windowState).forEach((frame, i) => { if (!frame.get('pinnedLocation') && ((i < currentIndex && isCloseLeft) || (i > currentIndex && isCloseRight))) { - windowActions.closeFrame(frame) + if (frame) { + appActions.tabCloseRequested(frame.get('tabId')) + } } }) }) @@ -405,12 +412,6 @@ class Main extends ImmutableComponent { }) const self = this - ipc.on(messages.SHORTCUT_SET_ACTIVE_FRAME_BY_INDEX, (e, i) => - windowActions.setActiveFrame(frameStateUtil.getFrameByDisplayIndex(self.props.windowState, i))) - - ipc.on(messages.SHORTCUT_SET_ACTIVE_FRAME_TO_LAST, () => - windowActions.setActiveFrame(self.props.windowState.getIn(['frames', frameStateUtil.getFrames(self.props.windowState).size - 1]))) - ipc.on(messages.BLOCKED_RESOURCE, (e, blockType, details) => { const frameProps = frameStateUtil.getFrameByTabId(self.props.windowState, details.tabId) frameProps && windowActions.setBlockedBy(frameProps, blockType, details.url) diff --git a/js/components/window.js b/js/components/window.js index 309d5b5f4eb..f0c5a901ba8 100644 --- a/js/components/window.js +++ b/js/components/window.js @@ -42,24 +42,23 @@ class Window extends React.Component { } componentWillMount () { - if (!this.props.initWindowState || this.props.initWindowState.frames.length === 0) { - if (this.props.frames.length === 0) { - appActions.createTabRequested({}) + const activeFrameKey = this.state.immutableData.windowState.get('activeFrameKey') + this.props.frames.forEach((frame, i) => { + if (frame.guestInstanceId) { + appActions.newWebContentsAdded(getCurrentWindowId(), frame) } else { - this.props.frames.forEach((frame, i) => { - if (frame.guestInstanceId) { - appActions.newWebContentsAdded(getCurrentWindowId(), frame) - return - } - appActions.createTabRequested({ - url: frame.location, - partitionNumber: frame.partitionNumber, - isPrivate: frame.isPrivate, - active: i === 0 - }) + appActions.createTabRequested({ + url: frame.location || frame.src || frame.provisionalLocation, + partitionNumber: frame.partitionNumber, + isPrivate: frame.isPrivate, + active: activeFrameKey ? frame.key === activeFrameKey : true, + discarded: frame.unloaded, + title: frame.title, + faviconUrl: frame.icon, + index: i }) } - } + }) } render () { diff --git a/js/constants/appConstants.js b/js/constants/appConstants.js index fbe9a9bacf6..7589c26c5a1 100644 --- a/js/constants/appConstants.js +++ b/js/constants/appConstants.js @@ -44,7 +44,6 @@ const appConstants = { APP_UPDATE_PUBLISHER_INFO: _, APP_SHOW_NOTIFICATION: _, /** @param {Object} detail */ APP_HIDE_NOTIFICATION: _, /** @param {string} message */ - APP_CLEAR_NOTIFICATIONS: _, /** @param {string} origin */ APP_ADD_WORD: _, /** @param {string} word, @param {boolean} learn */ APP_SET_DICTIONARY: _, /** @param {string} locale */ APP_BACKUP_KEYS: _, @@ -65,8 +64,9 @@ const appConstants = { APP_FRAME_CHANGED: _, APP_TAB_CREATED: _, APP_TAB_MOVED: _, + APP_TAB_ACTIVATE_REQUESTED: _, + APP_TAB_CLOSE_REQUESTED: _, APP_CREATE_TAB_REQUESTED: _, - APP_MAYBE_CREATE_TAB_REQUESTED: _, APP_LOAD_URL_REQUESTED: _, APP_LOAD_URL_IN_ACTIVE_TAB_REQUESTED: _, APP_SIMPLE_SHARE_ACTIVE_TAB_REQUESTED: _, @@ -96,7 +96,6 @@ const appConstants = { APP_SHUTTING_DOWN: _, APP_CLIPBOARD_TEXT_UPDATED: _, APP_TAB_TOGGLE_DEV_TOOLS: _, - APP_ACTIVE_WEB_CONTENTS_CLOSED: _, APP_TAB_CLONED: _, APP_SET_OBJECT_ID: _, APP_CREATE_SYNC_CACHE: _, diff --git a/js/constants/windowConstants.js b/js/constants/windowConstants.js index da8ef1d55db..7ffe9537f63 100644 --- a/js/constants/windowConstants.js +++ b/js/constants/windowConstants.js @@ -6,11 +6,10 @@ const mapValuesByKeys = require('../lib/functional').mapValuesByKeys const _ = null const windowConstants = { + WINDOW_ACTIVE_FRAME_CHANGED: _, WINDOW_SET_NAVBAR_INPUT: _, - WINDOW_UNLOADED_TAB_CREATED: _, WINDOW_CLOSE_FRAME: _, WINDOW_CLOSE_FRAMES: _, - WINDOW_SET_ACTIVE_FRAME: _, WINDOW_SET_FOCUSED_FRAME: _, WINDOW_SET_PREVIEW_FRAME: _, WINDOW_SET_PREVIEW_TAB_PAGE_INDEX: _, diff --git a/js/dispatcher/appDispatcher.js b/js/dispatcher/appDispatcher.js index 8cff2ec7687..dd1b843921d 100644 --- a/js/dispatcher/appDispatcher.js +++ b/js/dispatcher/appDispatcher.js @@ -174,21 +174,26 @@ if (process.type === 'browser') { const dispatchEventPayload = (event, payload) => { let queryInfo = payload.queryInfo || payload.frameProps || (payload.queryInfo = {}) queryInfo = queryInfo.toJS ? queryInfo.toJS() : queryInfo - if (!event.sender.isDestroyed() && event.sender.hostWebContents) { - // received from an extension - // only extension messages will have a hostWebContents - // because other messages come from the main window - - // default to the windowId of the hostWebContents - if (!queryInfo.windowId) { - let win = require('electron').BrowserWindow.fromWebContents(event.sender.hostWebContents) - if (!win) { - return + let sender = event.sender + if (!sender.isDestroyed()) { + const hostWebContents = sender.hostWebContents + sender = hostWebContents || sender + const win = require('electron').BrowserWindow.fromWebContents(sender) + + if (hostWebContents) { + // received from an extension + // only extension messages will have a hostWebContents + // because other messages come from the main window + + // default to the windowId of the hostWebContents + if (!queryInfo.windowId && win) { + queryInfo.windowId = win.id } - queryInfo.windowId = win.id } - // add queryInfo if we only had frameProps before payload.queryInfo = queryInfo + if (win) { + payload.senderWindowId = win.id + } payload.senderTabId = event.sender.getId() } appDispatcher.dispatch(payload) diff --git a/js/state/frameStateUtil.js b/js/state/frameStateUtil.js index d99b6bf13cd..60aa8e8e355 100644 --- a/js/state/frameStateUtil.js +++ b/js/state/frameStateUtil.js @@ -239,14 +239,16 @@ function setActiveFrameKey (windowState, activeFrameKey) { }) } -function makeNextFrameActive (windowState) { +function getNextFrame (windowState) { const activeFrameIndex = getActiveFrameDisplayIndex(windowState) - return setActiveFrameDisplayIndex(windowState, (activeFrameIndex + 1) % windowState.get('frames').size) + const index = (activeFrameIndex + 1) % windowState.get('frames').size + return getFrameByDisplayIndex(windowState, index) } -function makePrevFrameActive (windowState) { +function getPreviousFrame (windowState) { const activeFrameIndex = getActiveFrameDisplayIndex(windowState) - return setActiveFrameDisplayIndex(windowState, (windowState.get('frames').size + activeFrameIndex - 1) % windowState.get('frames').size) + const index = (windowState.get('frames').size + activeFrameIndex - 1) % windowState.get('frames').size + return getFrameByDisplayIndex(windowState, index) } /** @@ -503,27 +505,6 @@ function addFrame (windowState, tabs, frameOpts, newKey, partitionNumber, active } } -/** - * Undoes a frame close and inserts it at the last index - * @return Immutable top level application state ready to merge back in - */ -function undoCloseFrame (windowState, closedFrames) { - if (closedFrames.size === 0) { - return {} - } - const closedFrame = closedFrames.last() - const insertIndex = closedFrame.get('closedAtIndex') - return { - closedFrames: closedFrames.pop(), - tabs: windowState.get('tabs').splice(insertIndex, 0, tabFromFrame(closedFrame)), - frames: windowState.get('frames').splice(insertIndex, 0, - closedFrame - .delete('guestInstanceId') - .set('src', closedFrame.get('location'))), - activeFrameKey: closedFrame.get('key') - } -} - /** * Removes a frame specified by frameProps * @return Immutable top level application state ready to merge back in @@ -604,90 +585,6 @@ function removeFrame (frames, tabs, closedFrames, frameProps, activeFrameKey, fr frames: newFrames } } -/** - * Removes a frames specified by framePropsList - * @return Immutable top level application state ready to merge back in - */ -function removeFrames (frames, tabs, closedFrames, framePropsList, activeFrameRemoved, activeFrameKey, closeAction) { - function getLastTab (newFrames) { - const sorted = newFrames - .filter((frame) => !frame.get('pinnedLocation')) - .sortBy((item) => item.get('key')) - - return (sorted.size === 0) ? 0 : sorted.last().get('key') - } - - function getLastActiveTab (newFrames) { - const sorted = newFrames - .filter((frame) => !frame.get('pinnedLocation')) - .sortBy((item) => item.get('lastAccessedTime') || 0) - - return (sorted.size === 0) ? 0 : sorted.last().get('key') - } - - let newFrames = makeImmutable(frames) - let newTabs = makeImmutable(tabs) - - framePropsList.forEach((frameProps) => { - if (!frameProps.get('isPrivate') && frameProps.get('location') !== 'about:newtab') { - frameProps = frameProps.set('isFullScreen', false) - closedFrames = closedFrames.push(frameProps) - if (frameProps.get('thumbnailBlob')) { - window.URL.revokeObjectURL(frameProps.get('thumbnailBlob')) - } - if (closedFrames.size > config.maxClosedFrames) { - closedFrames = closedFrames.shift() - } - } - - let framePropsIndex = getFramePropsIndex(newFrames, frameProps) - newFrames = newFrames.splice(framePropsIndex, 1) - newTabs = newTabs.splice(framePropsIndex, 1) - }) - - // return last non pinned frame index if active frame was removed - if (activeFrameRemoved) { - switch (closeAction) { - case tabCloseAction.LAST_ACTIVE: - activeFrameKey = getLastActiveTab(newFrames) - break - default: - activeFrameKey = getLastTab(newFrames) - break - } - } - - return { - previewFrameKey: null, - activeFrameKey, - closedFrames, - tabs: newTabs, - frames: newFrames - } -} - -/** - * Removes all but the specified frameProps - * @return Immutable top level application state ready to merge back in - */ -function removeOtherFrames (frames, tabs, closedFrames, frameProps) { - closedFrames = closedFrames.concat(frames.filter((currentFrameProps) => !currentFrameProps.get('isPrivate') && currentFrameProps.get('key') !== frameProps.get('key'))) - .take(config.maxClosedFrames) - closedFrames.forEach((currentFrameProps) => { - if (currentFrameProps.get('thumbnailBlob')) { - window.URL.revokeObjectURL(currentFrameProps.get('thumbnailBlob')) - } - }) - - frames = Immutable.fromJS([frameProps]) - tabs = tabFromFrame(frames.get(0)) - return { - activeFrameKey: frameProps.get('key'), - closedFrames, - tabs, - frames - } -} function getFrameTabPageIndex (frames, frameProps, tabsPerTabPage) { const index = getFramePropsIndex(frames, frameProps) @@ -776,8 +673,8 @@ module.exports = { setActiveFrameDisplayIndex, setActiveFrameIndex, setActiveFrameKey, - makeNextFrameActive, - makePrevFrameActive, + getNextFrame, + getPreviousFrame, getFramePropValue, getFramePropPath, findIndexForFrameKey, @@ -788,10 +685,7 @@ module.exports = { getPartition, getPartitionFromNumber, addFrame, - undoCloseFrame, removeFrame, - removeFrames, - removeOtherFrames, tabFromFrame, frameOptsFromFrame, getFrameKeyByTabId, diff --git a/js/stores/appStore.js b/js/stores/appStore.js index 93f051c2e61..efff4de46af 100644 --- a/js/stores/appStore.js +++ b/js/stores/appStore.js @@ -42,6 +42,7 @@ const basicAuthState = require('../../app/common/state/basicAuthState') const extensionState = require('../../app/common/state/extensionState') const aboutNewTabState = require('../../app/common/state/aboutNewTabState') const aboutHistoryState = require('../../app/common/state/aboutHistoryState') +const tabState = require('../../app/common/state/tabState') const isDarwin = process.platform === 'darwin' const isWindows = process.platform === 'win32' @@ -199,18 +200,28 @@ const createWindow = (action) => { // initialize frames state let frames = [] - if (frameOpts && Object.keys(frameOpts).length > 0) { - if (frameOpts.forEach) { - frames = frameOpts - } else { - frames.push(frameOpts) - } - } else if (startupSetting === 'homePage' && homepageSetting) { - frames = homepageSetting.split('|').map((homepage) => { - return { - location: homepage + if (action.restoredState) { + frames = action.restoredState.frames + action.restoredState.frames = [] + action.restoredState.tabs = [] + } else { + if (frameOpts && Object.keys(frameOpts).length > 0) { + if (frameOpts.forEach) { + frames = frameOpts + } else { + frames.push(frameOpts) } - }) + } else if (startupSetting === 'homePage' && homepageSetting) { + frames = homepageSetting.split('|').map((homepage) => { + return { + location: homepage + } + }) + } + } + + if (frames.length === 0) { + frames = [{}] } if (windowState.ui && windowState.ui.isMaximized) { @@ -400,7 +411,8 @@ const handleAppAction = (action) => { } // maintain backwards compatibility for now by adding an additional param for immutableAction - appState = applyReducers(appState, action, makeImmutable(action)) + const immutableAction = makeImmutable(action) + appState = applyReducers(appState, action, immutableAction) switch (action.actionType) { case appConstants.APP_SET_STATE: @@ -618,10 +630,22 @@ const handleAppAction = (action) => { return notification.get('message') === action.message })) break - case appConstants.APP_CLEAR_NOTIFICATIONS: - appState = appState.set('notifications', appState.get('notifications').filterNot((notification) => { - return notification.get('frameOrigin') === action.origin - })) + case appConstants.APP_TAB_CLOSE_REQUESTED: + const tabValue = tabState.getByTabId(appState, immutableAction.get('tabId')) + if (!tabValue) { + break + } + const origin = siteUtil.getOrigin(tabValue.get('url')) + + if (origin) { + const tabsInOrigin = tabState.getTabs(appState).find((tabValue) => + siteUtil.getOrigin(tabValue.get('url')) === origin && tabValue.get('tabId') !== immutableAction.get('tabId')) + if (!tabsInOrigin) { + appState = appState.set('notifications', appState.get('notifications').filterNot((notification) => { + return notification.get('frameOrigin') === origin + })) + } + } break case appConstants.APP_ADD_WORD: let listType = 'ignoredWords' diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index 8f1f519f945..d3fb9a37c8b 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -362,18 +362,26 @@ const doAction = (action) => { loading: false }) break - case windowConstants.WINDOW_UNLOADED_TAB_CREATED: - action.frameOpts.unloaded = true - newFrame(action.frameOpts, action.openInForeground) - break case windowConstants.WINDOW_UNDO_CLOSED_FRAME: - windowState = windowState.merge(frameStateUtil.undoCloseFrame(windowState, windowState.get('closedFrames'))) - focusWebview(activeFrameStatePath(windowState)) + { + const closedFrames = windowState.get('closedFrames') + if (closedFrames.size === 0) { + break + } + const frame = closedFrames.last() + windowState = windowState.set('closedFrames', closedFrames.pop()) + appActions.createTabRequested({ + url: frame.get('location'), + partitionNumber: frame.get('partitionNumber'), + active: true, + index: frame.get('closedAtIndex') + }) + } break case windowConstants.WINDOW_CLEAR_CLOSED_FRAMES: windowState = windowState.set('closedFrames', new Immutable.List()) break - case windowConstants.WINDOW_SET_ACTIVE_FRAME: + case windowConstants.WINDOW_ACTIVE_FRAME_CHANGED: if (!action.frameProps) { break } @@ -756,16 +764,32 @@ const doAction = (action) => { } } +ipc.on(messages.SHORTCUT_SET_ACTIVE_FRAME_BY_INDEX, (e, i) => { + const frameProps = frameStateUtil.getFrameByDisplayIndex(windowState, i) + if (frameProps) { + appActions.tabActivateRequested(frameProps.get('tabId')) + } +}) + +ipc.on(messages.SHORTCUT_SET_ACTIVE_FRAME_TO_LAST, () => { + const frameProps = windowState.getIn(['frames', frameStateUtil.getFrames(windowState).size - 1]) + if (frameProps) { + appActions.tabActivateRequested(frameProps.get('tabId')) + } +}) + ipc.on(messages.SHORTCUT_NEXT_TAB, () => { - windowState = frameStateUtil.makeNextFrameActive(windowState) - windowState = updateTabPageIndex(windowState, frameStateUtil.getActiveFrame(windowState)) - emitChanges() + const frame = frameStateUtil.getNextFrame(windowState) + if (frame && frame.get('tabId') !== -1) { + appActions.tabActivateRequested(frame.get('tabId')) + } }) ipc.on(messages.SHORTCUT_PREV_TAB, () => { - windowState = frameStateUtil.makePrevFrameActive(windowState) - windowState = updateTabPageIndex(windowState, frameStateUtil.getActiveFrame(windowState)) - emitChanges() + const frame = frameStateUtil.getPreviousFrame(windowState) + if (frame && frame.get('tabId') !== -1) { + appActions.tabActivateRequested(frame.get('tabId')) + } }) ipc.on(messages.SHORTCUT_OPEN_CLEAR_BROWSING_DATA_PANEL, (e) => { diff --git a/test/lib/brave.js b/test/lib/brave.js index 911c0b54e57..c87ff33a3b3 100644 --- a/test/lib/brave.js +++ b/test/lib/brave.js @@ -529,13 +529,6 @@ var exports = { }) }) - this.app.client.addCommand('unloadedTabCreated', function (frameOpts, active) { - logVerbose('unloadedTabCreated()') - return this.execute(function (frameOpts, active) { - return devTools('electron').testData.windowActions.unloadedTabCreated(frameOpts, active) - }, frameOpts, active) - }) - this.app.client.addCommand('waitForInputText', function (selector, input) { logVerbose('waitForInputText("' + selector + '", "' + input + '")') return this @@ -610,7 +603,7 @@ var exports = { return this.waitForTab({index}).getAppState().then((val) => { const tab = val.value.tabs.find((tab) => tab.index === index) return this.execute(function (tabId) { - devTools('appActions').tabClosed({tabId}) + devTools('appActions').tabCloseRequested(tabId) }, tab.tabId) }) }) diff --git a/test/tab-components/tabTest.js b/test/tab-components/tabTest.js index 996ecf6ddbe..3e625a91132 100644 --- a/test/tab-components/tabTest.js +++ b/test/tab-components/tabTest.js @@ -246,8 +246,9 @@ describe('tab tests', function () { yield this.app.client .waitForBrowserWindow() .windowByUrl(Brave.browserWindowUrl) - .unloadedTabCreated({ - location: this.page1 + .newTab({ + location: this.page1, + discarded: true }, false) .waitForElementCount('[data-test-id="tab"]', 2) // This ensures it's actually unloaded diff --git a/test/unit/app/browser/reducers/sitesReducerTest.js b/test/unit/app/browser/reducers/sitesReducerTest.js index 635e9f0e7dd..bf597fe60dd 100644 --- a/test/unit/app/browser/reducers/sitesReducerTest.js +++ b/test/unit/app/browser/reducers/sitesReducerTest.js @@ -6,6 +6,7 @@ const sinon = require('sinon') const appConstants = require('../../../../../js/constants/appConstants') const siteTags = require('../../../../../js/constants/siteTags') +const { makeImmutable } = require('../../../../../app/common/state/immutableUtil') require('../../../braveUnit') const initState = Immutable.fromJS({ @@ -41,7 +42,7 @@ describe('sitesReducerTest', function () { clearDataDetail: {browserHistory: true} } this.clearHistory = sinon.stub(this.fakeFiltering, 'clearHistory') - this.state = sitesReducer(initState, this.action) + this.state = sitesReducer(initState, this.action, makeImmutable(this.action)) }) after(function () { diff --git a/test/unit/app/browser/reducers/tabsReducerTest.js b/test/unit/app/browser/reducers/tabsReducerTest.js index a182327fd75..d9448cb0175 100644 --- a/test/unit/app/browser/reducers/tabsReducerTest.js +++ b/test/unit/app/browser/reducers/tabsReducerTest.js @@ -131,7 +131,7 @@ describe('tabsReducer', function () { }) }) - describe.skip('WINDOW_SET_ACTIVE_FRAME', function () { + describe.skip('APP_TAB_ACTIVATED', function () { it('sets the frame as active', function () { // TODO }) @@ -143,30 +143,6 @@ describe('tabsReducer', function () { }) }) - describe('APP_ACTIVE_WEB_CONTENTS_CLOSED', function () { - const action = { - actionType: appConstants.APP_ACTIVE_WEB_CONTENTS_CLOSED - } - afterEach(function () { - this.tabsAPI.toggleDevTools.reset() - this.tabsAPI.closeTab.reset() - this.tabsAPI.moveTo.reset() - this.tabsAPI.isDevToolsFocused.restore() - }) - it('closes devtools when opened and focused', function () { - this.isDevToolsFocused = sinon.stub(this.tabsAPI, 'isDevToolsFocused', () => true) - tabsReducer(this.state, action) - assert(this.tabsAPI.toggleDevTools.withArgs(this.state, 1).calledOnce) - assert(this.tabsAPI.closeTab.notCalled) - }) - it('closes tab when tab is focused with no devtools', function () { - this.isDevToolsFocused = sinon.stub(this.tabsAPI, 'isDevToolsFocused', () => false) - tabsReducer(this.state, action) - assert(this.tabsAPI.toggleDevTools.notCalled) - assert(this.tabsAPI.closeTab.withArgs(this.state, 1).calledOnce) - }) - }) - describe.skip('APP_LOAD_URL_REQUESTED', function () { it('loads the specified URL', function () { // TODO diff --git a/test/unit/app/browser/tabsTest.js b/test/unit/app/browser/tabsTest.js index 38f9d6f4537..c36fbe99623 100644 --- a/test/unit/app/browser/tabsTest.js +++ b/test/unit/app/browser/tabsTest.js @@ -110,7 +110,7 @@ describe('tabs API', function () { describe('toggleDevTools', function () { it('opens dev tools if closed', function () { - tabs.toggleDevTools(Immutable.Map(), 1) + tabs.toggleDevTools(1) assert(this.tabWithDevToolsClosed.openDevTools.calledOnce) assert(this.tabWithDevToolsClosed.closeDevTools.notCalled) // Also check it leaves other tabs alone @@ -120,7 +120,7 @@ describe('tabs API', function () { assert(this.tabWithDevToolsOpenedAndFocused.closeDevTools.notCalled) }) it('closes dev tools if opened', function () { - tabs.toggleDevTools(Immutable.Map(), 2) + tabs.toggleDevTools(2) assert(this.tabWithDevToolsOpened.openDevTools.notCalled) assert(this.tabWithDevToolsOpened.closeDevTools.calledOnce) // Also check it leaves other tabs alone @@ -129,10 +129,6 @@ describe('tabs API', function () { assert(this.tabWithDevToolsOpenedAndFocused.openDevTools.notCalled) assert(this.tabWithDevToolsOpenedAndFocused.closeDevTools.notCalled) }) - it('does not modify state', function () { - const state = Immutable.Map() - assert.equal(state, tabs.toggleDevTools(state, 2)) - }) }) describe('isDevToolsFocused', function () { it('returns false if devtools are opened but not focused', function () { @@ -166,10 +162,9 @@ describe('tabs API', function () { indexByFrameKey: undefined, prependIndexByFrameKey: undefined } - const newState = tabs.moveTo(state, frameOpts.tabId, frameOpts, this.browserOpts, state.getIn(['dragData', 'dropWindowId'])) + tabs.moveTo(state, frameOpts.tabId, frameOpts, this.browserOpts, state.getIn(['dragData', 'dropWindowId'])) assert.equal(this.newWindowSpy.calledOnce, true) assert.equal(this.newWebContentsAddedSpy.notCalled, true) - assert(Immutable.is(newState, state)) }) it('moves tab to an existing window', function () { const state = this.state.set('dragData', Immutable.fromJS({ @@ -186,10 +181,9 @@ describe('tabs API', function () { indexByFrameKey: undefined, prependIndexByFrameKey: undefined } - const newState = tabs.moveTo(state, frameOpts.tabId, frameOpts, this.browserOpts, state.getIn(['dragData', 'dropWindowId'])) + tabs.moveTo(state, frameOpts.tabId, frameOpts, this.browserOpts, state.getIn(['dragData', 'dropWindowId'])) assert.equal(this.newWindowSpy.notCalled, true) assert.equal(this.newWebContentsAddedSpy.calledOnce, true) - assert(Immutable.is(newState, state)) }) it('does not move pinned tabs', function () { const state = this.state.set('dragData', Immutable.fromJS({ @@ -206,10 +200,9 @@ describe('tabs API', function () { indexByFrameKey: undefined, prependIndexByFrameKey: undefined } - const newState = tabs.moveTo(state, frameOpts.tabId, frameOpts, this.browserOpts, state.getIn(['dragData', 'dropWindowId'])) + tabs.moveTo(state, frameOpts.tabId, frameOpts, this.browserOpts, state.getIn(['dragData', 'dropWindowId'])) assert.equal(this.newWindowSpy.notCalled, true) assert.equal(this.newWebContentsAddedSpy.notCalled, true) - assert(Immutable.is(newState, state)) }) it('does not move pinned tabs to alt window', function () { const state = this.state.set('dragData', Immutable.fromJS({ @@ -226,10 +219,9 @@ describe('tabs API', function () { indexByFrameKey: undefined, prependIndexByFrameKey: undefined } - const newState = tabs.moveTo(state, frameOpts.tabId, frameOpts, this.browserOpts, state.getIn(['dragData', 'dropWindowId'])) + tabs.moveTo(state, frameOpts.tabId, frameOpts, this.browserOpts, state.getIn(['dragData', 'dropWindowId'])) assert.equal(this.newWindowSpy.notCalled, true) assert.equal(this.newWebContentsAddedSpy.notCalled, true) - assert(Immutable.is(newState, state)) }) it('does not move single tab windows into new window', function () { const state = this.state.set('dragData', Immutable.fromJS({ @@ -246,10 +238,9 @@ describe('tabs API', function () { indexByFrameKey: undefined, prependIndexByFrameKey: undefined } - const newState = tabs.moveTo(state, frameOpts.tabId, frameOpts, this.browserOpts, state.getIn(['dragData', 'dropWindowId'])) + tabs.moveTo(state, frameOpts.tabId, frameOpts, this.browserOpts, state.getIn(['dragData', 'dropWindowId'])) assert.equal(this.newWindowSpy.notCalled, true) assert.equal(this.newWebContentsAddedSpy.notCalled, true) - assert(Immutable.is(newState, state)) }) it('allows combining single tab into alt window', function () { const state = this.state.set('dragData', Immutable.fromJS({ @@ -266,10 +257,9 @@ describe('tabs API', function () { indexByFrameKey: undefined, prependIndexByFrameKey: undefined } - const newState = tabs.moveTo(state, frameOpts.tabId, frameOpts, this.browserOpts, state.getIn(['dragData', 'dropWindowId'])) + tabs.moveTo(state, frameOpts.tabId, frameOpts, this.browserOpts, state.getIn(['dragData', 'dropWindowId'])) assert.equal(this.newWindowSpy.notCalled, true) assert.equal(this.newWebContentsAddedSpy.calledOnce, true) - assert(Immutable.is(newState, state)) }) }) diff --git a/test/unit/app/common/state/tabStateTest.js b/test/unit/app/common/state/tabStateTest.js index 97cf3bba44a..1399e448431 100644 --- a/test/unit/app/common/state/tabStateTest.js +++ b/test/unit/app/common/state/tabStateTest.js @@ -33,7 +33,7 @@ const shouldValidateId = function (cb) { ) }) - it('throws an AssertionError if tabId < 1 and !== -1', function () { + it('throws an AssertionError if tabId < 1 and !== -1 and !== -2', function () { assert.throws( () => { cb(0) @@ -41,6 +41,12 @@ const shouldValidateId = function (cb) { AssertionError ) assert.throws( + () => { + cb(-3) + }, + AssertionError + ) + assert.doesNotThrow( () => { cb(-2) }, @@ -316,33 +322,6 @@ describe('tabState unit tests', function () { }) }) - describe('removeTab', function () { - before(function () { - this.appState = defaultAppState.set('tabs', Immutable.fromJS([ - { tabId: 1 }, - { tabId: 2 } - ])) - }) - - it('returns a new immutable state with the tab removed by `tabId`', function () { - assert.deepEqual( - tabState.removeTab(this.appState, { tabId: 2 }).get('tabs').toJS(), - [{ tabId: 1 }]) - }) - - shouldValidateTabValue((tabValue) => { - tabState.removeTab(defaultAppState, tabValue) - }) - - shouldValidateId((tabId) => { - tabState.removeTab(defaultAppState, { tabId }) - }) - - shouldValidateTabState((state) => { - tabState.removeTab(state, { tabId: 1 }) - }) - }) - describe('insertTab', function () { before(function () { this.appState = defaultAppState.set('tabs', Immutable.fromJS([ diff --git a/test/unit/state/frameStateUtilTest.js b/test/unit/state/frameStateUtilTest.js index 2c0042766f9..ab7520c6f65 100644 --- a/test/unit/state/frameStateUtilTest.js +++ b/test/unit/state/frameStateUtilTest.js @@ -284,77 +284,6 @@ describe('frameStateUtil', function () { }) }) - describe('removeFrames', function () { - let frames, tabs, closedFrames, framePropsList, activeFrameKey, activeFrameRemoved, activeFrameKeyKeep - - beforeEach(function () { - frames = Immutable.fromJS([ - { key: 2 }, - { key: 3, parentFrameKey: 2 }, - { key: 4 }, - { key: 5, pinnedLocation: 'https://twitter.com/', lastAccessedTime: 1488184050731 }, - { key: 6, lastAccessedTime: 1488184050721 }, - { key: 7, lastAccessedTime: undefined }, - { key: 8, lastAccessedTime: 1488184050711 }, - { key: 9, pinnedLocation: 'https://brave.com/' } - ]) - tabs = Immutable.fromJS([ - { key: 2 }, - { key: 3 }, - { key: 4 }, - { key: 5, pinnedLocation: 'https://twitter.com/' }, - { key: 6 }, - { key: 7 }, - { key: 8 }, - { key: 9, pinnedLocation: 'https://brave.com/' } - ]) - closedFrames = Immutable.fromJS([ - { key: 1 } - ]) - framePropsList = Immutable.fromJS([ - { key: 2 }, - { key: 3 }, - { key: 4 } - ]) - activeFrameRemoved = Immutable.fromJS({ key: 2 }) - activeFrameKey = 2 - activeFrameKeyKeep = 6 - }) - - it('removes frames from `frames`', function () { - const result = frameStateUtil.removeFrames(frames, tabs, closedFrames, framePropsList, undefined, activeFrameKeyKeep) - assert.equal(5, result.frames.size) - }) - - it('removed frames are in `closedFrame`', function () { - const result = frameStateUtil.removeFrames(frames, tabs, closedFrames, framePropsList, activeFrameRemoved, activeFrameKey) - assert.equal((framePropsList.size + closedFrames.size), result.closedFrames.size) - }) - - it('active frame is the same', function () { - const result = frameStateUtil.removeFrames(frames, tabs, closedFrames, framePropsList, undefined, activeFrameKeyKeep) - assert.equal(activeFrameKeyKeep, result.activeFrameKey) - }) - - describe('when active frame is removed', function () { - it('removes frames from `frames`', function () { - const result = frameStateUtil.removeFrames(frames, tabs, closedFrames, framePropsList, activeFrameRemoved, activeFrameKey) - const inFrames = result.frames.find((frame) => frame.get('key') === activeFrameRemoved.get('key')) - assert.equal(true, inFrames === undefined) - }) - - it('new active frame (last tab)', function () { - const result = frameStateUtil.removeFrames(frames, tabs, closedFrames, framePropsList, activeFrameRemoved, activeFrameKey) - assert.equal(8, result.activeFrameKey) - }) - - it('new active frame (last active tab)', function () { - const result = frameStateUtil.removeFrames(frames, tabs, closedFrames, framePropsList, activeFrameRemoved, activeFrameKey, tabCloseAction.LAST_ACTIVE) - assert.equal(6, result.activeFrameKey) - }) - }) - }) - describe('getFrameByLastAccessedTime', function () { let framesWithLastAccessedTime, framesWithoutLastAccessedTime, framesWithNullifiedLastAccessedTime