Cloned tabs appear next to the original one

js/state/frameStateUtil.js

@@ -284,7 +284,7 @@ const tabFromFrame = (frame) => {
  * Adds a frame specified by frameOpts and newKey and sets the activeFrameKey
  * @return Immutable top level application state ready to merge back in
  */
-function addFrame (frames, tabs, frameOpts, newKey, partitionNumber, activeFrameKey) {
+function addFrame (frames, tabs, frameOpts, newKey, partitionNumber, activeFrameKey, insertionIndex) {
   const url = frameOpts.location || config.defaultUrl
 
   // delayedLoadUrl is used as a placeholder when the new frame is created
@@ -352,22 +352,6 @@ function addFrame (frames, tabs, frameOpts, newKey, partitionNumber, activeFrame
     history: []
   }, frameOpts))
 
-  // Find the closest index to the current frame's index which has
-  // a different ancestor frame key.
-  let insertionIndex = findIndexForFrameKey(frames, frameOpts.parentFrameKey)
-  if (insertionIndex === -1) {
-    insertionIndex = frames.size
-  }
-  while (insertionIndex < frames.size) {
-    ++insertionIndex
-    if (!isAncestorFrameKey(frames, frames.get(insertionIndex), frameOpts.parentFrameKey)) {
-      break
-    }
-  }
-  if (isFrameKeyPinned(frames, frameOpts.parentFrameKey)) {
-    insertionIndex = 0
-  }
-
   return {
     tabs: tabs.splice(insertionIndex, 0, tabFromFrame(frame)),
     frames: frames.splice(insertionIndex, 0, frame),
@@ -505,7 +489,9 @@ function getFrameTabPageIndex (frames, frameProps, tabsPerTabPage) {
 module.exports = {
   query,
   find,
+  isAncestorFrameKey,
   isFrameKeyActive,
+  isFrameKeyPinned,
   getFrameIndex,
   getFrameDisplayIndex,
   getActiveFrameIndex,

js/stores/windowStore.js

@@ -154,7 +154,9 @@ const addToHistory = (frameProps) => {
   return history.slice(-10)
 }
 
-const newFrame = (frameOpts, openInForeground) => {
+const newFrame = (frameOpts, openInForeground, insertionIndex) => {
+  const frames = windowState.get('frames')
+
   if (frameOpts === undefined) {
     frameOpts = {}
   }
@@ -185,8 +187,32 @@ const newFrame = (frameOpts, openInForeground) => {
   } else if (frameOpts.isPartitioned) {
     nextPartitionNumber = incrementPartitionNumber()
   }
-  windowState = windowState.merge(FrameStateUtil.addFrame(windowState.get('frames'), windowState.get('tabs'), frameOpts,
-    nextKey, nextPartitionNumber, openInForeground ? nextKey : windowState.get('activeFrameKey')))
+
+  // Find the closest index to the current frame's index which has
+  // a different ancestor frame key.
+  if (insertionIndex === undefined) {
+    insertionIndex = FrameStateUtil.findIndexForFrameKey(frames, frameOpts.parentFrameKey)
+    if (insertionIndex === -1) {
+      insertionIndex = frames.size
+    } else {
+      while (insertionIndex < frames.size) {
+        ++insertionIndex
+        if (!FrameStateUtil.isAncestorFrameKey(frames, frames.get(insertionIndex), frameOpts.parentFrameKey)) {
+          break
+        }
+      }
+    }
+  }
+  if (FrameStateUtil.isFrameKeyPinned(frames, frameOpts.parentFrameKey)) {
+    insertionIndex = 0
+  }
+
+  windowState = windowState.merge(
+    FrameStateUtil.addFrame(
+      frames, windowState.get('tabs'), frameOpts,
+    nextKey, nextPartitionNumber, openInForeground ? nextKey : windowState.get('activeFrameKey'), insertionIndex)
+  )
+
   if (openInForeground) {
     const activeFrame = FrameStateUtil.getActiveFrame(windowState)
     updateTabPageIndex(activeFrame)
@@ -390,7 +416,8 @@ const doAction = (action) => {
       newFrame(action.frameOpts, action.openInForeground)
       break
     case WindowConstants.WINDOW_CLONE_FRAME:
-      newFrame(FrameStateUtil.cloneFrame(action.frameOpts, action.guestInstanceId), action.openInForeground)
+      let insertionIndex = FrameStateUtil.findIndexForFrameKey(windowState.get('frames'), action.frameOpts.key) + 1
+      newFrame(FrameStateUtil.cloneFrame(action.frameOpts, action.guestInstanceId), action.openInForeground, insertionIndex)
       break
     case WindowConstants.WINDOW_CLOSE_FRAME:
       // Use the frameProps we passed in, or default to the active frame

test/components/frameTest.js

@@ -150,6 +150,47 @@ describe('clone tab', function () {
     })
   })
 
+  describe('index', function () {
+    Brave.beforeAll(this)
+
+    before(function * () {
+      this.clickWithTargetPage = Brave.server.url('click_with_target.html')
+      this.page1 = Brave.server.url('page1.html')
+
+      yield setup(this.app.client)
+      yield this.app.client
+        .waitUntilWindowLoaded()
+        .windowByUrl(Brave.browserWindowUrl)
+        .waitForUrl(Brave.newTabUrl)
+        .url(this.clickWithTargetPage)
+        .waitForVisible('#name')
+        .click('#name')
+      yield this.app.client
+        .windowByUrl(Brave.browserWindowUrl)
+        .waitForExist('.tab[data-frame-key="2"]')
+      yield this.app.client
+        .ipcSend('shortcut-set-active-frame-by-index', 0)
+        .windowByUrl(Brave.browserWindowUrl)
+        .ipcSend(messages.SHORTCUT_ACTIVE_FRAME_CLONE)
+        .waitUntil(function () {
+          return this.getTabCount().then((count) => {
+            return count === 3
+          })
+        })
+    })
+
+    it('inserts after the tab to clone', function * () {
+      this.tab1 = '.tabArea:nth-child(1) .tab[data-frame-key="1"]'
+      this.tab2 = '.tabArea:nth-child(2) .tab[data-frame-key="3"]'
+      this.tab3 = '.tabArea:nth-child(3) .tab[data-frame-key="2"]'
+      yield this.app.client
+        .windowByUrl(Brave.browserWindowUrl)
+        .waitForExist(this.tab1)
+        .waitForExist(this.tab2)
+        .waitForExist(this.tab3)
+    })
+  })
+
   describe('history', function () {
     Brave.beforeAll(this)
 
@@ -283,4 +324,3 @@ function * setup (client) {
     .waitForVisible('#window')
     .waitForVisible(urlInput)
 }
-