Bug 1571913 - Quantumbar: Allow keyword offers to specify whether to show the keyword, and fix secondary text color when there's no title. r=dao a=RyanVM
authorDrew Willcoxon <adw@mozilla.com>
Wed, 07 Aug 2019 08:07:25 +0000
changeset 545063 70aa6961b410e14b94220824d48d6ed00ad21c6c
parent 545062 48cb377f63cf6de0221656201a1f867ad5ba5440
child 545064 513c1ea45a6a4e630e079f5214d1f2c350c063f6
push id2131
push userffxbld-merge
push dateMon, 26 Aug 2019 18:30:20 +0000
treeherdermozilla-release@b19ffb3ca153 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdao, RyanVM
bugs1571913
milestone69.0
Bug 1571913 - Quantumbar: Allow keyword offers to specify whether to show the keyword, and fix secondary text color when there's no title. r=dao a=RyanVM Replace the "is heuristic?" logic for hiding keywords in keyword offers with something more flexible. Here's how the top sites extension uses this new `keywordOffer` payload property: https://github.com/0c0w3/top-sites-experiment/commit/8408d13d2f80478ad04f6b7a2927280cc97032f1 When a result's title is hidden, use the title color for the secondary color. Differential Revision: https://phabricator.services.mozilla.com/D40898
browser/components/urlbar/UrlbarInput.jsm
browser/components/urlbar/UrlbarProviderUnifiedComplete.jsm
browser/components/urlbar/UrlbarResult.jsm
browser/components/urlbar/UrlbarUtils.jsm
browser/components/urlbar/UrlbarView.jsm
browser/docs/AddressBar.rst
browser/themes/shared/urlbar-autocomplete.inc.css
--- a/browser/components/urlbar/UrlbarInput.jsm
+++ b/browser/components/urlbar/UrlbarInput.jsm
@@ -498,17 +498,17 @@ class UrlbarInput {
   pickResult(result, event) {
     let isCanonized = this.setValueFromResult(result, event);
     let where = this._whereToOpen(event);
     let openParams = {
       allowInheritPrincipal: false,
     };
 
     let selIndex = this.view.selectedIndex;
-    if (!result.payload.isKeywordOffer) {
+    if (!result.payload.keywordOffer) {
       this.view.close();
     }
 
     this.controller.recordSelectedResult(event, result);
 
     if (isCanonized) {
       this.controller.engagementEvent.record(event, {
         numChars: this._lastSearchString.length,
@@ -555,17 +555,17 @@ class UrlbarInput {
           loadOpts
         );
         if (switched && prevTab.isEmpty) {
           this.window.gBrowser.removeTab(prevTab);
         }
         return;
       }
       case UrlbarUtils.RESULT_TYPE.SEARCH: {
-        if (result.payload.isKeywordOffer) {
+        if (result.payload.keywordOffer) {
           // The user confirmed a token alias, so just move the caret
           // to the end of it. Because there's a trailing space in the value,
           // the user can directly start typing a query string at that point.
           this.selectionStart = this.selectionEnd = this.value.length;
 
           this.controller.engagementEvent.record(event, {
             numChars: this._lastSearchString.length,
             selIndex,
--- a/browser/components/urlbar/UrlbarProviderUnifiedComplete.jsm
+++ b/browser/components/urlbar/UrlbarProviderUnifiedComplete.jsm
@@ -209,29 +209,31 @@ function convertResultToMatches(context,
     // we may have added already. This means we'll end up adding things in the
     // wrong order here, but that's a task for the UrlbarMuxer.
     let url = result.getFinalCompleteValueAt(i);
     if (urls.has(url)) {
       continue;
     }
     urls.add(url);
     let style = result.getStyleAt(i);
+    let isHeuristic = i == 0 && style.includes("heuristic");
     let match = makeUrlbarResult(context.tokens, {
       url,
       icon: result.getImageAt(i),
       style,
       comment: result.getCommentAt(i),
       firstToken: context.tokens[0],
+      isHeuristic,
     });
     // Should not happen, but better safe than sorry.
     if (!match) {
       continue;
     }
     // Manage autofill and preselected properties for the first match.
-    if (i == 0 && style.includes("heuristic")) {
+    if (isHeuristic) {
       if (style.includes("autofill") && result.defaultIndex == 0) {
         let autofillValue = result.getValueAt(i);
         if (
           autofillValue
             .toLocaleLowerCase()
             .startsWith(context.searchString.toLocaleLowerCase())
         ) {
           match.autofill = {
@@ -257,34 +259,40 @@ function convertResultToMatches(context,
  * @param {array} tokens the search tokens.
  * @param {object} info includes properties from the legacy match.
  * @returns {object} an UrlbarResult
  */
 function makeUrlbarResult(tokens, info) {
   let action = PlacesUtils.parseActionUrl(info.url);
   if (action) {
     switch (action.type) {
-      case "searchengine":
+      case "searchengine": {
+        let keywordOffer = UrlbarUtils.KEYWORD_OFFER.NONE;
+        if (
+          action.params.alias &&
+          !action.params.searchQuery.trim() &&
+          action.params.alias.startsWith("@")
+        ) {
+          keywordOffer = info.isHeuristic
+            ? UrlbarUtils.KEYWORD_OFFER.HIDE
+            : UrlbarUtils.KEYWORD_OFFER.SHOW;
+        }
         return new UrlbarResult(
           UrlbarUtils.RESULT_TYPE.SEARCH,
           UrlbarUtils.RESULT_SOURCE.SEARCH,
           ...UrlbarResult.payloadAndSimpleHighlights(tokens, {
             engine: [action.params.engineName, true],
             suggestion: [action.params.searchSuggestion, true],
             keyword: [action.params.alias, true],
             query: [action.params.searchQuery.trim(), true],
             icon: [info.icon, false],
-            isKeywordOffer: [
-              action.params.alias &&
-                !action.params.searchQuery.trim() &&
-                action.params.alias.startsWith("@"),
-              false,
-            ],
+            keywordOffer,
           })
         );
+      }
       case "keyword": {
         let title = info.comment;
         if (!title) {
           // If the url doesn't have an host (e.g. javascript urls), comment
           // will be empty, and we can't build the usual title. Thus use the url.
           title = Services.textToSubURI.unEscapeURIForUI(
             "UTF-8",
             action.params.url
--- a/browser/components/urlbar/UrlbarResult.jsm
+++ b/browser/components/urlbar/UrlbarResult.jsm
@@ -111,20 +111,21 @@ class UrlbarResult {
       case UrlbarUtils.RESULT_TYPE.TAB_SWITCH:
       case UrlbarUtils.RESULT_TYPE.URL:
       case UrlbarUtils.RESULT_TYPE.OMNIBOX:
       case UrlbarUtils.RESULT_TYPE.REMOTE_TAB:
         return this.payload.title
           ? [this.payload.title, this.payloadHighlights.title]
           : [this.payload.url || "", this.payloadHighlights.url || []];
       case UrlbarUtils.RESULT_TYPE.SEARCH:
-        if (this.payload.isKeywordOffer) {
-          return this.heuristic
-            ? ["", []]
-            : [this.payload.keyword, this.payloadHighlights.keyword];
+        switch (this.payload.keywordOffer) {
+          case UrlbarUtils.KEYWORD_OFFER.SHOW:
+            return [this.payload.keyword, this.payloadHighlights.keyword];
+          case UrlbarUtils.KEYWORD_OFFER.HIDE:
+            return ["", []];
         }
         return this.payload.suggestion
           ? [this.payload.suggestion, this.payloadHighlights.suggestion]
           : [this.payload.query, this.payloadHighlights.query];
       default:
         return ["", []];
     }
   }
--- a/browser/components/urlbar/UrlbarUtils.jsm
+++ b/browser/components/urlbar/UrlbarUtils.jsm
@@ -123,16 +123,27 @@ var UrlbarUtils = {
     COMMIT: 3,
     CANCELED: 4,
   },
 
   // Limit the length of titles and URLs we display so layout doesn't spend too
   // much time building text runs.
   MAX_TEXT_LENGTH: 255,
 
+  // Search results with keywords and empty queries are called "keyword offers".
+  // When the user selects a keyword offer, the keyword followed by a space is
+  // put in the input as a hint that the user can search using the keyword.
+  // Depending on the use case, keyword-offer results can show or not show the
+  // keyword itself.
+  KEYWORD_OFFER: {
+    NONE: 0,
+    SHOW: 1,
+    HIDE: 2,
+  },
+
   // UnifiedComplete's autocomplete results store their titles and tags together
   // in their comments.  This separator is used to separate them.  When we
   // rewrite UnifiedComplete for quantumbar, we should stop using this old hack
   // and store titles and tags separately.  It's important that this be a
   // character that no title would ever have.  We use \x1F, the non-printable
   // unit separator.
   TITLE_TAGS_SEPARATOR: "\x1F",
 
--- a/browser/components/urlbar/UrlbarView.jsm
+++ b/browser/components/urlbar/UrlbarView.jsm
@@ -570,17 +570,17 @@ class UrlbarView {
   }
 
   _updateRow(item, result) {
     item.result = result;
     item.removeAttribute("stale");
 
     if (
       result.type == UrlbarUtils.RESULT_TYPE.SEARCH &&
-      !result.payload.isKeywordOffer
+      !result.payload.keywordOffer
     ) {
       item.setAttribute("type", "search");
     } else if (result.type == UrlbarUtils.RESULT_TYPE.REMOTE_TAB) {
       item.setAttribute("type", "remotetab");
     } else if (result.type == UrlbarUtils.RESULT_TYPE.TAB_SWITCH) {
       item.setAttribute("type", "switchtab");
     } else if (result.source == UrlbarUtils.RESULT_SOURCE.BOOKMARKS) {
       item.setAttribute("type", "bookmark");
--- a/browser/docs/AddressBar.rst
+++ b/browser/docs/AddressBar.rst
@@ -404,17 +404,17 @@ properties, supported by all of the resu
 
 The following RESULT_TYPEs are supported:
 
 .. highlight:: JavaScript
 .. code::
 
     // Payload: { icon, url, userContextId }
     TAB_SWITCH: 1,
-    // Payload: { icon, suggestion, keyword, query, isKeywordOffer }
+    // Payload: { icon, suggestion, keyword, query, keywordOffer }
     SEARCH: 2,
     // Payload: { icon, url, title, tags }
     URL: 3,
     // Payload: { icon, url, keyword, postData }
     KEYWORD: 4,
     // Payload: { icon, keyword, title, content }
     OMNIBOX: 5,
     // Payload: { icon, url, device, title }
--- a/browser/themes/shared/urlbar-autocomplete.inc.css
+++ b/browser/themes/shared/urlbar-autocomplete.inc.css
@@ -142,22 +142,25 @@
 }
 
 .urlbarView-title > strong,
 .urlbarView-url > strong,
 .urlbarView-tag > strong {
   font-weight: 600;
 }
 
-.urlbarView-secondary {
+.urlbarView-title:not(:empty) ~ .urlbarView-secondary {
   color: var(--urlbar-popup-action-color);
 }
 
 .urlbarView-url {
   overflow: hidden;
+}
+
+.urlbarView-title:not(:empty) ~ .urlbarView-url {
   color: var(--urlbar-popup-url-color);
 }
 
 .urlbarView-row[selected] > .urlbarView-row-inner > .urlbarView-title-separator::before,
 .urlbarView-row[selected] > .urlbarView-row-inner > .urlbarView-secondary {
   color: inherit;
 }