Bug 1344749 - Expose API to customize where new tabs open, r?mixedpuppy r?dao draft
authorBob Silverberg <bsilverberg@mozilla.com>
Fri, 19 Jan 2018 12:59:53 -0500
changeset 772592 4135e7771fdefb8d66dc25477a55d7b4f1aa4b01
parent 772574 7b9da7139d94951431a148dcaf8a388640c91b27
push id103973
push usermixedpuppy@gmail.com
push dateMon, 26 Mar 2018 15:09:49 +0000
reviewersmixedpuppy, dao
bugs1344749, 933532
milestone61.0a1
Bug 1344749 - Expose API to customize where new tabs open, r?mixedpuppy r?dao This patch implements the preference "browser.tabs.insertAfterCurrent" which, when set to true, will cause all tabs (related and unrelated) to be opened next to the current tab. It also implements the browserSettings API "newTabPosition", which allows extensions to control both "browser.tabs.insertRelatedAfterCurrent", and "browser.tabs.insertAfterCurrent" via values for "afterCurrent", "relatedAfterCurrent" and "atEnd". The code for "browser.tabs.insertAfterCurrent" including the test for it is mostly taken from a patch attached to bug 933532 written by Masayuki Nakano. MozReview-Commit-ID: KQE7M2FGpc7
browser/app/profile/firefox.js
browser/base/content/tabbrowser.js
browser/base/content/test/tabs/browser.ini
browser/base/content/test/tabs/browser_new_tab_insert_position.js
browser/base/content/test/tabs/file_new_tab_page.html
browser/components/sessionstore/SessionStore.jsm
toolkit/components/extensions/ext-browserSettings.js
toolkit/components/extensions/schemas/browser_settings.json
toolkit/components/extensions/test/xpcshell/test_ext_browserSettings.js
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -460,17 +460,25 @@ pref("browser.link.open_newwindow.restri
 pref("browser.link.open_newwindow.disabled_in_fullscreen", true);
 #else
 pref("browser.link.open_newwindow.disabled_in_fullscreen", false);
 #endif
 
 // Tabbed browser
 pref("browser.tabs.closeTabByDblclick", false);
 pref("browser.tabs.closeWindowWithLastTab", true);
+// Open related links to a tab, e.g., link in current tab, at next to the
+// current tab if |insertRelatedAfterCurrent| is true.  Otherwise, always
+// append new tab to the end.
 pref("browser.tabs.insertRelatedAfterCurrent", true);
+// Open all links, e.g., bookmarks, history items at next to current tab
+// if |insertAfterCurrent| is true.  Otherwise, append new tab to the end
+// for non-related links. Note that if this is set to true, it will trump
+// the value of browser.tabs.insertRelatedAfterCurrent.
+pref("browser.tabs.insertAfterCurrent", false);
 pref("browser.tabs.warnOnClose", true);
 pref("browser.tabs.warnOnCloseOtherTabs", true);
 pref("browser.tabs.warnOnOpen", true);
 pref("browser.tabs.maxOpenBeforeWarn", 15);
 pref("browser.tabs.loadInBackground", true);
 pref("browser.tabs.opentabfor.middleclick", true);
 pref("browser.tabs.loadDivertedInBackground", false);
 pref("browser.tabs.loadBookmarksInBackground", false);
--- a/browser/base/content/tabbrowser.js
+++ b/browser/base/content/tabbrowser.js
@@ -2144,16 +2144,17 @@ window._gBrowser = {
     var aOpener;
     var aOpenerBrowser;
     var aCreateLazyBrowser;
     var aSkipBackgroundNotify;
     var aNextTabParentId;
     var aNoInitialLabel;
     var aFocusUrlBar;
     var aName;
+    var aBulkOrderedOpen;
     if (arguments.length == 2 &&
         typeof arguments[1] == "object" &&
         !(arguments[1] instanceof Ci.nsIURI)) {
       let params = arguments[1];
       aTriggeringPrincipal = params.triggeringPrincipal;
       aReferrerURI = params.referrerURI;
       aReferrerPolicy = params.referrerPolicy;
       aCharset = params.charset;
@@ -2175,16 +2176,17 @@ window._gBrowser = {
       aOpener = params.opener;
       aOpenerBrowser = params.openerBrowser;
       aCreateLazyBrowser = params.createLazyBrowser;
       aSkipBackgroundNotify = params.skipBackgroundNotify;
       aNextTabParentId = params.nextTabParentId;
       aNoInitialLabel = params.noInitialLabel;
       aFocusUrlBar = params.focusUrlBar;
       aName = params.name;
+      aBulkOrderedOpen = params.bulkOrderedOpen;
     }
 
     // if we're adding tabs, we're past interrupt mode, ditch the owner
     if (this.selectedTab.owner) {
       this.selectedTab.owner = null;
     }
 
     // Find the tab that opened this one, if any. This is used for
@@ -2427,29 +2429,32 @@ window._gBrowser = {
           charset: aCharset,
           postData: aPostData,
         });
       } catch (ex) {
         Cu.reportError(ex);
       }
     }
 
-    // If we're opening a tab related to the an existing tab, move it
-    // to a position after that tab.
-    if (openerTab &&
-        Services.prefs.getBoolPref("browser.tabs.insertRelatedAfterCurrent")) {
-
-      let lastRelatedTab = this._lastRelatedTabMap.get(openerTab);
-      let newTabPos = (lastRelatedTab || openerTab)._tPos + 1;
+    // Move the new tab after another tab if needed.
+    if (!aBulkOrderedOpen &&
+        ((openerTab &&
+          Services.prefs.getBoolPref("browser.tabs.insertRelatedAfterCurrent")) ||
+         Services.prefs.getBoolPref("browser.tabs.insertAfterCurrent"))) {
+
+      let lastRelatedTab = openerTab && this._lastRelatedTabMap.get(openerTab);
+      let newTabPos = (lastRelatedTab || openerTab || this.selectedTab)._tPos + 1;
+
       if (lastRelatedTab)
         lastRelatedTab.owner = null;
-      else
+      else if (openerTab)
         t.owner = openerTab;
       this.moveTabTo(t, newTabPos, true);
-      this._lastRelatedTabMap.set(openerTab, t);
+      if (openerTab)
+        this._lastRelatedTabMap.set(openerTab, t);
     }
 
     // This field is updated regardless if we actually animate
     // since it's important that we keep this count correct in all cases.
     this.tabAnimationsInProgress++;
 
     if (animate) {
       requestAnimationFrame(function() {
--- a/browser/base/content/test/tabs/browser.ini
+++ b/browser/base/content/test/tabs/browser.ini
@@ -19,16 +19,18 @@ skip-if = !e10s # Tab spinner is e10s on
 skip-if = os == 'mac'
 [browser_navigatePinnedTab.js]
 [browser_new_file_whitelisted_http_tab.js]
 skip-if = !e10s # Test only relevant for e10s.
 [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_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_tabswitch_updatecommands.js]
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/tabs/browser_new_tab_insert_position.js
@@ -0,0 +1,177 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+"use strict";
+
+ChromeUtils.defineModuleGetter(this, "SessionStore",
+   "resource:///modules/sessionstore/SessionStore.jsm");
+ChromeUtils.defineModuleGetter(this, "TabStateFlusher",
+   "resource:///modules/sessionstore/TabStateFlusher.jsm");
+
+ChromeUtils.import("resource://gre/modules/sessionstore/Utils.jsm", this);
+const triggeringPrincipal_base64 = Utils.SERIALIZED_SYSTEMPRINCIPAL;
+
+function promiseBrowserStateRestored(state) {
+  if (typeof state != "string") {
+    state = JSON.stringify(state);
+  }
+  // We wait for the notification that restore is done, and for the notification
+  // that the active tab is loaded and restored.
+  let promise = Promise.all([
+    TestUtils.topicObserved("sessionstore-browser-state-restored"),
+    BrowserTestUtils.waitForEvent(gBrowser.tabContainer, "SSTabRestored")
+  ]);
+  SessionStore.setBrowserState(state);
+  return promise;
+}
+
+function promiseRemoveThenUndoCloseTab(tab) {
+  // We wait for the notification that restore is done, and for the notification
+  // that the active tab is loaded and restored.
+  let promise = Promise.all([
+    TestUtils.topicObserved("sessionstore-closed-objects-changed"),
+    BrowserTestUtils.waitForEvent(gBrowser.tabContainer, "SSTabRestored")
+  ]);
+  BrowserTestUtils.removeTab(tab);
+  SessionStore.undoCloseTab(window, 0);
+  return promise;
+}
+
+// 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}`);
+  }
+}
+
+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],
+    ["browser.sessionstore.restore_tabs_lazily", false],
+  ]});
+
+  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);
+
+  // Open a related tab via Middle click on the cell and test its position.
+  let openTabIndex = aInsertRelatedAfterCurrent || aInsertAfterCurrent ?
+    openerTabIndex + 1 : gBrowser.tabs.length;
+  let openTabDescription = aInsertRelatedAfterCurrent || aInsertAfterCurrent ?
+    "immediately to the right" : "at rightmost";
+
+  let newTabPromise = BrowserTestUtils.waitForNewTab(gBrowser, "http://example.com/#linkclick", true);
+  await BrowserTestUtils.synthesizeMouseAtCenter("#link_to_example_com",
+                                                 {button: 1}, gBrowser.selectedBrowser);
+  let openTab = await newTabPromise;
+  is(openTab.linkedBrowser.currentURI.spec, "http://example.com/#linkclick",
+     "Middle click should open site to correct url.");
+  is(openTab._tPos, openTabIndex,
+     kDescription + "Middle click should open site in a new tab " + openTabDescription);
+  if (aInsertRelatedAfterCurrent || aInsertAfterCurrent) {
+    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;
+
+  is(gBrowser.selectedBrowser.currentURI.spec, unrelatedTab.linkedBrowser.currentURI.spec,
+     `${kDescription} ${urlbarURL} should be loaded in the current tab.`);
+  is(unrelatedTab._tPos, openTabIndex,
+     `${kDescription} Alt+Enter in the URL bar should open page in a new tab ${openTabDescription}`);
+  is(unrelatedTab.owner, openerTab, "owner tab is set correctly");
+  ok(!unrelatedTab.openerTab, "no opener tab is set");
+
+  // Closing this should go back to the last selected tab, which just happens to be "openerTab"
+  // but is not in fact the opener.
+  BrowserTestUtils.removeTab(unrelatedTab);
+  is(gBrowser.selectedTab, openerTab,
+    kDescription + `openerTab should be selected after closing unrelated tab`);
+
+  // Go back to the opener tab.  Closing the child tab should return to the opener.
+  BrowserTestUtils.removeTab(openTab);
+  is(gBrowser.selectedTab, openerTab,
+     kDescription + "openerTab should be selected after closing related tab");
+
+  // Flush before messing with browser state.
+  for (let tab of gBrowser.tabs) {
+    await TabStateFlusher.flush(tab.linkedBrowser);
+  }
+
+  // Get the session state, verify SessionStore gives us expected data.
+  let newState = SessionStore.getBrowserState();
+  verifyTabState(newState);
+
+  // 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);
+
+  // 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);
+
+  // Restore pre-test state for next test.
+  await promiseBrowserStateRestored(oldState);
+}
+
+add_task(async function test_settings_insertRelatedAfter() {
+  // Firefox default settings.
+  await doTest(true, false);
+});
+
+add_task(async function test_settings_insertAfter() {
+  await doTest(true, true);
+});
+
+add_task(async function test_settings_always_insertAfter() {
+  await doTest(false, true);
+});
+
+add_task(async function test_settings_always_insertAtEnd() {
+    await doTest(false, false);
+});
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/tabs/file_new_tab_page.html
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+  </head>
+  <body>
+    <a href="http://example.com/#linkclick" id="link_to_example_com">go to example.com</a>
+  </body>
+</html>
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -3421,17 +3421,18 @@ var SessionStoreInternal = {
         // Setting noInitialLabel is a perf optimization. Rendering tab labels
         // would make resizing the tabs more expensive as we're adding them.
         // Each tab will get its initial label set in restoreTab.
         tab = tabbrowser.addTab(url,
                                 { createLazyBrowser,
                                   skipAnimation: true,
                                   noInitialLabel: true,
                                   userContextId,
-                                  skipBackgroundNotify: true });
+                                  skipBackgroundNotify: true,
+                                  bulkOrderedOpen: this._browserSetState });
 
         if (select) {
           let leftoverTab = tabbrowser.selectedTab;
           tabbrowser.selectedTab = tab;
           tabbrowser.removeTab(leftoverTab);
         }
       }
 
--- a/toolkit/components/extensions/ext-browserSettings.js
+++ b/toolkit/components/extensions/ext-browserSettings.js
@@ -130,16 +130,30 @@ ExtensionPreferencesManager.addSetting("
     "image.animation_mode",
   ],
 
   setCallback(value) {
     return {[this.prefNames[0]]: value};
   },
 });
 
+ExtensionPreferencesManager.addSetting("newTabPosition", {
+  prefNames: [
+    "browser.tabs.insertRelatedAfterCurrent",
+    "browser.tabs.insertAfterCurrent",
+  ],
+
+  setCallback(value) {
+    return {
+      "browser.tabs.insertAfterCurrent": value === "afterCurrent",
+      "browser.tabs.insertRelatedAfterCurrent": value === "relatedAfterCurrent",
+    };
+  },
+});
+
 ExtensionPreferencesManager.addSetting("openBookmarksInNewTabs", {
   prefNames: [
     "browser.tabs.loadBookmarksInTabs",
   ],
 
   setCallback(value) {
     return {[this.prefNames[0]]: value};
   },
@@ -270,16 +284,27 @@ this.browserSettings = class extends Ext
             return Services.prefs.getComplexValue(
               HOMEPAGE_URL_PREF, Ci.nsIPrefLocalizedString).data;
           }, undefined, true),
         imageAnimationBehavior: getSettingsAPI(
           extension, "imageAnimationBehavior",
           () => {
             return Services.prefs.getCharPref("image.animation_mode");
           }),
+        newTabPosition: getSettingsAPI(
+          extension, "newTabPosition",
+          () => {
+            if (Services.prefs.getBoolPref("browser.tabs.insertAfterCurrent")) {
+              return "afterCurrent";
+            }
+            if (Services.prefs.getBoolPref("browser.tabs.insertRelatedAfterCurrent")) {
+              return "relatedAfterCurrent";
+            }
+            return "atEnd";
+          }),
         newTabPageOverride: getSettingsAPI(
           extension, NEW_TAB_OVERRIDE_SETTING,
           () => {
             return aboutNewTabService.newTabURL;
           }, URL_STORE_TYPE, true),
         openBookmarksInNewTabs: getSettingsAPI(
           extension, "openBookmarksInNewTabs",
           () => {
--- a/toolkit/components/extensions/schemas/browser_settings.json
+++ b/toolkit/components/extensions/schemas/browser_settings.json
@@ -132,16 +132,20 @@
       "imageAnimationBehavior": {
         "$ref": "types.Setting",
         "description": "Controls the behaviour of image animation in the browser. This setting's value is of type ImageAnimationBehavior, defaulting to <code>normal</code>."
       },
       "newTabPageOverride": {
         "$ref": "types.Setting",
         "description": "Returns the value of the overridden new tab page. Read-only."
       },
+      "newTabPosition": {
+        "$ref": "types.Setting",
+        "description": "Controls where new tabs are opened. `afterCurrent` will open all new tabs next to the current tab, `relatedAfterCurrent` will open only related tabs next to the current tab, and `atEnd` will open all tabs at the end of the tab strip. The default is `relatedAfterCurrent`."
+      },
       "openBookmarksInNewTabs": {
         "$ref": "types.Setting",
         "description": "This boolean setting controls whether bookmarks are opened in the current tab or in a new tab."
       },
       "openSearchResultsInNewTabs": {
         "$ref": "types.Setting",
         "description": "This boolean setting controls whether search results are opened in the current tab or in a new tab."
       },
--- a/toolkit/components/extensions/test/xpcshell/test_ext_browserSettings.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_browserSettings.js
@@ -43,16 +43,18 @@ add_task(async function test_browser_set
     "network.proxy.ssl_port": 0,
     "network.proxy.socks": "",
     "network.proxy.socks_port": 0,
     "network.proxy.socks_version": 5,
     "network.proxy.socks_remote_dns": false,
     "network.proxy.no_proxies_on": "localhost, 127.0.0.1",
     "network.proxy.autoconfig_url": "",
     "signon.autologin.proxy": false,
+    "browser.tabs.insertRelatedAfterCurrent": true,
+    "browser.tabs.insertAfterCurrent": false,
   };
 
   async function background() {
     browser.test.onMessage.addListener(async (msg, apiName, value) => {
       let apiObj = browser.browserSettings[apiName];
       let result = await apiObj.set({value});
       if (msg === "set") {
         browser.test.assertTrue(result, "set returns true.");
@@ -170,16 +172,35 @@ add_task(async function test_browser_set
       "closeTabsByDoubleClick", true,
       {"browser.tabs.closeTabByDblclick": true});
     await testSetting(
       "closeTabsByDoubleClick", false,
       {"browser.tabs.closeTabByDblclick": false});
   }
 
   await testSetting(
+    "newTabPosition", "afterCurrent",
+    {
+      "browser.tabs.insertRelatedAfterCurrent": false,
+      "browser.tabs.insertAfterCurrent": true,
+    });
+  await testSetting(
+    "newTabPosition", "atEnd",
+    {
+      "browser.tabs.insertRelatedAfterCurrent": false,
+      "browser.tabs.insertAfterCurrent": false,
+    });
+  await testSetting(
+    "newTabPosition", "relatedAfterCurrent",
+    {
+      "browser.tabs.insertRelatedAfterCurrent": true,
+      "browser.tabs.insertAfterCurrent": false,
+    });
+
+  await testSetting(
     "openBookmarksInNewTabs", true,
     {"browser.tabs.loadBookmarksInTabs": true});
   await testSetting(
     "openBookmarksInNewTabs", false,
     {"browser.tabs.loadBookmarksInTabs": false});
 
   await testSetting(
     "openSearchResultsInNewTabs", true,