Bug 1337682 - CanonizeURL feature may override the next load of the current URL. r=adw, a=lizzard
authorMarco Bonardo <mbonardo@mozilla.com>
Wed, 15 Feb 2017 14:30:31 +0100
changeset 376614 6139d75fe055e310a4b436223d126f3690eda896
parent 376613 e9f2075a8de61086e6ba7207fdaf1f81b578f4ff
child 376615 06f51944867c197d2237f107f541ef470c563b40
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw, lizzard
bugs1337682
milestone53.0a2
Bug 1337682 - CanonizeURL feature may override the next load of the current URL. r=adw, a=lizzard MozReview-Commit-ID: ATdFsSXstZz
browser/base/content/test/urlbar/browser_canonizeURL.js
browser/base/content/urlbarBindings.xml
--- a/browser/base/content/test/urlbar/browser_canonizeURL.js
+++ b/browser/base/content/test/urlbar/browser_canonizeURL.js
@@ -1,34 +1,40 @@
-var pairs = [
-  ["example", "http://www.example.net/"],
-  ["ex-ample", "http://www.ex-ample.net/"],
-  ["  example ", "http://www.example.net/"],
-  [" example/foo ", "http://www.example.net/foo"],
-  [" example/foo bar ", "http://www.example.net/foo%20bar"],
-  ["example.net", "http://example.net/"],
-  ["http://example", "http://example/"],
-  ["example:8080", "http://example:8080/"],
-  ["ex-ample.foo", "http://ex-ample.foo/"],
-  ["example.foo/bar ", "http://example.foo/bar"],
-  ["1.1.1.1", "http://1.1.1.1/"],
-  ["ftp://example", "ftp://example/"],
-  ["ftp.example.bar", "http://ftp.example.bar/"],
-  ["ex ample", Services.search.defaultEngine.getSubmission("ex ample", null, "keyword").uri.spec],
-];
+add_task(function*() {
+  let testcases = [
+    ["example", "http://www.example.net/", { shiftKey: true }],
+    // Check that a direct load is not overwritten by a previous canonization.
+    ["http://example.com/test/", "http://example.com/test/", {}],
+    ["ex-ample", "http://www.ex-ample.net/", { shiftKey: true }],
+    ["  example ", "http://www.example.net/", { shiftKey: true }],
+    [" example/foo ", "http://www.example.net/foo", { shiftKey: true }],
+    [" example/foo bar ", "http://www.example.net/foo%20bar", { shiftKey: true }],
+    ["example.net", "http://example.net/", { shiftKey: true }],
+    ["http://example", "http://example/", { shiftKey: true }],
+    ["example:8080", "http://example:8080/", { shiftKey: true }],
+    ["ex-ample.foo", "http://ex-ample.foo/", { shiftKey: true }],
+    ["example.foo/bar ", "http://example.foo/bar", { shiftKey: true }],
+    ["1.1.1.1", "http://1.1.1.1/", { shiftKey: true }],
+    ["ftp://example", "ftp://example/", { shiftKey: true }],
+    ["ftp.example.bar", "http://ftp.example.bar/", { shiftKey: true }],
+    ["ex ample", Services.search.defaultEngine.getSubmission("ex ample", null, "keyword").uri.spec, { shiftKey: true }],
+  ];
 
-add_task(function*() {
   // Disable autoFill for this test, since it could mess up the results.
   let autoFill = Preferences.get("browser.urlbar.autoFill");
   Preferences.set("browser.urlbar.autoFill", false);
   registerCleanupFunction(() => {
     Preferences.set("browser.urlbar.autoFill", autoFill);
   });
 
-  for (let [inputValue, expectedURL] of pairs) {
+  for (let [inputValue, expectedURL, options] of testcases) {
     let promiseLoad = waitForDocLoadAndStopIt(expectedURL);
     gURLBar.focus();
-    gURLBar.inputField.value = inputValue.slice(0, -1);
-    EventUtils.synthesizeKey(inputValue.slice(-1), {});
-    EventUtils.synthesizeKey("VK_RETURN", { shiftKey: true });
+    if (Object.keys(options).length > 0) {
+      gURLBar.inputField.value = inputValue.slice(0, -1);
+      EventUtils.synthesizeKey(inputValue.slice(-1), {});
+    } else {
+      gURLBar.textValue = inputValue;
+    }
+    EventUtils.synthesizeKey("VK_RETURN", options);
     yield promiseLoad;
   }
 });
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -1148,16 +1148,17 @@ file, You can obtain one at http://mozil
             searchString: this.mController.searchString,
             event
           };
 
           if (this.popup.selectedIndex != 0 || this.gotResultForCurrentQuery) {
             this.maybeCanonizeURL(event, this.value);
             let rv = this.mController.handleEnter(false, event);
             this.handleEnterInstance = null;
+            this.popup.overrideValue = null;
             return rv;
           }
 
           return true;
         ]]></body>
       </method>
 
       <method name="handleDelete">
@@ -1827,16 +1828,17 @@ file, You can obtain one at http://mozil
               // Don't handle this immediately or we could cause a recursive
               // loop where the controller sets popupOpen and re-enters here.
               setTimeout(() => {
                 // Safety check: handle only if the search string didn't change.
                 let { event, searchString } = instance;
                 if (this.input.mController.searchString == searchString) {
                   this.input.maybeCanonizeURL(event, searchString);
                   this.input.mController.handleEnter(false, event);
+                  this.overrideValue = null;
                 }
               }, 0);
             }
           ]]>
         </body>
       </method>
 
       <method name="_onSearchBegin">
@@ -1848,17 +1850,16 @@ file, You can obtain one at http://mozil
           // 1. if a search starts we set selectedIndex to 0 here, and it will
           //    be updated by onResultsAdded. Since selectedIndex is 0,
           //    handleEnter will delay the action if a result didn't arrive yet.
           // 2. if a search doesn't start (for example if autocomplete is
           //    disabled), this won't be called, and the selectedIndex will be
           //    the default -1 value. Then handleEnter will know it should not
           //    delay the action, cause a result wont't ever arrive.
           this.input.controller.setInitiallySelectedIndex(0);
-          this.overrideValue = null;
         ]]></body>
       </method>
 
       <field name="_addonIframe">null</field>
       <field name="_addonIframeOwner">null</field>
       <field name="_addonIframeOverriddenFunctionsByName">{}</field>
 
       <!-- These methods must be overridden and properly handled by the API