Bug 1299428 - Searching an URL looking string using a one-off button doesn't open a search result. r=adw
authorMarco Bonardo <mbonardo@mozilla.com>
Wed, 02 Nov 2016 18:32:38 +0100
changeset 321082 370bcd32def2c9f551455063d9c4b9d7b0d6ae43
parent 321081 55884e6a3c8ceb6374678f66d36ea56d62744178
child 321083 b885dbd862e5ea2d0f85e4db9a9c9f757587ac16
push id30915
push userphilringnalda@gmail.com
push dateSat, 05 Nov 2016 03:42:29 +0000
treeherdermozilla-central@a7c654513f2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs1299428
milestone52.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 1299428 - Searching an URL looking string using a one-off button doesn't open a search result. r=adw MozReview-Commit-ID: FA9qkqLw5Cw
browser/base/content/test/urlbar/browser_urlbarOneOffs.js
browser/base/content/urlbarBindings.xml
--- a/browser/base/content/test/urlbar/browser_urlbarOneOffs.js
+++ b/browser/base/content/test/urlbar/browser_urlbarOneOffs.js
@@ -169,43 +169,51 @@ add_task(function* searchWith() {
 
   yield hidePopup();
 });
 
 // Clicks a one-off.
 add_task(function* oneOffClick() {
   gBrowser.selectedTab = gBrowser.addTab();
 
-  let typedValue = "foo";
+  // We are explicitly using something that looks like a url, to make the test
+  // stricter. Even if it looks like a url, we should search.
+  let typedValue = "foo.bar";
   yield promiseAutocompleteResultPopup(typedValue);
 
   assertState(0, -1, typedValue);
 
   let oneOffs = gURLBar.popup.oneOffSearchButtons.getSelectableButtons(true);
-  let resultsPromise = promiseSearchResultsLoaded();
+  let resultsPromise =
+    BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser, false,
+                                   "http://mochi.test:8888/");
   EventUtils.synthesizeMouseAtCenter(oneOffs[0], {});
   yield resultsPromise;
 
   gBrowser.removeTab(gBrowser.selectedTab);
 });
 
 // Presses the Return key when a one-off is selected.
 add_task(function* oneOffReturn() {
   gBrowser.selectedTab = gBrowser.addTab();
 
-  let typedValue = "foo";
+  // We are explicitly using something that looks like a url, to make the test
+  // stricter. Even if it looks like a url, we should search.
+  let typedValue = "foo.bar";
   yield promiseAutocompleteResultPopup(typedValue, window, true);
 
   assertState(0, -1, typedValue);
 
   // Alt+Down to select the first one-off.
   EventUtils.synthesizeKey("VK_DOWN", { altKey: true })
   assertState(0, 0, typedValue);
 
-  let resultsPromise = promiseSearchResultsLoaded();
+  let resultsPromise =
+    BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser, false,
+                                   "http://mochi.test:8888/");
   EventUtils.synthesizeKey("VK_RETURN", {})
   yield resultsPromise;
 
   gBrowser.removeTab(gBrowser.selectedTab);
 });
 
 
 function assertState(result, oneOff, textValue = undefined) {
@@ -217,17 +225,8 @@ function assertState(result, oneOff, tex
     Assert.equal(gURLBar.textValue, textValue, "Expected textValue");
   }
 }
 
 function* hidePopup() {
   EventUtils.synthesizeKey("VK_ESCAPE", {});
   yield promisePopupHidden(gURLBar.popup);
 }
-
-function promiseSearchResultsLoaded() {
-  let tab = gBrowser.selectedTab;
-  return promiseTabLoadEvent(tab).then(() => {
-    Assert.equal(tab.linkedBrowser.currentURI.spec,
-                 "http://mochi.test:8888/",
-                 'Expected "search results" page loaded');
-  });
-}
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -405,19 +405,27 @@ file, You can obtain one at http://mozil
           let url = this.value;
           if (!url) {
             return;
           }
 
           let mayInheritPrincipal = false;
           let postData = null;
           let browser = gBrowser.selectedBrowser;
+          let action = this._parseActionUrl(url);
 
-          let action = this._parseActionUrl(url);
-          if (action) {
+          if (selectedOneOff && selectedOneOff.engine) {
+            // If there's a selected one-off button then load a search using
+            // the one-off's engine.
+            [url, postData] =
+              this._parseAndRecordSearchEngineLoad(selectedOneOff.engine,
+                                                   this.oneOffSearchQuery,
+                                                   event, where,
+                                                   openUILinkParams);
+          } else if (action) {
             switch (action.type) {
               case "visiturl":
                 // Unifiedcomplete uses fixupURI to tell if something is a visit
                 // or a search, and passes out the fixedURI as the url param.
                 // By using that uri we would end up passing a different string
                 // to the docshell that may run a different not-found heuristic.
                 // For example, "mozilla/run" would be fixed by unifiedcomplete
                 // to "http://mozilla/run". The docshell, once it can't resolve
@@ -466,35 +474,16 @@ file, You can obtain one at http://mozil
                   action.params.searchSuggestion || action.params.searchQuery,
                   event,
                   where,
                   openUILinkParams,
                   actionDetails
                 );
                 break;
             }
-          } else if (selectedOneOff && selectedOneOff.engine) {
-            // If there's a selected one-off button and the input value is a
-            // search query (or "keyword" in URI-fixup terminology), then load a
-            // search using the one-off's engine.
-            let value = this.oneOffSearchQuery;
-            let fixup;
-            try {
-              fixup = Services.uriFixup.getFixupURIInfo(
-                value,
-                Services.uriFixup.FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP
-              );
-            } catch (ex) {}
-            if (!fixup || !fixup.keywordProviderName) {
-              return;
-            }
-
-            [url, postData] =
-              this._parseAndRecordSearchEngineLoad(selectedOneOff.engine, value,
-                                                   event, where, openUILinkParams);
           } else {
             // This is a fallback for add-ons and old testing code that directly
             // set value and try to confirm it. UnifiedComplete should always
             // resolve to a valid url.
             try {
               new URL(url);
             } catch (ex) {
               let lastLocationChange = browser.lastLocationChange;