Bug 933532 - Add a preference to open new tab which is not related to any existing tabs immediately to right of current tab r?dao draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Sun, 10 Dec 2017 09:29:38 +0900
changeset 711698 ae9ae7f786763df020bb0942d7162b35591ec5c3
parent 711413 22d2831cc1f41e1b3e1ebac9be5a7aff33684843
child 743843 004b0063f942ca8d522c3f22fcf6b15f382ed94a
push id93107
push usermasayuki@d-toybox.com
push dateThu, 14 Dec 2017 14:06:29 +0000
reviewersdao
bugs933532
milestone59.0a1
Bug 933532 - Add a preference to open new tab which is not related to any existing tabs immediately to right of current tab r?dao Currently, we have a pref, "browser.tabs.insertRelatedAfterCurrent", to open new tab for a link in current tab immediately to the right. This is also used for navigation buttons too. However, opening new tab from bookmarks, history itmes and Alt+Enter in URL bar and search bar is always appended to rightmost of the tab bar. So, there should be a new pref, "browser.tabs.insertUnreleatedAfterCurrent", to open new tab form them immediately to the right of current tab. Note that this patch just adds a pref for users who want the new behavior for now. Whether it should be default behavior or not should be discussed later. MozReview-Commit-ID: 6k2mZxvtutf
browser/app/profile/firefox.js
browser/base/content/tabbrowser.xml
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
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -449,17 +449,24 @@ 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 even unrelated links, e.g., bookmarks, history items at next to
+// current tab if |insertUnreleatedAfterCurrent| is true.  Otherwise,
+// always append new tab to the end.
+pref("browser.tabs.insertUnreleatedAfterCurrent", 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.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -2863,28 +2863,38 @@
                   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;
+            // to a position after that tab when "insertRelatedAfterCurrent"
+            // pref is set to true.
+            // If we're opening a tab not related to any tabs, e.g., opneing
+            // a bookmark, a history item or specific URL from the URL bar,
+            // move it to a position after current tab when
+            // "insertUnreleatedAfterCurrent" pref is set to true.
+            if (openerTab ?
+                 Services.prefs.getBoolPref("browser.tabs.insertRelatedAfterCurrent") :
+                 Services.prefs.getBoolPref("browser.tabs.insertUnreleatedAfterCurrent")) {
+
+              let lastRelatedTab =
+                openerTab ? this._lastRelatedTabMap.get(openerTab) : null;
+              let prevTabOfNewTab =
+                lastRelatedTab || openerTab || this.mCurrentTab;
+              let newTabPos = prevTabOfNewTab._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
@@ -11,16 +11,18 @@ support-files =
 [browser_tabCloseProbes.js]
 [browser_tabSpinnerProbe.js]
 skip-if = !e10s # Tab spinner is e10s only.
 [browser_tabSwitchPrintPreview.js]
 skip-if = os == 'mac'
 [browser_navigatePinnedTab.js]
 [browser_new_file_whitelisted_http_tab.js]
 skip-if = !e10s # Test only relevant for e10s.
+[browser_new_tab_insert_position.js]
+support-files = file_new_tab_page.html
 [browser_new_web_tab_in_file_process_pref.js]
 skip-if = !e10s # Pref and test only relevant for e10s.
 [browser_opened_file_tab_navigated_to_web.js]
 [browser_overflowScroll.js]
 [browser_pinnedTabs.js]
 [browser_pinnedTabs_closeByKeyboard.js]
 [browser_positional_attributes.js]
 [browser_preloadedBrowser_zoom.js]
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/tabs/browser_new_tab_insert_position.js
@@ -0,0 +1,88 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+async function doTest(aInsertRelatedAfterCurrent, aInsertUnrelatedAfterCurrent) {
+  const kDescription = "(aInsertRelatedAfterCurrent=" + aInsertRelatedAfterCurrent +
+                       ", aInsertUnrelatedAfterCurrent=" + aInsertUnrelatedAfterCurrent + "): ";
+  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.insertUnreleatedAfterCurrent", aInsertUnrelatedAfterCurrent]
+  ]});
+
+  // 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 dir = getChromeDir(getResolvedURI(gTestPath));
+  dir.append("file_new_tab_page.html");
+  let newTab = await BrowserTestUtils.openNewForegroundTab(gBrowser, Services.io.newFileURI(dir).spec);
+  let newTabURISpec = newTab.linkedBrowser.currentURI.spec;
+  const kNewTabIndex = 1;
+  gBrowser.moveTabTo(newTab, kNewTabIndex);
+
+  let openTabIndex = aInsertRelatedAfterCurrent ? kNewTabIndex + 1 : gBrowser.tabs.length;
+  let openTabDescription = aInsertRelatedAfterCurrent ? "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/",
+     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 = aInsertUnrelatedAfterCurrent ? kNewTabIndex + 1 : gBrowser.tabs.length;
+  openTabDescription = aInsertUnrelatedAfterCurrent ? "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 + "Alt+Enter in the URL bar should open previous 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,4 @@
+<!DOCTYPE html>
+<body>
+<a href="http://example.com/" id="link_to_example_com">go to example.com</a>
+</body>