diff --git a/app/browser/tabs.js b/app/browser/tabs.js index 317e80f62f3..0c075d54bb7 100644 --- a/app/browser/tabs.js +++ b/app/browser/tabs.js @@ -331,7 +331,7 @@ const api = { const frameOpts = { location, partition: newTab.session.partition, - openInForeground: !!newTabValue.get('active'), + active: !!newTabValue.get('active'), guestInstanceId: newTab.guestInstanceId, isPinned: !!newTabValue.get('pinned'), openerTabId, diff --git a/app/sessionStore.js b/app/sessionStore.js index b7aae20c1c5..e68797f478a 100644 --- a/app/sessionStore.js +++ b/app/sessionStore.js @@ -169,6 +169,10 @@ module.exports.cleanPerWindowData = (perWindowData, isShutdown) => { delete frame.adblock delete frame.noScript + // clean up any legacy frame opening props + delete frame.openInForeground + delete frame.disposition + // Guest instance ID's are not valid after restarting. // Electron won't know about them. delete frame.guestInstanceId diff --git a/js/state/frameStateUtil.js b/js/state/frameStateUtil.js index 14d7d071184..e64f739038d 100644 --- a/js/state/frameStateUtil.js +++ b/js/state/frameStateUtil.js @@ -357,15 +357,9 @@ function addFrame (state, frameOpts, newKey, partitionNumber, openInForeground, history: [] }, frameOpts)) - const result = { + return { frames: frames.splice(insertionIndex, 0, frame) } - - if (openInForeground) { - result.activeFrameKey = newKey - } - - return result } /** diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index aaabbcd4849..a6953e31ab3 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -98,7 +98,7 @@ const addToHistory = (frameProps) => { return history.slice(-10) } -const newFrame = (state, frameOpts, openInForeground) => { +const newFrame = (state, frameOpts) => { if (frameOpts === undefined) { frameOpts = {} } @@ -117,12 +117,17 @@ const newFrame = (state, frameOpts, openInForeground) => { } frameOpts.partitionNumber = frameOpts.partitionNumber || 0 - if (frameOpts.disposition) { + const active = frameOpts.active + delete frameOpts.active + delete frameOpts.openInForeground // clean up any legacy openInForeground props + let openInForeground = active + + if (openInForeground == null && frameOpts.disposition) { openInForeground = frameOpts.disposition !== 'background-tab' + delete frameOpts.disposition } - const activeFrame = frameStateUtil.getActiveFrame(state) - if (openInForeground === undefined || !activeFrame) { + if (openInForeground == null || state.get('activeFrameKey') == null) { openInForeground = true } @@ -183,10 +188,13 @@ const newFrame = (state, frameOpts, openInForeground) => { state = frameStateUtil.updateFramesInternalIndex(state, insertionIndex) if (openInForeground) { - const activeFrame = frameStateUtil.getActiveFrame(state) - const tabId = activeFrame.get('tabId') - state = updateTabPageIndex(state, activeFrame) - if (tabId) { + const tabId = frameOpts.tabId + const frame = frameStateUtil.getFrameByTabId(state, tabId) + state = frameStateUtil.updateTabPageIndex(state, frame) + if (active) { + // only set the activeFrameKey if the tab is already active + state = state.set('activeFrameKey', frame.get('key')) + } else { appActions.tabActivateRequested(tabId) } } @@ -698,7 +706,7 @@ const doAction = (action) => { action.frameOpts.tabId = tabValue.get('tabId') action.frameOpts.icon = action.frameOpts.icon || tabValue.get('favIconUrl') } - windowState = newFrame(windowState, action.frameOpts, action.frameOpts.openInForeground) + windowState = newFrame(windowState, action.frameOpts) break case windowConstants.WINDOW_FRAME_MOUSE_ENTER: windowState = windowState.setIn(['ui', 'mouseInFrame'], true) diff --git a/test/app/sessionStoreTest.js b/test/app/sessionStoreTest.js index 316e2ed5b1c..b0ba92845a5 100644 --- a/test/app/sessionStoreTest.js +++ b/test/app/sessionStoreTest.js @@ -2,7 +2,7 @@ const Brave = require('../lib/brave') const Immutable = require('immutable') -const {urlInput, navigatorBookmarked, navigatorNotBookmarked} = require('../lib/selectors') +const {urlInput, navigatorBookmarked, navigatorNotBookmarked, pinnedTabsTabs, tabsTabs} = require('../lib/selectors') const siteTags = require('../../js/constants/siteTags') const siteUtil = require('../../js/state/siteUtil') @@ -62,6 +62,91 @@ describe('sessionStore', function () { }) }) + describe('restore active tab', function () { + describe('regular tab', function () { + Brave.beforeAllServerSetup(this) + before(function * () { + this.page1Url = Brave.server.url('page1.html') + this.page2Url = Brave.server.url('page2.html') + this.activeTabSelector = '.frameWrapper.isActive webview[data-frame-key="1"][src="' + this.page1Url + '"]' + yield Brave.startApp() + yield setup(Brave.app.client) + yield Brave.app.client + .waitForBrowserWindow() + .waitForUrl(Brave.newTabUrl) + .loadUrl(this.page1Url) + .windowByUrl(Brave.browserWindowUrl) + .newTab({url: this.page2Url}) + .waitForUrl(this.page2Url) + .windowByUrl(Brave.browserWindowUrl) + .newTab({url: this.page2Url}) + .waitForUrl(this.page2Url) + .windowByUrl(Brave.browserWindowUrl) + .waitForElementCount(tabsTabs, 3) + .pinTabByIndex(2, true) + .waitForExist(pinnedTabsTabs) + .waitForElementCount(pinnedTabsTabs, 1) + .waitForElementCount(tabsTabs, 2) + .activateTabByIndex(0) + .waitForExist(this.activeTabSelector) + yield Brave.stopApp(false) + yield Brave.startApp() + yield setup(Brave.app.client) + }) + + after(function * () { + yield Brave.stopApp() + }) + + it('restores the last active tab', function * () { + yield Brave.app.client + .waitForUrl(this.page1Url) + // page2Url is lazy loaded + .waitForBrowserWindow() + .waitForExist(this.activeTabSelector) + }) + }) + + describe('pinned tab', function () { + Brave.beforeAllServerSetup(this) + before(function * () { + this.page1Url = Brave.server.url('page1.html') + this.page2Url = Brave.server.url('page2.html') + this.activeTabSelector = '.frameWrapper.isActive webview[data-frame-key="2"][src="' + this.page2Url + '"]' + yield Brave.startApp() + yield setup(Brave.app.client) + yield Brave.app.client + .waitForBrowserWindow() + .waitForUrl(Brave.newTabUrl) + .loadUrl(this.page1Url) + .windowByUrl(Brave.browserWindowUrl) + .newTab({url: this.page2Url}) + .waitForUrl(this.page2Url) + .windowByUrl(Brave.browserWindowUrl) + .pinTabByIndex(1, true) + .waitForExist(pinnedTabsTabs) + .waitForElementCount(pinnedTabsTabs, 1) + .waitForElementCount(tabsTabs, 1) + .waitForExist(this.activeTabSelector) + yield Brave.stopApp(false) + yield Brave.startApp() + yield setup(Brave.app.client) + }) + + after(function * () { + yield Brave.stopApp() + }) + + it('restores the last active pinned tab', function * () { + yield Brave.app.client + // page1Url is lazy loaded + .waitForUrl(this.page2Url) + .waitForBrowserWindow() + .waitForExist(this.activeTabSelector) + }) + }) + }) + describe('firstRunTimestamp', function () { Brave.beforeAllServerSetup(this) before(function * () { diff --git a/test/lib/brave.js b/test/lib/brave.js index 14f01991d65..7a262be3801 100644 --- a/test/lib/brave.js +++ b/test/lib/brave.js @@ -649,6 +649,15 @@ var exports = { }, createProperties) }) + this.app.client.addCommand('activateTabByIndex', function (index) { + return this.waitForTab({index}).getAppState().then((val) => { + const tab = val.value.tabs.find((tab) => tab.index === index) + return this.execute(function (tabId) { + devTools('appActions').tabActivateRequested(tabId) + }, tab.tabId) + }) + }) + /** * Adds a site to the sites list, such as a bookmarks. * diff --git a/test/unit/js/stores/windowStoreTest.js b/test/unit/js/stores/windowStoreTest.js index 0e8cde0dfca..6caf15e07d4 100644 --- a/test/unit/js/stores/windowStoreTest.js +++ b/test/unit/js/stores/windowStoreTest.js @@ -1,16 +1,18 @@ /* This Source Code Form is subject to the terms of the Mozilla Public * 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/. */ -/* global describe, before, after, it */ +/* global describe, before, beforeEach, after, afterEach, it */ const mockery = require('mockery') const assert = require('assert') -// const sinon = require('sinon') +const sinon = require('sinon') const Immutable = require('immutable') const fakeElectron = require('../../lib/fakeElectron') const windowConstants = require('../../../../js/constants/windowConstants') +const appConstants = require('../../../../js/constants/appConstants') let doAction let windowStore +let appActions require('../../braveUnit') describe('Window store unit tests', function () { @@ -28,6 +30,7 @@ describe('Window store unit tests', function () { ] before(function () { + appActions = require('../../../../js/actions/appActions.js') mockery.enable({ warnOnReplace: false, warnOnUnregistered: false, @@ -35,6 +38,7 @@ describe('Window store unit tests', function () { }) mockery.registerMock('electron', fakeElectron) mockery.registerMock('../dispatcher/appDispatcher', fakeDispatcher) + mockery.registerMock('../actions/appActions', appActions) windowStore = require('../../../../js/stores/windowStore.js') }) @@ -122,6 +126,159 @@ describe('Window store unit tests', function () { }) }) + describe('APP_NEW_WEB_CONTENTS_ADDED', function () { + let windowState + let tabIndexChangedStub + const demoWindowState = { + frames: [{ + security: { + isSecure: null + }, + src: 'https://brave.com', + lastAccessedTime: null, + guestInstanceId: 2, + partition: 'persist:default', + findDetail: { + searchString: '', + caseSensitivity: false + }, + endLoadTime: null, + navbar: { + urlbar: { + location: 'https://brave.com', + suggestions: { + selectedIndex: 0, + searchResults: [], + suggestionList: null + }, + selected: false, + focused: true, + active: false + } + }, + tabId: 8, + zoomLevel: 0, + breakpoint: 'default', + index: 1, + partitionNumber: 0, + history: [], + audioMuted: false, + startLoadTime: null, + location: 'https://brave.com', + disposition: 'background-tab', + title: 'page title goes here', + searchDetail: null, + icon: 'https://brave.com/favicon.ico', + isPrivate: false, + openerTabId: 1, + parentFrameKey: null, + loading: false, + unloaded: true, + key: 2 + }] + } + const demoAction = { + actionType: appConstants.APP_NEW_WEB_CONTENTS_ADDED, + queryInfo: { + windowId: 1 + }, + frameOpts: { + location: 'about:blank', + partition: 'persist:default', + active: true, + guestInstanceId: 8, + isPinned: false, + openerTabId: 8, + disposition: 'foreground-tab', + unloaded: false + }, + tabValue: { + audible: false, + width: 300, + active: true, + height: 300, + guestInstanceId: 8, + autoDiscardable: true, + partition: 'persist:default', + windowId: -1, + incognito: false, + canGoForward: false, + url: '', + tabId: 33, + index: -1, + status: 'complete', + highlighted: false, + partitionNumber: 0, + title: '', + pinned: false, + mutedInfo: { + muted: false, + reason: 'user' + }, + id: 33, + selected: true, + discarded: false, + canGoBack: false + } + } + + beforeEach(function () { + tabIndexChangedStub = sinon.stub(appActions, 'tabIndexChanged') + }) + + afterEach(function () { + reducers.forEach((reducer) => { + mockery.deregisterMock(reducer) + }) + + tabIndexChangedStub.restore() + }) + + describe('when tab being opened is active', function () { + before(function () { + const fakeReducer = (state, action) => { + return Immutable.fromJS(demoWindowState) + } + reducers.forEach((reducer) => { + mockery.registerMock(reducer, fakeReducer) + }) + doAction(demoAction) + + // get the updated windowState (AFTER doAction runs) + windowStore = require('../../../../js/stores/windowStore.js') + windowState = windowStore.getState() + }) + + it('sets activeFrameKey', function () { + assert(windowState.get('activeFrameKey')) + }) + }) + + describe('when tab being opened is not active', function () { + before(function () { + const newAction = Object.assign(demoAction, {}) + newAction.frameOpts.active = false + newAction.tabValue.active = false + + const fakeReducer = (state, action) => { + return Immutable.fromJS(demoWindowState) + } + reducers.forEach((reducer) => { + mockery.registerMock(reducer, fakeReducer) + }) + doAction(newAction) + + // get the updated windowState (AFTER doAction runs) + windowStore = require('../../../../js/stores/windowStore.js') + windowState = windowStore.getState() + }) + + it('does not set activeFrameKey', function () { + assert.equal(windowState.get('activeFrameKey'), undefined) + }) + }) + }) + // TODO: add your tests if you modify windowStore.js :) }) })