Bug 1626891 - Style tail suggestions differently. r=mak

Differential Revision: https://phabricator.services.mozilla.com/D74740

browser/components/extensions/test/xpcshell/test_ext_urlbar.js

@@ -286,7 +286,9 @@ add_task(async function test_onProviderResultsRequested() {
         query: "test",
         engine: "Test engine",
         suggestion: undefined,
+        tailPrefix: undefined,
         tail: undefined,
+        tailOffsetIndex: -1,
         keyword: undefined,
         isSearchHistory: false,
         icon: "",

browser/components/urlbar/UrlbarProviderSearchSuggestions.jsm

@@ -369,6 +369,19 @@ class ProviderSearchSuggestions extends UrlbarProvider {
         continue;
       }
 
+      if (suggestion.entry.tail && suggestion.entry.tailOffsetIndex < 0) {
+        Cu.reportError(
+          `Error in tail suggestion parsing. Value: ${suggestion.entry.value}, tail: ${suggestion.entry.tail}.`
+        );
+        continue;
+      }
+
+      let tail, tailPrefix;
+      if (UrlbarPrefs.get("richSuggestions.tail")) {
+        tail = suggestion.entry.tail;
+        tailPrefix = suggestion.entry.matchPrefix;
+      }
+
       try {
         results.push(
           new UrlbarResult(
@@ -380,14 +393,9 @@ class ProviderSearchSuggestions extends UrlbarProvider {
                 suggestion.entry.value,
                 UrlbarUtils.HIGHLIGHT.SUGGESTED,
               ],
-              tail: [
+              tailPrefix,
-                UrlbarPrefs.get("richSuggestions.tail") &&
+              tail: [tail, UrlbarUtils.HIGHLIGHT.SUGGESTED],
-                suggestion.entry.matchPrefix &&
+              tailOffsetIndex: suggestion.entry.tailOffsetIndex,
-                suggestion.entry.tail
-                  ? suggestion.entry.matchPrefix + suggestion.entry.tail
-                  : undefined,
-                UrlbarUtils.HIGHLIGHT.SUGGESTED,
-              ],
               keyword: [alias ? alias : undefined, UrlbarUtils.HIGHLIGHT.TYPED],
               query: [searchString.trim(), UrlbarUtils.HIGHLIGHT.NONE],
               isSearchHistory: false,

browser/components/urlbar/UrlbarProviderUnifiedComplete.jsm

@@ -209,7 +209,9 @@ function makeUrlbarResult(tokens, info) {
               UrlbarUtils.HIGHLIGHT.SUGGESTED,
             ],
             // For test interoperabilty with UrlbarProviderSearchSuggestions.
+            tailPrefix: undefined,
             tail: undefined,
+            tailOffsetIndex: -1,
             keyword: [action.params.alias, UrlbarUtils.HIGHLIGHT.TYPED],
             query: [
               action.params.searchQuery.trim(),

browser/components/urlbar/UrlbarResult.jsm

@@ -128,7 +128,7 @@ class UrlbarResult {
           case UrlbarUtils.KEYWORD_OFFER.HIDE:
             return ["", []];
         }
-        if (this.payload.tail) {
+        if (this.payload.tail && this.payload.tailOffsetIndex >= 0) {
           return [this.payload.tail, this.payloadHighlights.tail];
         } else if (this.payload.suggestion) {
           return [this.payload.suggestion, this.payloadHighlights.suggestion];

browser/components/urlbar/UrlbarUtils.jsm

@@ -645,6 +645,12 @@ UrlbarUtils.RESULT_PAYLOAD_SCHEMA = {
       tail: {
         type: "string",
       },
+      tailPrefix: {
+        type: "string",
+      },
+      tailOffsetIndex: {
+        type: "number",
+      },
       title: {
         type: "string",
       },

browser/components/urlbar/UrlbarView.jsm

@@ -759,6 +759,24 @@ class UrlbarView {
     typeIcon.className = "urlbarView-type-icon";
     noWrap.appendChild(typeIcon);
 
+    let tailPrefix = this._createElement("span");
+    tailPrefix.className = "urlbarView-tail-prefix";
+    noWrap.appendChild(tailPrefix);
+    item._elements.set("tailPrefix", tailPrefix);
+    // tailPrefix holds text only for alignment purposes so it should never be
+    // read to screen readers.
+    tailPrefix.toggleAttribute("aria-hidden", true);
+
+    let tailPrefixStr = this._createElement("span");
+    tailPrefixStr.className = "urlbarView-tail-prefix-string";
+    tailPrefix.appendChild(tailPrefixStr);
+    item._elements.set("tailPrefixStr", tailPrefixStr);
+
+    let tailPrefixChar = this._createElement("span");
+    tailPrefixChar.className = "urlbarView-tail-prefix-char";
+    tailPrefix.appendChild(tailPrefixChar);
+    item._elements.set("tailPrefixChar", tailPrefixChar);
+
     let title = this._createElement("span");
     title.className = "urlbarView-title";
     noWrap.appendChild(title);
@@ -896,6 +914,16 @@ class UrlbarView {
       result.title,
       result.titleHighlights
     );
+
+    if (result.payload.tail && result.payload.tailOffsetIndex >= 0) {
+      this._fillTailSuggestionPrefix(item, result);
+      title.setAttribute("aria-label", result.payload.suggestion);
+      item.toggleAttribute("tail-suggestion", true);
+    } else {
+      item.removeAttribute("tail-suggestion");
+      title.removeAttribute("aria-label");
+    }
+
     title._tooltip = result.title;
     if (title.hasAttribute("overflow")) {
       title.setAttribute("title", title._tooltip);
@@ -1325,6 +1353,25 @@ class UrlbarView {
     }
   }
 
+  /**
+   * Adds markup for a tail suggestion prefix to a row.
+   * @param {Node} item
+   *   The node for the result row.
+   * @param {UrlbarResult} result
+   *   A UrlbarResult representing a tail suggestion.
+   */
+  _fillTailSuggestionPrefix(item, result) {
+    let tailPrefixStrNode = item._elements.get("tailPrefixStr");
+    let tailPrefixStr = result.payload.suggestion.substring(
+      0,
+      result.payload.tailOffsetIndex
+    );
+    tailPrefixStrNode.textContent = tailPrefixStr;
+
+    let tailPrefixCharNode = item._elements.get("tailPrefixChar");
+    tailPrefixCharNode.textContent = result.payload.tailPrefix;
+  }
+
   _enableOrDisableOneOffSearches(enable = true) {
     if (enable) {
       this.oneOffSearchButtons.telemetryOrigin = "urlbar";

browser/components/urlbar/tests/unit/head.js

@@ -303,7 +303,9 @@ function makeSearchResult(
   queryContext,
   {
     suggestion,
+    tailPrefix,
     tail,
+    tailOffsetIndex,
     engineName,
     alias,
     query,
@@ -323,13 +325,24 @@ function makeSearchResult(
     }
   }
 
+  // Tail suggestion common cases, handled here to reduce verbosity in tests.
+  if (tail && !tailPrefix) {
+    tailPrefix = "… ";
+  }
+
+  if (!tailOffsetIndex) {
+    tailOffsetIndex = tail ? suggestion.indexOf(tail) : -1;
+  }
+
   let result = new UrlbarResult(
     UrlbarUtils.RESULT_TYPE.SEARCH,
     UrlbarUtils.RESULT_SOURCE.SEARCH,
     ...UrlbarResult.payloadAndSimpleHighlights(queryContext.tokens, {
       engine: [engineName, UrlbarUtils.HIGHLIGHT.TYPED],
       suggestion: [suggestion, UrlbarUtils.HIGHLIGHT.SUGGESTED],
+      tailPrefix,
       tail: [tail, UrlbarUtils.HIGHLIGHT.SUGGESTED],
+      tailOffsetIndex,
       keyword: [alias, UrlbarUtils.HIGHLIGHT.TYPED],
       // Check against undefined so consumers can pass in the empty string.
       query: [

browser/components/urlbar/tests/unit/test_search_suggestions_tail.js

@@ -136,12 +136,12 @@ add_task(async function basic_tail() {
       makeSearchResult(context, {
         engineName: ENGINE_NAME,
         suggestion: query + "oronto",
-        tail: "… toronto",
+        tail: "toronto",
       }),
       makeSearchResult(context, {
         engineName: ENGINE_NAME,
         suggestion: query + "unisia",
-        tail: "… tunisia",
+        tail: "tunisia",
       }),
     ],
   });
@@ -188,12 +188,12 @@ add_task(async function mixed_results() {
       makeSearchResult(context, {
         engineName: ENGINE_NAME,
         suggestion: query + "oronto",
-        tail: "… toronto",
+        tail: "toronto",
       }),
       makeSearchResult(context, {
         engineName: ENGINE_NAME,
         suggestion: query + "unisia",
-        tail: "… tunisia",
+        tail: "tunisia",
       }),
     ],
   });
@@ -222,7 +222,7 @@ add_task(async function dedupe_local() {
       makeSearchResult(context, {
         engineName: ENGINE_NAME,
         suggestion: query + "unisia",
-        tail: "… tunisia",
+        tail: "tunisia",
       }),
     ],
   });
@@ -246,7 +246,7 @@ add_task(async function limit_results() {
       makeSearchResult(context, {
         engineName: ENGINE_NAME,
         suggestion: query + "oronto",
-        tail: "… toronto",
+        tail: "toronto",
       }),
     ],
   });

browser/themes/shared/urlbarView.inc.css

@@ -365,6 +365,25 @@
   flex-shrink: 1;
 }
 
+/* Tail suggestions */
+.urlbarView-tail-prefix {
+  display: none;
+  justify-content: flex-end;
+  white-space: pre;
+}
+
+.urlbarView-row[tail-suggestion] > .urlbarView-row-inner > .urlbarView-no-wrap > .urlbarView-tail-prefix {
+  display: inline-flex;
+}
+
+.urlbarView-tail-prefix > .urlbarView-tail-prefix-string {
+  visibility: hidden;
+}
+
+.urlbarView-tail-prefix > .urlbarView-tail-prefix-char {
+  position: absolute;
+}
+
 /* Title separator */
 
 .urlbarView-title-separator::before {

toolkit/components/search/SearchSuggestionController.jsm

@@ -93,6 +93,27 @@ class SearchSuggestionEntry {
   get tail() {
     return this._tail;
   }
+
+  get tailOffsetIndex() {
+    if (!this._tail) {
+      return -1;
+    }
+
+    let offsetIndex = this._value.lastIndexOf(this._tail);
+    if (offsetIndex + this._tail.length < this._value.length) {
+      // We might have a tail suggestion that starts with a word contained in
+      // the full-text suggestion. e.g. "london sights in l" ... "london".
+      let lastWordIndex = this._value.lastIndexOf(" ");
+      if (this._tail.startsWith(this._value.substring(lastWordIndex))) {
+        offsetIndex = lastWordIndex;
+      } else {
+        // Something's gone wrong. Consumers should not show this result.
+        offsetIndex = -1;
+      }
+    }
+
+    return offsetIndex;
+  }
 }
 
 // Maps each engine name to a unique firstPartyDomain, so that requests to

toolkit/components/search/tests/xpcshell/test_searchSuggest.js

@@ -347,6 +347,18 @@ add_task(async function empty_rich_results() {
   Assert.ok(!result.remote[2].tail);
 });
 
+add_task(async function tail_offset_index() {
+  let controller = new SearchSuggestionController();
+  let result = await controller.fetch("tail tail 1 t", false, getEngine);
+  Assert.equal(result.term, "tail tail 1 t");
+  Assert.equal(result.local.length, 0);
+  Assert.equal(result.remote.length, 3);
+  Assert.equal(result.remote[1].value, "tail tail 1 t tail 1");
+  Assert.equal(result.remote[1].matchPrefix, "… ");
+  Assert.equal(result.remote[1].tail, "tail 1");
+  Assert.equal(result.remote[1].tailOffsetIndex, 14);
+});
+
 add_task(async function fetch_twice_in_a_row() {
   // Two entries since the first will match the first fetch but not the second.
   await updateSearchHistory("bump", "delay local");