Bug 1460221 - fix newtab placement when opened from pinned tab, r=Gijs draft
authorShane Caraveo <scaraveo@mozilla.com>
Thu, 10 May 2018 10:44:55 -0500
changeset 793656 0fd393dd70dd1d51ffec226ea6d0cd7d0ce4c83c
parent 793056 9294f67b3f3bd4a3dd898961148cecd8bfc1ce9c
push id109463
push usermixedpuppy@gmail.com
push dateThu, 10 May 2018 15:46:21 +0000
reviewersGijs
bugs1460221
milestone62.0a1
Bug 1460221 - fix newtab placement when opened from pinned tab, r=Gijs MozReview-Commit-ID: CwoFsFo0ThE
browser/base/content/tabbrowser.js
browser/base/content/test/tabs/browser.ini
browser/base/content/test/tabs/browser_pinnedTabs_clickOpen.js
--- a/browser/base/content/tabbrowser.js
+++ b/browser/base/content/tabbrowser.js
@@ -2322,25 +2322,31 @@ window._gBrowser = {
 
           let lastRelatedTab = openerTab && this._lastRelatedTabMap.get(openerTab);
           aIndex = (lastRelatedTab || openerTab || this.selectedTab)._tPos + 1;
 
           if (lastRelatedTab) {
             lastRelatedTab.owner = null;
           } else if (openerTab) {
             t.owner = openerTab;
+          }
+          // Always set related map if opener exists.
+          if (openerTab) {
             this._lastRelatedTabMap.set(openerTab, t);
           }
         } else {
           // This is intentionally past bounds, see the comment below on insertBefore.
           aIndex = this.tabs.length;
         }
       }
+      // Ensure position respectes tab pinned state.
       if (aPinned) {
         aIndex = Math.min(aIndex, this._numPinnedTabs);
+      } else {
+        aIndex = Math.max(aIndex, this._numPinnedTabs);
       }
 
       // use .item() instead of [] because dragging to the end of the strip goes out of
       // bounds: .item() returns null (so it acts like appendChild), but [] throws
       let tabAfter = this.tabs.item(aIndex);
       this.tabContainer.insertBefore(t, tabAfter);
       if (tabAfter) {
         this._updateTabsAfterInsert();
--- a/browser/base/content/test/tabs/browser.ini
+++ b/browser/base/content/test/tabs/browser.ini
@@ -23,16 +23,17 @@ skip-if = !e10s # Test only relevant for
 [browser_new_web_tab_in_file_process_pref.js]
 skip-if = !e10s # Pref and test only relevant for e10s.
 [browser_newwindow_tabstrip_overflow.js]
 [browser_opened_file_tab_navigated_to_web.js]
 [browser_new_tab_insert_position.js]
 support-files = file_new_tab_page.html
 [browser_overflowScroll.js]
 [browser_pinnedTabs.js]
+[browser_pinnedTabs_clickOpen.js]
 [browser_pinnedTabs_closeByKeyboard.js]
 [browser_positional_attributes.js]
 [browser_preloadedBrowser_zoom.js]
 [browser_reload_deleted_file.js]
 skip-if = (debug && os == 'mac') || (debug && os == 'linux' && bits == 64) #Bug 1421183, disabled on Linux/OSX for leaked windows
 [browser_tabReorder_overflow.js]
 [browser_tabswitch_updatecommands.js]
 [browser_viewsource_of_data_URI_in_file_process.js]
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/tabs/browser_pinnedTabs_clickOpen.js
@@ -0,0 +1,47 @@
+/* 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/. */
+
+"use strict";
+
+function index(tab) {
+  return Array.indexOf(gBrowser.tabs, tab);
+}
+
+async function testNewTabPosition(expectedPosition, modifiers = {}) {
+  let opening = BrowserTestUtils.waitForNewTab(gBrowser, "http://mochi.test:8888/");
+  BrowserTestUtils.synthesizeMouseAtCenter("#link", modifiers, gBrowser.selectedBrowser);
+  let newtab = await opening;
+  is(index(newtab), expectedPosition, "clicked tab is in correct position");
+  return newtab;
+}
+
+// Test that a tab opened from a pinned tab is not in the pinned region.
+add_task(async function test_pinned_content_click() {
+  let testUri = "data:text/html;charset=utf-8,<a href=\"http://mochi.test:8888/\" target=\"_blank\" id=\"link\">link</a>";
+  let tabs = [gBrowser.selectedTab, BrowserTestUtils.addTab(gBrowser, testUri), BrowserTestUtils.addTab(gBrowser)];
+  gBrowser.pinTab(tabs[1]);
+  gBrowser.pinTab(tabs[2]);
+
+  // First test new active tabs open at the start of non-pinned tabstrip.
+  await BrowserTestUtils.switchTab(gBrowser, tabs[1]);
+  let newtab1 = await testNewTabPosition(2);
+
+  await BrowserTestUtils.switchTab(gBrowser, tabs[1]);
+  let newtab2 = await testNewTabPosition(2);
+
+  gBrowser.removeTab(newtab1);
+  gBrowser.removeTab(newtab2);
+
+  // Second test new background tabs open in order.
+  let modifiers = AppConstants.platform == "macosx" ? {metaKey: true} : {ctrlKey: true};
+  await BrowserTestUtils.switchTab(gBrowser, tabs[1]);
+
+  newtab1 = await testNewTabPosition(2, modifiers);
+  newtab2 = await testNewTabPosition(3, modifiers);
+
+  gBrowser.removeTab(tabs[1]);
+  gBrowser.removeTab(tabs[2]);
+  gBrowser.removeTab(newtab1);
+  gBrowser.removeTab(newtab2);
+});