Bug 1251841 - switchIFURIInWindow can ignore fragments AND query strings, add tests. r=markh
authorKurt Carpenter <kurtcarpenter@hotmail.com>
Tue, 26 Jul 2016 10:27:42 +1000
changeset 348642 94968a940273882150fc98556d4abf961b287ad8
parent 348641 5a8b18f503baa996bcba9ea249e1762b7e536211
child 348643 8e9b741583f2c514c9123460204312bb1a52eb79
child 348745 82e3f2a1196a227ce83ae8e0d53d5c2b273205a5
push id1230
push userjlund@mozilla.com
push dateMon, 31 Oct 2016 18:13:35 +0000
treeherdermozilla-release@5e06e3766db2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh
bugs1251841
milestone50.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 1251841 - switchIFURIInWindow can ignore fragments AND query strings, add tests. r=markh MozReview-Commit-ID: DVLzg2KHQCd
browser/base/content/browser.js
browser/base/content/test/urlbar/browser_bug1025195_switchToTabHavingURI_aOpenParams.js
browser/base/content/utilityOverlay.js
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -7447,17 +7447,17 @@ var gRemoteTabsUI = {
  *        If no suitable window is found, a new one will be opened.
  * @param aOpenParams
  *        If switching to this URI results in us opening a tab, aOpenParams
  *        will be the parameter object that gets passed to openUILinkIn. Please
  *        see the documentation for openUILinkIn to see what parameters can be
  *        passed via this object.
  *        This object also allows:
  *        - 'ignoreFragment' property to be set to true to exclude fragment-portion
- *        matching when comparing URIs.
+ *        matching when comparing URIs. Fragment will be replaced.
  *        - 'ignoreQueryString' property to be set to true to exclude query string
  *        matching when comparing URIs.
  *        - 'replaceQueryString' property to be set to true to exclude query string
  *        matching when comparing URIs and overwrite the initial query string with
  *        the one from the new URI.
  * @return True if an existing tab was found, false otherwise
  */
 function switchToTabHavingURI(aURI, aOpenNew, aOpenParams={}) {
@@ -7483,41 +7483,51 @@ function switchToTabHavingURI(aURI, aOpe
     // are private and they are not in permanent private browsing mode
     if (!kPrivateBrowsingWhitelist.has(aURI.spec) &&
         (PrivateBrowsingUtils.isWindowPrivate(window) ||
          PrivateBrowsingUtils.isWindowPrivate(aWindow)) &&
         !PrivateBrowsingUtils.permanentPrivateBrowsing) {
       return false;
     }
 
+    //Remove the query string, fragment, both, or neither from a given url.
+    function cleanURL(url, removeQuery, removeFragment) {
+      let ret = url;
+      if (removeFragment) {
+        ret = ret.split("#")[0];
+        if (removeQuery) {
+          // This removes a query, if present before the fragment.
+          ret = ret.split("?")[0];
+        }
+      } else if (removeQuery) {
+        // This is needed in case there is a fragment after the query.
+        let fragment = ret.split("#")[1];
+        ret = ret.split("?")[0].concat(
+          (fragment != undefined) ? "#".concat(fragment) : "");
+      }
+      return ret;
+    }
+
+    // Need to handle nsSimpleURIs here too (e.g. about:...), which don't
+    // work correctly with URL objects - so treat them as strings
+    let requestedCompare = cleanURL(
+        aURI.spec, ignoreQueryString || replaceQueryString, ignoreFragment);
     let browsers = aWindow.gBrowser.browsers;
     for (let i = 0; i < browsers.length; i++) {
       let browser = browsers[i];
-      if (ignoreFragment ? browser.currentURI.equalsExceptRef(aURI) :
-                           browser.currentURI.equals(aURI)) {
-        // Focus the matching window & tab
+      let browserCompare = cleanURL(
+          browser.currentURI.spec, ignoreQueryString || replaceQueryString, ignoreFragment);
+      if (requestedCompare == browserCompare) {
         aWindow.focus();
-        if (ignoreFragment) {
-          let spec = aURI.spec;
-          browser.loadURI(spec);
+        if (ignoreFragment || replaceQueryString) {
+          browser.loadURI(aURI.spec);
         }
         aWindow.gBrowser.tabContainer.selectedIndex = i;
         return true;
       }
-      if (ignoreQueryString || replaceQueryString) {
-        if (browser.currentURI.spec.split("?")[0] == aURI.spec.split("?")[0]) {
-          // Focus the matching window & tab
-          aWindow.focus();
-          if (replaceQueryString) {
-            browser.loadURI(aURI.spec);
-          }
-          aWindow.gBrowser.tabContainer.selectedIndex = i;
-          return true;
-        }
-      }
     }
     return false;
   }
 
   // This can be passed either nsIURI or a string.
   if (!(aURI instanceof Ci.nsIURI))
     aURI = Services.io.newURI(aURI, null, null);
 
--- a/browser/base/content/test/urlbar/browser_bug1025195_switchToTabHavingURI_aOpenParams.js
+++ b/browser/base/content/test/urlbar/browser_bug1025195_switchToTabHavingURI_aOpenParams.js
@@ -1,13 +1,13 @@
 /* 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/. */
 
-add_task(function test_ignoreFragment() {
+add_task(function *test_ignoreFragment() {
   let tabRefAboutHome = gBrowser.addTab("about:home#1");
   yield promiseTabLoaded(tabRefAboutHome);
   let tabRefAboutMozilla = gBrowser.addTab("about:mozilla");
   yield promiseTabLoaded(tabRefAboutMozilla);
 
   gBrowser.selectedTab = tabRefAboutMozilla;
   let numTabsAtStart = gBrowser.tabs.length;
 
@@ -30,34 +30,34 @@ add_task(function test_ignoreFragment() 
   yield promiseWaitForCondition(function() {
     return tabRefAboutHome.linkedBrowser.currentURI.spec == "about:home";
   });
   is(tabRefAboutHome.linkedBrowser.currentURI.spec, "about:home", "about:home shouldn't have hash");
   switchTab("about:about", false, { ignoreFragment: true });
   cleanupTestTabs();
 });
 
-add_task(function test_ignoreQueryString() {
+add_task(function* test_ignoreQueryString() {
   let tabRefAboutHome = gBrowser.addTab("about:home?hello=firefox");
   yield promiseTabLoaded(tabRefAboutHome);
   let tabRefAboutMozilla = gBrowser.addTab("about:mozilla");
   yield promiseTabLoaded(tabRefAboutMozilla);
   gBrowser.selectedTab = tabRefAboutMozilla;
 
   switchTab("about:home?hello=firefox", true);
   switchTab("about:home?hello=firefoxos", false);
   // Remove the last opened tab to test ignoreQueryString option.
   gBrowser.removeCurrentTab();
   switchTab("about:home?hello=firefoxos", true, { ignoreQueryString: true });
   is(tabRefAboutHome, gBrowser.selectedTab, "Selected tab should be the initial about:home tab");
   is(gBrowser.currentURI.spec, "about:home?hello=firefox", "The spec should NOT be updated to the new query string");
   cleanupTestTabs();
 });
 
-add_task(function test_replaceQueryString() {
+add_task(function* test_replaceQueryString() {
   let tabRefAboutHome = gBrowser.addTab("about:home?hello=firefox");
   yield promiseTabLoaded(tabRefAboutHome);
   let tabRefAboutMozilla = gBrowser.addTab("about:mozilla");
   yield promiseTabLoaded(tabRefAboutMozilla);
   gBrowser.selectedTab = tabRefAboutMozilla;
 
   switchTab("about:home", false);
   switchTab("about:home?hello=firefox", true);
@@ -67,16 +67,48 @@ add_task(function test_replaceQueryStrin
   switchTab("about:home?hello=firefoxos", true, { replaceQueryString: true });
   is(tabRefAboutHome, gBrowser.selectedTab, "Selected tab should be the initial about:home tab");
   // Wait for the tab to load the new URI spec.
   yield promiseTabLoaded(tabRefAboutHome);
   is(gBrowser.currentURI.spec, "about:home?hello=firefoxos", "The spec should be updated to the new spec");
   cleanupTestTabs();
 });
 
+add_task(function* test_replaceQueryStringAndFragment() {
+  let tabRefAboutHome = gBrowser.addTab("about:home?hello=firefox#aaa");
+  yield promiseTabLoaded(tabRefAboutHome);
+  let tabRefAboutMozilla = gBrowser.addTab("about:mozilla?hello=firefoxos#aaa");
+  yield promiseTabLoaded(tabRefAboutMozilla);
+  gBrowser.selectedTab = tabRefAboutMozilla;
+
+  switchTab("about:home", false);
+  gBrowser.removeCurrentTab();
+  switchTab("about:home?hello=firefox#aaa", true);
+  is(tabRefAboutHome, gBrowser.selectedTab, "Selected tab should be the initial about:home tab");
+  switchTab("about:mozilla?hello=firefox#bbb", true, { replaceQueryString: true, ignoreFragment: true });
+  is(tabRefAboutMozilla, gBrowser.selectedTab, "Selected tab should be the initial about:mozilla tab");
+  switchTab("about:home?hello=firefoxos#bbb", true, { ignoreQueryString: true, ignoreFragment: true });
+  is(tabRefAboutHome, gBrowser.selectedTab, "Selected tab should be the initial about:home tab");
+  cleanupTestTabs();
+});
+
+add_task(function* test_ignoreQueryStringIgnoresFragment() {
+  let tabRefAboutHome = gBrowser.addTab("about:home?hello=firefox#aaa");
+  yield promiseTabLoaded(tabRefAboutHome);
+  let tabRefAboutMozilla = gBrowser.addTab("about:mozilla?hello=firefoxos#aaa");
+  yield promiseTabLoaded(tabRefAboutMozilla);
+  gBrowser.selectedTab = tabRefAboutMozilla;
+
+  switchTab("about:home?hello=firefox#bbb", false, { ignoreQueryString: true });
+  gBrowser.removeCurrentTab();
+  switchTab("about:home?hello=firefoxos#aaa", true, { ignoreQueryString: true });
+  is(tabRefAboutHome, gBrowser.selectedTab, "Selected tab should be the initial about:home tab");
+  cleanupTestTabs();
+});
+
 // Begin helpers
 
 function cleanupTestTabs() {
   while (gBrowser.tabs.length > 1)
     gBrowser.removeCurrentTab();
 }
 
 function switchTab(aURI, aShouldFindExistingTab, aOpenParams = {}) {
--- a/browser/base/content/utilityOverlay.js
+++ b/browser/base/content/utilityOverlay.js
@@ -622,17 +622,17 @@ function openPreferences(paneID, extraAr
     let supportsStringPrefURL = Cc["@mozilla.org/supports-string;1"]
                                   .createInstance(Ci.nsISupportsString);
     supportsStringPrefURL.data = preferencesURL;
     windowArguments.AppendElement(supportsStringPrefURL);
 
     win = Services.ww.openWindow(null, Services.prefs.getCharPref("browser.chromeURL"),
                                  "_blank", "chrome,dialog=no,all", windowArguments);
   } else {
-    newLoad = !win.switchToTabHavingURI(preferencesURL, true, {ignoreFragment: true});
+    newLoad = !win.switchToTabHavingURI(preferencesURL, true, { ignoreFragment: true, replaceQueryString: true });
     browser = win.gBrowser.selectedBrowser;
   }
 
   if (newLoad) {
     Services.obs.addObserver(function advancedPaneLoadedObs(prefWin, topic, data) {
       if (!browser) {
         browser = win.gBrowser.selectedBrowser;
       }