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 761910 e311c7e57fb7864588609da5961b1440ad528e80
parent 761128 c50f5f846c2e6b4c8da1e80b6790926ad84c3450
push id101046
push userbmo:bob.silverberg@gmail.com
push dateThu, 01 Mar 2018 17:24:33 +0000
reviewersmixedpuppy, dao
bugs1344749, 933532
milestone60.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
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
@@ -453,17 +453,25 @@ pref("browser.link.open_newwindow.restri
 #ifdef XP_MACOSX
 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.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
@@ -2906,30 +2906,32 @@ class TabBrowser {
           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;
-      if (lastRelatedTab)
-        lastRelatedTab.owner = null;
-      else
-        t.owner = openerTab;
-      this.moveTabTo(t, newTabPos, true);
+    // Move the new tab after another tab if needed.
+    if ((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.mCurrentTab)._tPos + 1;
+
+    if (lastRelatedTab)
+      lastRelatedTab.owner = null;
+    else if (openerTab)
+      t.owner = openerTab;
+    this.moveTabTo(t, newTabPos, true);
+    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() {
         // kick the animation off
--- a/browser/base/content/test/tabs/browser.ini
+++ b/browser/base/content/test/tabs/browser.ini
@@ -18,16 +18,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,94 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+async function doTest(aInsertRelatedAfterCurrent, aInsertAfterCurrent) {
+  const kDescription = "(aInsertRelatedAfterCurrent=" + aInsertRelatedAfterCurrent +
+                       ", aInsertAfterCurrent=" + aInsertAfterCurrent + "): ";
+  is(gBrowser.tabs.length, 1, kDescription + "one tab is open initially");
+
+  await SpecialPowers.pushPrefEnv({set: [
+    ["browser.tabs.opentabfor.middleclick", true],
+    ["browser.tabs.loadBookmarksInBackground", false],
+    ["browser.tabs.insertRelatedAfterCurrent", aInsertRelatedAfterCurrent],
+    ["browser.tabs.insertAfterCurrent", aInsertAfterCurrent]
+  ]});
+
+  // Add a few tabs.
+  let tabs = [];
+  function addTab(aURL, aReferrer) {
+    let tab = BrowserTestUtils.addTab(gBrowser, aURL, {referrerURI: aReferrer});
+    tabs.push(tab);
+    return BrowserTestUtils.browserLoaded(tab.linkedBrowser);
+  }
+
+  await addTab("http://mochi.test:8888/#0");
+  await addTab("http://mochi.test:8888/#1");
+  await addTab("http://mochi.test:8888/#2");
+  await addTab("http://mochi.test:8888/#3");
+
+  // Create a new 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 newTab = await BrowserTestUtils.openNewForegroundTab(gBrowser, pageURL);
+  let newTabURISpec = newTab.linkedBrowser.currentURI.spec;
+  const kNewTabIndex = 1;
+  gBrowser.moveTabTo(newTab, kNewTabIndex);
+
+  let openTabIndex = aInsertRelatedAfterCurrent || aInsertAfterCurrent ?
+    kNewTabIndex + 1 : gBrowser.tabs.length;
+  let openTabDescription = aInsertRelatedAfterCurrent || aInsertAfterCurrent ?
+    "immediately to the right" : "at rightmost";
+
+  // Middle click on the cell should open example.com in a new tab.
+  let newTabPromise = BrowserTestUtils.waitForNewTab(gBrowser, "http://example.com/", true);
+  await BrowserTestUtils.synthesizeMouseAtCenter("#link_to_example_com",
+                                                 {button: 1}, gBrowser.selectedBrowser);
+  let openTab = await newTabPromise;
+  is(openTab.linkedBrowser.currentURI.spec, "http://example.com/",
+     "Middle click should open site to correct url.");
+  is(openTab._tPos, openTabIndex,
+     kDescription + "Middle click should open site in a new tab " + openTabDescription);
+
+  // Remove the new opened tab which loaded example.com.
+  gBrowser.removeTab(gBrowser.tabs[openTabIndex]);
+
+  // Go back to the new tab.
+  gBrowser.selectedTab = newTab;
+  is(gBrowser.selectedBrowser.currentURI.spec, newTabURISpec,
+     kDescription + "New tab URI shouldn't be changed");
+
+  openTabIndex = aInsertAfterCurrent ? kNewTabIndex + 1 : gBrowser.tabs.length;
+  openTabDescription = aInsertAfterCurrent ? "immediately to the right" : "at rightmost";
+
+  // Open about:mozilla in new tab from the URL bar.
+  gURLBar.focus();
+  gURLBar.select();
+  newTabPromise = BrowserTestUtils.waitForNewTab(gBrowser, "about:mozilla");
+  EventUtils.sendString("about:mozilla");
+  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" });
+  openTab = await newTabPromise;
+
+  is(newTab.linkedBrowser.currentURI.spec, newTabURISpec,
+     kDescription + "example.com should be loaded in current tab.");
+  is(openTab.linkedBrowser.currentURI.spec, "about:mozilla",
+     kDescription + "about:mozilla should be loaded in the new tab.");
+  is(openTab._tPos, openTabIndex,
+     kDescription + "Alt+Enter in the URL bar should open page in a new tab " + openTabDescription);
+
+  // Remove all tabs opened by this test.
+  while (gBrowser.tabs[1]) {
+    gBrowser.removeTab(gBrowser.tabs[1]);
+  }
+}
+
+add_task(async function() {
+  // Current default settings.
+  await doTest(true, false);
+  // Perhaps, some users would love this settings.
+  await doTest(true, true);
+  // Maybe, unrealistic cases, but we should test these cases too.
+  await doTest(false, true);
+  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/" id="link_to_example_com">go to example.com</a>
+  </body>
+</html>
--- a/toolkit/components/extensions/ext-browserSettings.js
+++ b/toolkit/components/extensions/ext-browserSettings.js
@@ -120,16 +120,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};
   },
@@ -255,16 +269,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
@@ -128,16 +128,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
@@ -42,16 +42,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.");
@@ -160,16 +162,35 @@ add_task(async function test_browser_set
       {"ui.context_menus.after_mouseup": AppConstants.platform === "win"});
   } else {
     await testSetting(
       "contextMenuShowEvent", "mousedown",
       {"ui.context_menus.after_mouseup": 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,