fix newtab ordering with pinned tabs

brave · May 15, 2017 · 40937bd · 40937bd
1 parent 5283bdf
commit 40937bd
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 4 deletions.
diff --git a/app/sessionStore.js b/app/sessionStore.js
@@ -110,6 +110,8 @@ module.exports.cleanPerWindowData = (perWindowData, isShutdown) => {
   if (!perWindowData) {
     perWindowData = {}
   }
+  // delete the frame index because tabId is per-session
+  delete perWindowData.framesInternal
   // Hide the context menu when we restore.
   delete perWindowData.contextMenuDetail
   // Don't save preview frame since they are only related to hovering on a tab

diff --git a/js/state/frameStateUtil.js b/js/state/frameStateUtil.js
@@ -62,12 +62,15 @@ function getActiveFrameTabId (state) {
 }
 
 function getFrameByIndex (state, i) {
+  if (i === -1) {
+    return null
+  }
   return state.getIn(['frames', i])
 }
 
 // This will eventually go away fully when we replace frameKey by tabId
 function getFrameKeyByTabId (state, tabId) {
-  let parentFrameKey
+  let parentFrameKey = null
   const openerFrame = getFrameByTabId(state, tabId)
   if (openerFrame) {
     parentFrameKey = openerFrame.get('key')

diff --git a/test/tab-components/tabTest.js b/test/tab-components/tabTest.js
@@ -3,7 +3,7 @@
 const Brave = require('../lib/brave')
 const messages = require('../../js/constants/messages')
 const settings = require('../../js/constants/settings')
-const {urlInput, backButton, forwardButton, activeTab, activeTabTitle, activeTabFavicon, newFrameButton, notificationBar, contextMenu} = require('../lib/selectors')
+const {urlInput, backButton, forwardButton, activeTab, activeTabTitle, activeTabFavicon, newFrameButton, notificationBar, contextMenu, pinnedTabsTabs, tabsTabs} = require('../lib/selectors')
 
 describe('tab tests', function () {
   function * setup (client) {
@@ -14,8 +14,8 @@ describe('tab tests', function () {
   }
 
   describe('back forward actions', function () {
-    Brave.beforeAll(this)
-    before(function * () {
+    Brave.beforeEach(this)
+    beforeEach(function * () {
       yield setup(this.app.client)
       this.page1 = Brave.server.url('page1.html')
       this.page2 = Brave.server.url('page2.html')
@@ -117,6 +117,30 @@ describe('tab tests', function () {
   })
 
   describe('tab order', function () {
+    describe('with pinned tabs', function () {
+      Brave.beforeAll(this)
+      before(function * () {
+        this.page1 = Brave.server.url('page1.html')
+        this.page2 = Brave.server.url('page2.html')
+        yield setup(this.app.client)
+        yield this.app.client
+          .waitForExist('.tabArea:nth-of-type(1) [data-frame-key="1"]') // original newtab
+          .newTab({ url: this.page1, pinned: true })
+          .waitForExist(pinnedTabsTabs + '[data-frame-key="2"]')
+          .newTab({ url: this.page1 })
+          .waitForExist(tabsTabs + '[data-frame-key="3"]')
+          .newTab({ url: this.page2 })
+          .waitForExist(tabsTabs + '[data-frame-key="4"]')
+      })
+
+      it('sequentially by default', function * () {
+        yield this.app.client
+          .waitForExist('.tabArea:nth-of-type(2) [data-frame-key="3"]')
+          .waitForExist('webview[data-frame-key="3"][src="' + this.page1 + '"]')
+          .waitForExist('.tabArea:nth-of-type(3) [data-frame-key="4"]')
+          .waitForExist('.frameWrapper.isActive webview[data-frame-key="4"][src="' + this.page2 + '"]')
+      })
+    })
     describe('sequentially by default', function () {
       Brave.beforeAll(this)
       before(function * () {