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

only set the activeFrameKey if the tab is already active #9219

Merged
merged 3 commits into from
Jun 4, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/browser/tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,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,
Expand Down
4 changes: 4 additions & 0 deletions app/sessionStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 1 addition & 7 deletions js/state/frameStateUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -367,15 +367,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
}

/**
Expand Down
26 changes: 17 additions & 9 deletions js/stores/windowStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ const addToHistory = (frameProps) => {
return history.slice(-10)
}

const newFrame = (state, frameOpts, openInForeground) => {
const newFrame = (state, frameOpts) => {
if (frameOpts === undefined) {
frameOpts = {}
}
Expand All @@ -114,12 +114,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
}

Expand Down Expand Up @@ -180,10 +185,13 @@ const newFrame = (state, frameOpts, openInForeground) => {
state = frameStateUtil.updateFramesInternalIndex(state, insertionIndex)

if (openInForeground) {
const activeFrame = frameStateUtil.getActiveFrame(state)
const tabId = activeFrame.get('tabId')
state = frameStateUtil.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)
}
}
Expand Down Expand Up @@ -695,7 +703,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)
Expand Down
87 changes: 86 additions & 1 deletion test/app/sessionStoreTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down Expand Up @@ -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 * () {
Expand Down
9 changes: 9 additions & 0 deletions test/lib/brave.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
Loading