From 52c65880a47046ac82d221c7ced3c125e25d7e3d Mon Sep 17 00:00:00 2001
From: Harry Twyford <htwyford@mozilla.com>
Date: Thu, 14 May 2020 16:32:58 +0000
Subject: [PATCH] Bug 1636696 - Highlight suggestions only if the typed token
 prefixes a word in the suggestion. r=mak

Differential Revision: https://phabricator.services.mozilla.com/D74966
---
 browser/components/urlbar/UrlbarUtils.jsm     | 30 +++++++++++++++++--
 .../browser_autocomplete_a11y_label.js        |  4 +--
 .../unit/test_UrlbarUtils_getTokenMatches.js  | 19 ++++++++----
 3 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/browser/components/urlbar/UrlbarUtils.jsm b/browser/components/urlbar/UrlbarUtils.jsm
index e646cce70984..a8932ce505e7 100644
--- a/browser/components/urlbar/UrlbarUtils.jsm
+++ b/browser/components/urlbar/UrlbarUtils.jsm
@@ -275,8 +275,11 @@ var UrlbarUtils = {
    *
    * @param {array} tokens The tokens to search for.
    * @param {string} str The string to match against.
-   * @param {boolean} highlightType If HIGHLIGHT.SUGGESTED, return a list of all
-   *   the token string non-matches. Otherwise, return matches.
+   * @param {boolean} highlightType
+   *   One of the HIGHLIGHT values:
+   *     TYPED: match ranges matching the tokens; or
+   *     SUGGESTED: match ranges for words not matching the tokens and the
+   *                endings of words that start with a token.
    * @returns {array} An array: [
    *            [matchIndex_0, matchLength_0],
    *            [matchIndex_1, matchLength_1],
@@ -289,7 +292,7 @@ var UrlbarUtils = {
     str = str.toLocaleLowerCase();
     // To generate non-overlapping ranges, we start from a 0-filled array with
     // the same length of the string, and use it as a collision marker, setting
-    // 1 where a token matches.
+    // 1 where the text should be highlighted.
     let hits = new Array(str.length).fill(
       highlightType == this.HIGHLIGHT.SUGGESTED ? 1 : 0
     );
@@ -308,6 +311,20 @@ var UrlbarUtils = {
         if (index < 0) {
           break;
         }
+
+        if (highlightType == UrlbarUtils.HIGHLIGHT.SUGGESTED) {
+          // We de-emphasize the match only if it's preceded by a space, thus
+          // it's a perfect match or the beginning of a longer word.
+          let previousSpaceIndex = str.lastIndexOf(" ", index) + 1;
+          if (index != previousSpaceIndex) {
+            index += needle.length;
+            // We found the token but we won't de-emphasize it, because it's not
+            // after a word boundary.
+            found = true;
+            continue;
+          }
+        }
+
         hits.fill(
           highlightType == this.HIGHLIGHT.SUGGESTED ? 0 : 1,
           index,
@@ -335,6 +352,13 @@ var UrlbarUtils = {
         while (index < str.length) {
           let hay = str.substr(index, needle.length);
           if (compareIgnoringDiacritics(needle, hay) === 0) {
+            if (highlightType == UrlbarUtils.HIGHLIGHT.SUGGESTED) {
+              let previousSpaceIndex = str.lastIndexOf(" ", index) + 1;
+              if (index != previousSpaceIndex) {
+                index += needle.length;
+                continue;
+              }
+            }
             hits.fill(
               highlightType == this.HIGHLIGHT.SUGGESTED ? 0 : 1,
               index,
diff --git a/browser/components/urlbar/tests/browser/browser_autocomplete_a11y_label.js b/browser/components/urlbar/tests/browser/browser_autocomplete_a11y_label.js
index 0344424fa1de..d02dba6dcc16 100644
--- a/browser/components/urlbar/tests/browser/browser_autocomplete_a11y_label.js
+++ b/browser/components/urlbar/tests/browser/browser_autocomplete_a11y_label.js
@@ -105,9 +105,9 @@ add_task(async function searchSuggestions() {
   );
   // The first expected search is the search term itself since the heuristic
   // result will come before the search suggestions.
-  // The extra space is here due to bug 1550644.
+  // The extra spaces are here due to bug 1550644.
   let searchTerm = "foo ";
-  let expectedSearches = [searchTerm, "foofoo ", "foo bar"];
+  let expectedSearches = [searchTerm, "foo foo", "foo bar"];
   for (let i = 0; i < length; i++) {
     let result = await UrlbarTestUtils.getDetailsOfResultAt(window, i);
     if (result.type === UrlbarUtils.RESULT_TYPE.SEARCH) {
diff --git a/browser/components/urlbar/tests/unit/test_UrlbarUtils_getTokenMatches.js b/browser/components/urlbar/tests/unit/test_UrlbarUtils_getTokenMatches.js
index 9ffce25a827d..bae6ffc879a0 100644
--- a/browser/components/urlbar/tests/unit/test_UrlbarUtils_getTokenMatches.js
+++ b/browser/components/urlbar/tests/unit/test_UrlbarUtils_getTokenMatches.js
@@ -211,7 +211,11 @@ add_task(function test() {
   }
 });
 
-add_task(function testReverse() {
+/**
+ * Tests suggestion highlighting. Note that suggestions are only highlighted if
+ * the matching token is at the beginning of a word in the matched string.
+ */
+add_task(function testSuggestions() {
   const tests = [
     {
       tokens: ["mozilla", "is", "i"],
@@ -232,12 +236,12 @@ add_task(function testReverse() {
     {
       tokens: ["mo", "zilla"],
       phrase: "mOzIlLa",
-      expected: [],
+      expected: [[2, 5]],
     },
     {
       tokens: ["MO", "ZILLA"],
       phrase: "mozilla",
-      expected: [],
+      expected: [[2, 5]],
     },
     {
       tokens: [""], // Should never happen in practice.
@@ -245,27 +249,30 @@ add_task(function testReverse() {
       expected: [[0, 7]],
     },
     {
-      tokens: ["mo", "om"],
+      tokens: ["mo", "om", "la"],
       phrase: "mozilla mozzarella momo",
       expected: [
         [2, 6],
         [10, 9],
+        [21, 2],
       ],
     },
     {
-      tokens: ["mo", "om"],
+      tokens: ["mo", "om", "la"],
       phrase: "MOZILLA MOZZARELLA MOMO",
       expected: [
         [2, 6],
         [10, 9],
+        [21, 2],
       ],
     },
     {
-      tokens: ["MO", "OM"],
+      tokens: ["MO", "OM", "LA"],
       phrase: "mozilla mozzarella momo",
       expected: [
         [2, 6],
         [10, 9],
+        [21, 2],
       ],
     },
   ];