Bug 1442679 - fix insertAfterRelated for bulk opening via tabbrowser.loadTabs, r=dao
authorShane Caraveo <scaraveo@mozilla.com>
Thu, 05 Apr 2018 11:33:39 -0500
changeset 412623 d34178082ca879e801132c333de935de37280e1e
parent 412622 9d0fc1d12d9a29d989df38bf74747490a86b114f
child 412624 98d330009c839f8d4d67d84589b1a07a8497265d
push id33813
push userccoroiu@mozilla.com
push dateTue, 10 Apr 2018 21:54:55 +0000
treeherdermozilla-central@d42671c2e69d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdao
bugs1442679
milestone61.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 1442679 - fix insertAfterRelated for bulk opening via tabbrowser.loadTabs, r=dao MozReview-Commit-ID: 1E90mhjvm81
browser/base/content/tabbrowser.js
browser/base/content/test/tabs/browser_new_tab_insert_position.js
--- a/browser/base/content/tabbrowser.js
+++ b/browser/base/content/tabbrowser.js
@@ -1474,16 +1474,23 @@ window._gBrowser = {
     //    == 1              false                     YES
     //    == 1              true                      NO
     //    > 1               false/true                NO
     var multiple = aURIs.length > 1;
     var owner = multiple || aLoadInBackground ? null : this.selectedTab;
     var firstTabAdded = null;
     var targetTabIndex = -1;
 
+    // When bulk opening tabs, such as from a bookmark folder, we want to insertAfterCurrent
+    // if necessary, but we also will set the bulkOrderedOpen flag so that the bookmarks
+    // open in the same order they are in the folder.
+    if (multiple && aNewIndex < 0 && Services.prefs.getBoolPref("browser.tabs.insertAfterCurrent")) {
+      aNewIndex = this.selectedTab._tPos + 1;
+    }
+
     if (aReplace) {
       let browser;
       if (aTargetTab) {
         browser = this.getBrowserForTab(aTargetTab);
         targetTabIndex = aTargetTab._tPos;
       } else {
         browser = this.selectedBrowser;
         targetTabIndex = this.tabContainer.selectedIndex;
@@ -1506,31 +1513,33 @@ window._gBrowser = {
     } else {
       firstTabAdded = this.addTab(aURIs[0], {
         ownerTab: owner,
         skipAnimation: multiple,
         allowThirdPartyFixup: aAllowThirdPartyFixup,
         postData: aPostDatas[0],
         userContextId: aUserContextId,
         triggeringPrincipal: aTriggeringPrincipal,
+        bulkOrderedOpen: multiple,
       });
       if (aNewIndex !== -1) {
         this.moveTabTo(firstTabAdded, aNewIndex);
         targetTabIndex = firstTabAdded._tPos;
       }
     }
 
     let tabNum = targetTabIndex;
     for (let i = 1; i < aURIs.length; ++i) {
       let tab = this.addTab(aURIs[i], {
         skipAnimation: true,
         allowThirdPartyFixup: aAllowThirdPartyFixup,
         postData: aPostDatas[i],
         userContextId: aUserContextId,
         triggeringPrincipal: aTriggeringPrincipal,
+        bulkOrderedOpen: true,
       });
       if (targetTabIndex !== -1)
         this.moveTabTo(tab, ++tabNum);
     }
 
     if (firstTabAdded && !aLoadInBackground) {
       this.selectedTab = firstTabAdded;
     }
--- a/browser/base/content/test/tabs/browser_new_tab_insert_position.js
+++ b/browser/base/content/test/tabs/browser_new_tab_insert_position.js
@@ -39,40 +39,44 @@ function promiseRemoveThenUndoCloseTab(t
 // Compare the current browser tab order against the session state ordering, they should always match.
 function verifyTabState(state) {
   let newStateTabs = JSON.parse(state).windows[0].tabs;
   for (let i = 0; i < gBrowser.tabs.length; i++) {
     is(gBrowser.tabs[i].linkedBrowser.currentURI.spec, newStateTabs[i].entries[0].url, `tab pos ${i} matched ${gBrowser.tabs[i].linkedBrowser.currentURI.spec}`);
   }
 }
 
+const bulkLoad = ["http://mochi.test:8888/#5", "http://mochi.test:8888/#6",
+                "http://mochi.test:8888/#7", "http://mochi.test:8888/#8"];
+
+const sessData = {
+  windows: [{
+    tabs: [
+      {entries: [{url: "http://mochi.test:8888/#0", triggeringPrincipal_base64}]},
+      {entries: [{url: "http://mochi.test:8888/#1", triggeringPrincipal_base64}]},
+      {entries: [{url: "http://mochi.test:8888/#3", triggeringPrincipal_base64}]},
+      {entries: [{url: "http://mochi.test:8888/#4", triggeringPrincipal_base64}]},
+    ],
+  }],
+};
+const urlbarURL = "http://example.com/#urlbar";
+
 async function doTest(aInsertRelatedAfterCurrent, aInsertAfterCurrent) {
   const kDescription = "(aInsertRelatedAfterCurrent=" + aInsertRelatedAfterCurrent +
                        ", aInsertAfterCurrent=" + aInsertAfterCurrent + "): ";
 
   await SpecialPowers.pushPrefEnv({set: [
     ["browser.tabs.opentabfor.middleclick", true],
     ["browser.tabs.loadBookmarksInBackground", false],
     ["browser.tabs.insertRelatedAfterCurrent", aInsertRelatedAfterCurrent],
     ["browser.tabs.insertAfterCurrent", aInsertAfterCurrent],
   ]});
 
   let oldState = SessionStore.getBrowserState();
 
-  let sessData = {
-    windows: [{
-      tabs: [
-        {entries: [{url: "http://mochi.test:8888/#0", triggeringPrincipal_base64}]},
-        {entries: [{url: "http://mochi.test:8888/#1", triggeringPrincipal_base64}]},
-        {entries: [{url: "http://mochi.test:8888/#3", triggeringPrincipal_base64}]},
-        {entries: [{url: "http://mochi.test:8888/#4", triggeringPrincipal_base64}]},
-      ],
-    }],
-  };
-
   await promiseBrowserStateRestored(sessData);
 
   // Create a *opener* tab page which has a link to "example.com".
   let pageURL = getRootDirectory(gTestPath).replace("chrome://mochitests/content", "http://example.com");
   pageURL = `${pageURL}file_new_tab_page.html`;
   let openerTab = await BrowserTestUtils.openNewForegroundTab(gBrowser, pageURL);
   const openerTabIndex = 1;
   gBrowser.moveTabTo(openerTab, openerTabIndex);
@@ -95,17 +99,16 @@ async function doTest(aInsertRelatedAfte
     is(openTab.owner, openerTab, "tab owner is set correctly");
   }
   is(openTab.openerTab, openerTab, "opener tab is set");
 
   // Open an unrelated tab from the URL bar and test its position.
   openTabIndex = aInsertAfterCurrent ? openerTabIndex + 1 : gBrowser.tabs.length;
   openTabDescription = aInsertAfterCurrent ? "immediately to the right" : "at rightmost";
 
-  const urlbarURL = "http://example.com/#urlbar";
   gURLBar.focus();
   gURLBar.select();
   newTabPromise = BrowserTestUtils.waitForNewTab(gBrowser, urlbarURL, true);
   EventUtils.sendString(urlbarURL);
   EventUtils.synthesizeKey("KEY_Alt", { altKey: true, code: "AltLeft", type: "keydown" });
   EventUtils.synthesizeKey("KEY_Enter", { altKey: true, code: "Enter" });
   EventUtils.synthesizeKey("KEY_Alt", { altKey: false, code: "AltLeft", type: "keyup" });
   let unrelatedTab = await newTabPromise;
@@ -140,16 +143,28 @@ async function doTest(aInsertRelatedAfte
   // Remove the tab at the end, then undo.  It should reappear where it was.
   await promiseRemoveThenUndoCloseTab(gBrowser.tabs[gBrowser.tabs.length - 1]);
   verifyTabState(newState);
 
   // Remove a tab in the middle, then undo.  It should reappear where it was.
   await promiseRemoveThenUndoCloseTab(gBrowser.tabs[2]);
   verifyTabState(newState);
 
+  // Bug 1442679 - Test bulk opening with loadTabs loads the tabs in order
+
+  let loadPromises = Promise.all(bulkLoad.map(url => BrowserTestUtils.waitForNewTab(gBrowser, url, false, true)));
+  // loadTabs will insertAfterCurrent
+  let nextTab = aInsertAfterCurrent ? gBrowser.selectedTab._tPos + 1 : gBrowser.tabs.length;
+
+  gBrowser.loadTabs(bulkLoad, true);
+  await loadPromises;
+  for (let i = nextTab, j = 0; j < bulkLoad.length; i++, j++) {
+    is(gBrowser.tabs[i].linkedBrowser.currentURI.spec, bulkLoad[j], `bulkLoad tab pos ${i} matched`);
+  }
+
   // Now we want to test that positioning remains correct after a session restore.
 
   // Restore pre-test state so we can restore and test tab ordering.
   await promiseBrowserStateRestored(oldState);
 
   // Restore test state and verify it is as it was.
   await promiseBrowserStateRestored(newState);
   verifyTabState(newState);