Bug 1460221 - fix newtab placement when opened from pinned tab, r=Gijs
authorShane Caraveo <scaraveo@mozilla.com>
Thu, 10 May 2018 10:44:55 -0500
changeset 417803 444314e5f95f1dfce0917868a089c9b55f59355f
parent 417802 49db791ec3a9540b5b34bcfacf88d211140dfc4b
child 417804 0693ec09dd681c54f3e0c8c64ec4dbc4916d10a1
push id103165
push userebalazs@mozilla.com
push dateFri, 11 May 2018 09:45:25 +0000
treeherdermozilla-inbound@59a49b12b268 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1460221
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
@@ -2324,25 +2324,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);
+});