Bug 1299428 - Searching an URL looking string using a one-off button doesn't open a search result. r=adw draft
authorMarco Bonardo <mbonardo@mozilla.com>
Wed, 02 Nov 2016 18:32:38 +0100
changeset 434095 e4c60d7e950b02a822f8982fe08c35ff2a313414
parent 434094 c5546272d80ace66d3347000cd8fd3691792ec49
child 536022 1d6b931f752abecfadb35ff00ae924eccb652228
push id34721
push usermak77@bonardo.net
push dateFri, 04 Nov 2016 21:07:22 +0000
reviewersadw
bugs1299428
milestone52.0a1
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;