Bug 1521366 - Searching for a space in the Quantum Bar causes an infinite loop. r=adw
authorMarco Bonardo <mbonardo@mozilla.com>
Fri, 15 Feb 2019 20:55:30 +0000
changeset 459603 6c8c7b1f4e2cf205e28b5e27145ca8acbc3f2452
parent 459602 db4946736508bb1cc3cbb288cdb498703fb4d904
child 459604 8e1840b7a9923ac6e7e4c143edf87167b4cac717
push id35563
push userccoroiu@mozilla.com
push dateSat, 16 Feb 2019 09:36:04 +0000
treeherdermozilla-central@1cfd69d05aa1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs1521366
milestone67.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 1521366 - Searching for a space in the Quantum Bar causes an infinite loop. r=adw Differential Revision: https://phabricator.services.mozilla.com/D19979
browser/components/urlbar/UrlbarTokenizer.jsm
browser/components/urlbar/UrlbarUtils.jsm
browser/components/urlbar/UrlbarView.jsm
browser/components/urlbar/tests/UrlbarTestUtils.jsm
browser/components/urlbar/tests/browser/browser.ini
browser/components/urlbar/tests/browser/browser_urlbar_empty_search.js
browser/components/urlbar/tests/legacy/browser.ini
browser/components/urlbar/tests/unit/test_tokenizer.js
--- a/browser/components/urlbar/UrlbarTokenizer.jsm
+++ b/browser/components/urlbar/UrlbarTokenizer.jsm
@@ -168,17 +168,17 @@ var UrlbarTokenizer = {
    * @param {UrlbarQueryContext} queryContext
    *        The query context object to tokenize
    * @returns {UrlbarQueryContext} the same query context object with a new
    *          tokens property.
    */
   tokenize(queryContext) {
     logger.info("Tokenizing", queryContext);
     let searchString = queryContext.searchString;
-    if (searchString.length == 0) {
+    if (!searchString.trim()) {
       queryContext.tokens = [];
       return queryContext;
     }
 
     let unfiltered = splitString(searchString);
     let tokens = filterTokens(unfiltered);
     queryContext.tokens = tokens;
     return queryContext;
--- a/browser/components/urlbar/UrlbarUtils.jsm
+++ b/browser/components/urlbar/UrlbarUtils.jsm
@@ -216,17 +216,19 @@ var UrlbarUtils = {
    *            ...
    *            [matchIndex_n, matchLength_n]
    *          ].
    *          The array is sorted by match indexes ascending.
    */
   getTokenMatches(tokens, str) {
     return tokens.reduce((matches, token) => {
       let index = 0;
-      while (index >= 0) {
+      // Ideally we should never hit the empty token case, but just in case
+      // the value check protects us from an infinite loop.
+      while (index >= 0 && token.value) {
         index = str.indexOf(token.value, index);
         if (index >= 0) {
           let match = [index, token.value.length];
           let matchesIndex = BinarySearch.insertionIndexOf((a, b) => {
             return a[0] - b[0];
           }, matches, match);
           matches.splice(matchesIndex, 0, match);
           index += token.value.length;
--- a/browser/components/urlbar/UrlbarView.jsm
+++ b/browser/components/urlbar/UrlbarView.jsm
@@ -405,17 +405,17 @@ class UrlbarView {
                                               [result.payload.engine], 1));
         break;
       case UrlbarUtils.RESULT_TYPE.KEYWORD:
         if (result.payload.input.trim() == result.payload.keyword) {
           setAction(bundle.GetStringFromName("visit"));
         }
         break;
       default:
-        if (resultIndex == 0) {
+        if (result.heuristic) {
           setAction(bundle.GetStringFromName("visit"));
         } else {
           setURL();
         }
         break;
     }
     if (action) {
       content.appendChild(action);
--- a/browser/components/urlbar/tests/UrlbarTestUtils.jsm
+++ b/browser/components/urlbar/tests/UrlbarTestUtils.jsm
@@ -370,17 +370,17 @@ class UrlbarAbstraction {
       if (index >= context.results.length) {
         throw new Error("Requested index not found in results");
       }
       let {url, postData} = UrlbarUtils.getUrlFromResult(context.results[index]);
       details.url = url;
       details.postData = postData;
       details.type = context.results[index].type;
       details.heuristic = context.results[index].heuristic;
-      details.autofill = index == 0 && context.results[index].autofill;
+      details.autofill = !!context.results[index].autofill;
       details.image = element.getElementsByClassName("urlbarView-favicon")[0].src;
       details.title = context.results[index].title;
       details.tags = "tags" in context.results[index].payload ?
         context.results[index].payload.tags :
         [];
       let actions = element.getElementsByClassName("urlbarView-action");
       let typeIcon = element.querySelector(".urlbarView-type-icon");
       let typeIconStyle = this.window.getComputedStyle(typeIcon);
@@ -402,22 +402,22 @@ class UrlbarAbstraction {
 
       let style = this.urlbar.controller.getStyleAt(index);
       let action = PlacesUtils.parseActionUrl(this.urlbar.controller.getValueAt(index));
       details.type = getType(style, action);
       details.heuristic = style.includes("heuristic");
       details.autofill = style.includes("autofill");
       details.image = element.getAttribute("image");
       details.title = element.getAttribute("title");
-      details.tags = [...element.getElementsByClassName("ac-tags")].map(e =>
-        e.textContent);
+      details.tags = style.includes("tag") ?
+        [...element.getElementsByClassName("ac-tags")].map(e => e.textContent) : [];
       let typeIconStyle = this.window.getComputedStyle(element._typeIcon);
       details.displayed = {
         title: element._titleText.textContent,
-        action: element._actionText.textContent,
+        action: action ? element._actionText.textContent : "",
         typeIcon: typeIconStyle.listStyleImage,
       };
       if (details.type == UrlbarUtils.RESULT_TYPE.SEARCH) {
         details.searchParams = {
           engine: action.params.engineName,
           keyword: action.params.alias,
           query: action.params.input,
           suggestion: action.params.searchSuggestion,
--- a/browser/components/urlbar/tests/browser/browser.ini
+++ b/browser/components/urlbar/tests/browser/browser.ini
@@ -53,16 +53,17 @@ skip-if = true # Bug 1526222 - Doesn't currently work with QuantumBar
 skip-if = os == 'linux' # Bug 1104755 (Intermittent failure)
 [browser_tabMatchesInAwesomebar.js]
 support-files =
   moz.png
 [browser_urlbar_blanking.js]
 support-files =
   file_blank_but_not_blank.html
 [browser_urlbar_content_opener.js]
+[browser_urlbar_empty_search.js]
 [browser_urlbar_locationchange_urlbar_edit_dos.js]
 support-files =
   file_urlbar_edit_dos.html
 [browser_urlbar_remoteness_switch.js]
 run-if = e10s
 [browser_urlbar_remove_match.js]
 [browser_urlbar_searchsettings.js]
 [browser_urlbar_speculative_connect.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/urlbar/tests/browser/browser_urlbar_empty_search.js
@@ -0,0 +1,47 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+// This test ensures that a search for "empty" strings doesn't break the urlbar.
+
+add_task(async function test_setup() {
+  await PlacesTestUtils.addVisits([
+    {
+      uri: `http://one.mozilla.org/`,
+      transition: PlacesUtils.history.TRANSITIONS.TYPED,
+    },
+    {
+      uri: `http://two.mozilla.org/`,
+      transition: PlacesUtils.history.TRANSITIONS.TYPED,
+    },
+  ]);
+  registerCleanupFunction(PlacesUtils.history.clear);
+});
+
+add_task(async function test_empty() {
+  info("Test searching for nothing");
+  await promiseAutocompleteResultPopup("", window, true);
+  // The first search collects the results, the following ones check results
+  // are the same.
+  let results = [{}]; // Add a fake first result, to account for heuristic.
+  for (let i = 0; i < await UrlbarTestUtils.getResultCount(window); ++i) {
+    results.push(await UrlbarTestUtils.getDetailsOfResultAt(window, i));
+  }
+
+  for (let str of [" ", "  "]) {
+    info(`Test searching for "${str}"`);
+    await promiseAutocompleteResultPopup(str, window, true);
+    // Skip the heuristic result.
+    Assert.ok((await UrlbarTestUtils.getDetailsOfResultAt(window, 0)).heuristic,
+              "The first result is heuristic");
+    let length = await UrlbarTestUtils.getResultCount(window);
+    Assert.equal(length, results.length, "Comparing results count");
+    for (let i = 1; i < length; ++i) {
+      Assert.deepEqual(await UrlbarTestUtils.getDetailsOfResultAt(window, i),
+                        results[i],
+                       `Comparing result at index ${i}`);
+    }
+  }
+});
--- a/browser/components/urlbar/tests/legacy/browser.ini
+++ b/browser/components/urlbar/tests/legacy/browser.ini
@@ -95,16 +95,17 @@ subsuite = clipboard
 support-files =
   ../browser/authenticate.sjs
 [../browser/browser_urlbarCutting.js]
 [../browser/browser_urlbarDecode.js]
 [../browser/browser_urlbar_blanking.js]
 support-files =
   ../browser/file_blank_but_not_blank.html
 [../browser/browser_urlbar_content_opener.js]
+[../browser/browser_urlbar_empty_search.js]
 [../browser/browser_urlbar_locationchange_urlbar_edit_dos.js]
 support-files =
   ../browser/file_urlbar_edit_dos.html
 [../browser/browser_urlbarDelete.js]
 [../browser/browser_urlbarEnter.js]
 [../browser/browser_urlbarEnterAfterMouseOver.js]
 skip-if = os == "linux" # Bug 1073339 - Investigate autocomplete test unreliability on Linux/e10s
 [../browser/browser_urlbar_whereToOpen.js]
--- a/browser/components/urlbar/tests/unit/test_tokenizer.js
+++ b/browser/components/urlbar/tests/unit/test_tokenizer.js
@@ -2,16 +2,20 @@
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 add_task(async function test_tokenizer() {
   let testContexts = [
     { desc: "Empty string",
       searchString: "",
       expectedTokens: [],
     },
+    { desc: "Spaces string",
+      searchString: "      ",
+      expectedTokens: [],
+    },
     { desc: "Single word string",
       searchString: "test",
       expectedTokens: [
         { value: "test", type: UrlbarTokenizer.TYPE.POSSIBLE_ORIGIN },
       ],
     },
     { desc: "Multi word string with mixed whitespace types",
       searchString: " test1 test2\u1680test3\u2004test4\u1680",