Bug 1410240 - Search suggestions keep displacing awesomebar results as I'm about to click on them. r=mak

MozReview-Commit-ID: 2NdV9qWzld1

--HG--
extra : rebase_source : 29191d546141d4af4ac1e4902f6981488f480c71

browser/base/content/test/urlbar/browser.ini

@@ -128,3 +128,7 @@ run-if = e10s
 subsuite = clipboard
 support-files =
   test_wyciwyg_copying.html
+[browser_urlbarStopSearchOnSelection.js]
+support-files =
+  searchSuggestionEngineSlow.xml
+  searchSuggestionEngine.sjs

browser/base/content/test/urlbar/browser_urlbarOneOffs_searchSuggestions.js

@@ -27,8 +27,7 @@ add_task(async function oneOffReturnAfterSuggestion() {
 
   let typedValue = "foo";
   await promiseAutocompleteResultPopup(typedValue, window, true);
-  await BrowserTestUtils.waitForCondition(suggestionsPresent,
-                                          "waiting for suggestions");
+  await promiseSuggestionsPresent();
   assertState(0, -1, typedValue);
 
   // Down to select the first search suggestion.
@@ -59,8 +58,7 @@ add_task(async function oneOffClickAfterSuggestion() {
 
   let typedValue = "foo";
   await promiseAutocompleteResultPopup(typedValue, window, true);
-  await BrowserTestUtils.waitForCondition(suggestionsPresent,
-                                          "waiting for suggestions");
+  await promiseSuggestionsPresent();
   assertState(0, -1, typedValue);
 
   // Down to select the first search suggestion.
@@ -87,8 +85,7 @@ add_task(async function overridden_engine_not_reused() {
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser);
     let typedValue = "foo";
     await promiseAutocompleteResultPopup(typedValue, window, true);
-    await BrowserTestUtils.waitForCondition(suggestionsPresent,
-                                            "waiting for suggestions");
+    await promiseSuggestionsPresent();
     // Down to select the first search suggestion.
     EventUtils.synthesizeKey("VK_DOWN", {});
     assertState(1, -1, "foofoo");
@@ -100,8 +97,7 @@ add_task(async function overridden_engine_not_reused() {
     let label = gURLBar.popup.richlistbox.children[gURLBar.popup.richlistbox.selectedIndex].label;
     // Run again the query, check the label has been replaced.
     await promiseAutocompleteResultPopup(typedValue, window, true);
-    await BrowserTestUtils.waitForCondition(suggestionsPresent,
-                                            "waiting for suggestions");
+    await promiseSuggestionsPresent();
     assertState(0, -1, "foo");
     let newLabel = gURLBar.popup.richlistbox.children[1].label;
     Assert.notEqual(newLabel, label, "The label should have been updated");
@@ -123,20 +119,3 @@ 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;
-}

browser/base/content/test/urlbar/browser_urlbarSearchSuggestions_opt-out.js

@@ -139,23 +139,6 @@ function setupVisibleHint() {
   Services.prefs.setBoolPref(SUGGEST_URLBAR_PREF, true);
 }
 
-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;
-}
-
 function assertVisible(visible, win = window) {
   let style =
     win.getComputedStyle(win.gURLBar.popup.searchSuggestionsNotification);

browser/base/content/test/urlbar/browser_urlbarStopSearchOnSelection.js

@@ -0,0 +1,88 @@
+/* eslint-disable mozilla/no-arbitrary-setTimeout */
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const TEST_ENGINE_BASENAME = "searchSuggestionEngineSlow.xml";
+
+// This should match the `timeout` query param used in the suggestions URL in
+// the test engine.
+const TEST_ENGINE_SUGGESTIONS_TIMEOUT = 1000;
+
+// The number of suggestions returned by the test engine.
+const TEST_ENGINE_NUM_EXPECTED_RESULTS = 2;
+
+add_task(async function init() {
+  await PlacesUtils.history.clear();
+  await SpecialPowers.pushPrefEnv({
+    set: [["browser.urlbar.suggest.searches", true]],
+  });
+  // Add a test search engine that returns suggestions on a delay.
+  let engine = await promiseNewSearchEngine(TEST_ENGINE_BASENAME);
+  let oldCurrentEngine = Services.search.currentEngine;
+  Services.search.moveEngine(engine, 0);
+  Services.search.currentEngine = engine;
+  registerCleanupFunction(async () => {
+    Services.search.currentEngine = oldCurrentEngine;
+    await PlacesUtils.history.clear();
+    // Make sure the popup is closed for the next test.
+    gURLBar.blur();
+    Assert.ok(!gURLBar.popup.popupOpen, "popup should be closed");
+  });
+});
+
+add_task(async function mainTest() {
+  // Trigger an initial search.  Use the "$" token to restrict matches to search
+  // suggestions.
+  await promiseAutocompleteResultPopup("$ test", window);
+  await promiseSuggestionsPresent("Waiting for initial suggestions");
+
+  // Now synthesize typing a character.  promiseAutocompleteResultPopup doesn't
+  // work in this case because it causes the popup to close and re-open with the
+  // new input text.
+  await new Promise(r => EventUtils.synthesizeKey("x", {}, window, r));
+
+  // At this point, a new search starts, the autocomplete's _invalidate is
+  // called, _appendCurrentResult is called, and matchCount remains 3 from the
+  // previous search (the heuristic result plus the two search suggestions).
+  // The suggestion results should not outwardly change, however, since the
+  // autocomplete controller should still be returning the initial suggestions.
+  // What has changed, though, is the search string, which is kept in the
+  // results' ac-text attributes.  Call waitForAutocompleteResultAt to wait for
+  // the results' ac-text to be set to the new search string, "testx", to make
+  // sure the autocomplete has seen the new search.
+  await waitForAutocompleteResultAt(TEST_ENGINE_NUM_EXPECTED_RESULTS);
+
+  // Now press the Down arrow key to change the selection.
+  await new Promise(r => EventUtils.synthesizeKey("VK_DOWN", {}, window, r));
+
+  // Changing the selection should have stopped the new search triggered by
+  // typing the "x" character.  Wait a bit to make sure it really stopped.
+  await new Promise(r => setTimeout(r, 2 * TEST_ENGINE_SUGGESTIONS_TIMEOUT));
+
+  // Both of the suggestion results should retain their initial values,
+  // "testfoo" and "testbar".  They should *not* be "testxfoo" and "textxbar".
+
+  // + 1 for the heuristic result
+  let numExpectedResults = TEST_ENGINE_NUM_EXPECTED_RESULTS + 1;
+  let results = gURLBar.popup.richlistbox.children;
+  let numActualResults = Array.reduce(results, (memo, result) => {
+    if (!result.collapsed) {
+      memo++;
+    }
+    return memo;
+  }, 0);
+  Assert.equal(numActualResults, numExpectedResults);
+
+  let expectedSuggestions = ["testfoo", "testbar"];
+  for (let i = 0; i < TEST_ENGINE_NUM_EXPECTED_RESULTS; i++) {
+    // + 1 to skip the heuristic result
+    let item = gURLBar.popup.richlistbox.children[i + 1];
+    let action = item._parseActionUrl(item.getAttribute("url"));
+    Assert.ok(action);
+    Assert.equal(action.type, "searchengine");
+    Assert.ok("searchSuggestion" in action.params);
+    Assert.equal(action.params.searchSuggestion, expectedSuggestions[i]);
+  }
+});

browser/base/content/test/urlbar/head.js

@@ -324,3 +324,25 @@ async function waitForAutocompleteResultAt(index) {
   await new Promise(resolve => window.requestIdleCallback(resolve, {timeout: 1000}));
   return gURLBar.popup.richlistbox.children[index];
 }
+
+function promiseSuggestionsPresent(msg = "") {
+  return TestUtils.waitForCondition(suggestionsPresent,
+                                    msg || "Waiting for suggestions");
+}
+
+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;
+}

browser/base/content/test/urlbar/searchSuggestionEngine.sjs

@@ -1,9 +1,48 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
+const { classes: Cc, interfaces: Ci } = Components;
+
+let gTimer;
+
 function handleRequest(req, resp) {
+  // Parse the query params.  If the params aren't in the form "foo=bar", then
+  // treat the entire query string as a search string.
+  let params = req.queryString.split("&").reduce((memo, pair) => {
+    let [key, val] = pair.split("=");
+    if (!val) {
+      // This part isn't in the form "foo=bar".  Treat it as the search string
+      // (the "query").
+      val = key;
+      key = "query";
+    }
+    memo[decode(key)] = decode(val);
+    return memo;
+  }, {});
+
+  let timeout = parseInt(params["timeout"]);
+  if (timeout) {
+    // Write the response after a timeout.
+    resp.processAsync();
+    gTimer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
+    gTimer.init(() => {
+      writeResponse(params, resp);
+      resp.finish();
+    }, timeout, Ci.nsITimer.TYPE_ONE_SHOT);
+    return;
+  }
+
+  writeResponse(params, resp);
+}
+
+function writeResponse(params, resp) {
+  // Echo back the search string with "foo" and "bar" appended.
   let suffixes = ["foo", "bar"];
-  let data = [req.queryString, suffixes.map(s => req.queryString + s)];
+  let data = [params["query"], suffixes.map(s => params["query"] + s)];
   resp.setHeader("Content-Type", "application/json", false);
   resp.write(JSON.stringify(data));
 }
+
+function decode(str) {
+  return decodeURIComponent(str.replace(/\+/g, encodeURIComponent(" ")));
+}

browser/base/content/test/urlbar/searchSuggestionEngineSlow.xml

@@ -0,0 +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>searchSuggestionEngineSlow.xml</ShortName>
+<Url type="application/x-suggestions+json" method="GET" template="http://mochi.test:8888/browser/browser/base/content/test/urlbar/searchSuggestionEngine.sjs?query={searchTerms}&amp;timeout=1000"/>
+<Url type="text/html" method="GET" template="http://mochi.test:8888/" rel="searchform">
+  <Param name="terms" value="{searchTerms}"/>
+</Url>
+</SearchPlugin>

toolkit/components/autocomplete/nsAutoCompleteController.cpp

@@ -502,6 +502,11 @@ nsAutoCompleteController::HandleKeyNavigation(uint32_t aKey, bool *_retval)
       bool completeSelection;
       input->GetCompleteSelectedIndex(&completeSelection);
 
+      // The user has keyed up or down to change the selection.  Stop the search
+      // (if there is one) now so that the results do not change while the user
+      // is making a selection.
+      Unused << StopSearch();
+
       // Instruct the result view to scroll by the given amount and direction
       popup->SelectBy(reverse, page);