Bug 1331736 - Use selected search suggestion with one-offs r=mak
authorDoug Thayer <dothayer@mozilla.com>
Wed, 07 Jun 2017 11:51:23 -0700
changeset 363298 f759d63d25b98c4e73a375fa2752b5be442e2435
parent 363297 d09e630c7054ca6e08200bbfc659784652ac7b3e
child 363299 7e925cbe797684129a52ecee13a1d7b38b1b2141
push id32002
push userarchaeopteryx@coole-files.de
push dateSat, 10 Jun 2017 09:14:38 +0000
treeherdermozilla-central@a305f9e66fa5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1331736
milestone55.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 1331736 - Use selected search suggestion with one-offs r=mak Currently in the location bar we don't match the search bar's behavior of allowing the user to use their currently selected search suggestion with a one-off search engine. This is due to logic which is intended to not search for autocompleted text, instead favoring only what the user has actually typed. This patch adds an exception to that logic for search suggestions. MozReview-Commit-ID: 13sA3r5zXvO
browser/base/content/test/urlbar/browser.ini
browser/base/content/test/urlbar/browser_urlbarOneOffs.js
browser/base/content/test/urlbar/browser_urlbarOneOffs_searchSuggestions.js
browser/base/content/test/urlbar/searchSuggestionEngine.xml
browser/base/content/urlbarBindings.xml
--- a/browser/base/content/test/urlbar/browser.ini
+++ b/browser/base/content/test/urlbar/browser.ini
@@ -68,16 +68,20 @@ support-files =
 skip-if = os == "linux" # Bug 1073339 - Investigate autocomplete test unreliability on Linux/e10s
 [browser_urlbarFocusedCmdK.js]
 [browser_urlbarHashChangeProxyState.js]
 [browser_urlbarKeepStateAcrossTabSwitches.js]
 [browser_urlbarOneOffs.js]
 support-files =
   searchSuggestionEngine.xml
   searchSuggestionEngine.sjs
+[browser_urlbarOneOffs_searchSuggestions.js]
+support-files =
+  searchSuggestionEngine.xml
+  searchSuggestionEngine.sjs
 [browser_urlbarPrivateBrowsingWindowChange.js]
 [browser_urlbarRaceWithTabs.js]
 [browser_urlbarRevert.js]
 [browser_urlbarSearchSingleWordNotification.js]
 [browser_urlbarSearchSuggestions.js]
 support-files =
   searchSuggestionEngine.xml
   searchSuggestionEngine.sjs
--- a/browser/base/content/test/urlbar/browser_urlbarOneOffs.js
+++ b/browser/base/content/test/urlbar/browser_urlbarOneOffs.js
@@ -191,17 +191,17 @@ add_task(async function oneOffClick() {
   let typedValue = "foo.bar";
   await promiseAutocompleteResultPopup(typedValue);
 
   assertState(0, -1, typedValue);
 
   let oneOffs = gURLBar.popup.oneOffSearchButtons.getSelectableButtons(true);
   let resultsPromise =
     BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser, false,
-                                   "http://mochi.test:8888/");
+                                   "http://mochi.test:8888/?terms=foo.bar");
   EventUtils.synthesizeMouseAtCenter(oneOffs[0], {});
   await resultsPromise;
 
   gBrowser.removeTab(gBrowser.selectedTab);
 });
 
 // Presses the Return key when a one-off is selected.
 add_task(async function oneOffReturn() {
@@ -215,17 +215,17 @@ add_task(async function oneOffReturn() {
   assertState(0, -1, typedValue);
 
   // Alt+Down to select the first one-off.
   EventUtils.synthesizeKey("VK_DOWN", { altKey: true })
   assertState(0, 0, typedValue);
 
   let resultsPromise =
     BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser, false,
-                                   "http://mochi.test:8888/");
+                                   "http://mochi.test:8888/?terms=foo.bar");
   EventUtils.synthesizeKey("VK_RETURN", {})
   await resultsPromise;
 
   gBrowser.removeTab(gBrowser.selectedTab);
 });
 
 
 function assertState(result, oneOff, textValue = undefined) {
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/urlbar/browser_urlbarOneOffs_searchSuggestions.js
@@ -0,0 +1,115 @@
+const TEST_ENGINE_BASENAME = "searchSuggestionEngine.xml";
+
+add_task(async function init() {
+  await PlacesTestUtils.clearHistory();
+  await SpecialPowers.pushPrefEnv({
+    set: [["browser.urlbar.oneOffSearches", true],
+          ["browser.urlbar.suggest.searches", true]],
+  });
+  let engine = await promiseNewSearchEngine(TEST_ENGINE_BASENAME);
+  let oldCurrentEngine = Services.search.currentEngine;
+  Services.search.moveEngine(engine, 0);
+  Services.search.currentEngine = engine;
+  registerCleanupFunction(async function() {
+    Services.search.currentEngine = oldCurrentEngine;
+
+    await PlacesTestUtils.clearHistory();
+    // Make sure the popup is closed for the next test.
+    gURLBar.blur();
+    Assert.ok(!gURLBar.popup.popupOpen, "popup should be closed");
+  });
+});
+
+// Presses the Return key when a one-off is selected after selecting a search
+// suggestion.
+add_task(async function oneOffReturnAfterSuggestion() {
+  let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser);
+
+  let typedValue = "foo";
+  await promiseAutocompleteResultPopup(typedValue, window, true);
+  await BrowserTestUtils.waitForCondition(suggestionsPresent,
+                                          "waiting for suggestions");
+  assertState(0, -1, typedValue);
+
+  // Down to select the first search suggestion.
+  EventUtils.synthesizeKey("VK_DOWN", {})
+  assertState(1, -1, "foofoo");
+
+  // Down to select the next search suggestion.
+  EventUtils.synthesizeKey("VK_DOWN", {})
+  assertState(2, -1, "foobar");
+
+  // Alt+Down to select the first one-off.
+  EventUtils.synthesizeKey("VK_DOWN", { altKey: true })
+  assertState(2, 0, "foobar");
+
+  let resultsPromise =
+    BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser, false,
+                                   `http://mochi.test:8888/?terms=foobar`);
+  EventUtils.synthesizeKey("VK_RETURN", {});
+  await resultsPromise;
+
+  await PlacesTestUtils.clearHistory();
+  await BrowserTestUtils.removeTab(tab);
+});
+
+// Clicks a one-off engine after selecting a search suggestion.
+add_task(async function oneOffClickAfterSuggestion() {
+  let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser);
+
+  let typedValue = "foo";
+  await promiseAutocompleteResultPopup(typedValue, window, true);
+  await BrowserTestUtils.waitForCondition(suggestionsPresent,
+                                          "waiting for suggestions");
+  assertState(0, -1, typedValue);
+
+  // Down to select the first search suggestion.
+  EventUtils.synthesizeKey("VK_DOWN", {})
+  assertState(1, -1, "foofoo");
+
+  // Down to select the next search suggestion.
+  EventUtils.synthesizeKey("VK_DOWN", {})
+  assertState(2, -1, "foobar");
+
+  let oneOffs = gURLBar.popup.oneOffSearchButtons.getSelectableButtons(true);
+  let resultsPromise =
+    BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser, false,
+                                   `http://mochi.test:8888/?terms=foobar`);
+  EventUtils.synthesizeMouseAtCenter(oneOffs[0], {});
+  await resultsPromise;
+
+  await PlacesTestUtils.clearHistory();
+  await BrowserTestUtils.removeTab(tab);
+});
+
+function assertState(result, oneOff, textValue = undefined) {
+  Assert.equal(gURLBar.popup.selectedIndex, result,
+               "Expected result should be selected");
+  Assert.equal(gURLBar.popup.oneOffSearchButtons.selectedButtonIndex, oneOff,
+               "Expected one-off should be selected");
+  if (textValue !== undefined) {
+    Assert.equal(gURLBar.textValue, textValue, "Expected textValue");
+  }
+}
+
+async function hidePopup() {
+  EventUtils.synthesizeKey("VK_ESCAPE", {});
+  await promisePopupHidden(gURLBar.popup);
+}
+
+function suggestionsPresent() {
+  let controller = gURLBar.popup.input.controller;
+  let matchCount = controller.matchCount;
+  for (let i = 0; i < matchCount; i++) {
+    let url = controller.getValueAt(i);
+    let mozActionMatch = url.match(/^moz-action:([^,]+),(.*)$/);
+    if (mozActionMatch) {
+      let [, type, paramStr] = mozActionMatch;
+      let params = JSON.parse(paramStr);
+      if (type == "searchengine" && "searchSuggestion" in params) {
+        return true;
+      }
+    }
+  }
+  return false;
+}
--- a/browser/base/content/test/urlbar/searchSuggestionEngine.xml
+++ b/browser/base/content/test/urlbar/searchSuggestionEngine.xml
@@ -1,9 +1,11 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!-- Any copyright is dedicated to the Public Domain.
    - http://creativecommons.org/publicdomain/zero/1.0/ -->
 
 <SearchPlugin xmlns="http://www.mozilla.org/2006/browser/search/">
 <ShortName>browser_searchSuggestionEngine searchSuggestionEngine.xml</ShortName>
 <Url type="application/x-suggestions+json" method="GET" template="http://mochi.test:8888/browser/browser/base/content/test/urlbar/searchSuggestionEngine.sjs?{searchTerms}"/>
-<Url type="text/html" method="GET" template="http://mochi.test:8888/" rel="searchform"/>
+<Url type="text/html" method="GET" template="http://mochi.test:8888/" rel="searchform">
+  <Param name="terms" value="{searchTerms}"/>
+</Url>
 </SearchPlugin>
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -666,16 +666,23 @@ file, You can obtain one at http://mozil
 
           this._loadURL(url, browser, postData, where, openUILinkParams,
                         mayInheritPrincipal);
         ]]></body>
       </method>
 
       <property name="oneOffSearchQuery">
         <getter><![CDATA[
+          // If the user has selected a search suggestion, chances are they
+          // want to use the one off search engine to search for that suggestion,
+          // not the string that they manually entered into the location bar.
+          let action = this._parseActionUrl(this.value);
+          if (action && action.type == "searchengine") {
+            return action.params.input;
+          }
           // this.textValue may be an autofilled string.  Search only with the
           // portion that the user typed, if any, by preferring the autocomplete
           // controller's searchString (including handleEnterInstance.searchString).
           return this.handleEnterSearchString ||
                  this.mController.searchString ||
                  this.textValue;
         ]]></getter>
       </property>