Bug 1662172 - Part 2 - Remove keyword code from UnifiedComplete. r=adw

Depends on D116898

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

browser/components/urlbar/UrlbarProviderBookmarkKeywords.jsm

@@ -15,7 +15,6 @@ const { XPCOMUtils } = ChromeUtils.import(
 );
 XPCOMUtils.defineLazyModuleGetters(this, {
   KeywordUtils: "resource://gre/modules/KeywordUtils.jsm",
-  PlacesUtils: "resource://gre/modules/PlacesUtils.jsm",
   Services: "resource://gre/modules/Services.jsm",
   UrlbarProvider: "resource:///modules/UrlbarUtils.jsm",
   UrlbarResult: "resource:///modules/UrlbarResult.jsm",
@@ -51,8 +50,6 @@ class ProviderBookmarkKeywords extends UrlbarProvider {
    * @returns {boolean} Whether this provider should be invoked for the search.
    */
   isActive(queryContext) {
-    return false;
-    // TODO: Part 2: Enable this provider.
     return (
       (!queryContext.restrictSource ||
         queryContext.restrictSource == UrlbarTokenizer.RESTRICT.BOOKMARK) &&
@@ -69,25 +66,16 @@ class ProviderBookmarkKeywords extends UrlbarProvider {
    */
   async startQuery(queryContext, addCallback) {
     let keyword = queryContext.tokens[0]?.value;
-    let entry = await PlacesUtils.keywords.fetch(keyword);
-    if (!entry) {
-      return;
-    }
 
     let searchString = UrlbarUtils.substringAfter(
       queryContext.searchString,
       keyword
     ).trim();
-
-    let url = null;
-    let postData = null;
-    try {
-      [url, postData] = await KeywordUtils.parseUrlAndPostData(
-        entry.url.href,
-        entry.postData,
-        searchString
-      );
-    } catch (ex) {
+    let { entry, url, postData } = await KeywordUtils.getBindableKeyword(
+      keyword,
+      searchString
+    );
+    if (!entry || !url) {
       return;
     }
 

browser/components/urlbar/UrlbarProviderUnifiedComplete.jsm

@@ -17,7 +17,6 @@ const { XPCOMUtils } = ChromeUtils.import(
 );
 XPCOMUtils.defineLazyModuleGetters(this, {
   PlacesUtils: "resource://gre/modules/PlacesUtils.jsm",
-  Services: "resource://gre/modules/Services.jsm",
   UrlbarPrefs: "resource:///modules/UrlbarPrefs.jsm",
   UrlbarProvider: "resource:///modules/UrlbarUtils.jsm",
   UrlbarResult: "resource:///modules/UrlbarResult.jsm",
@@ -225,37 +224,6 @@ function makeUrlbarResult(tokens, info) {
           })
         );
       }
-      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(action.params.url);
-        } else if (tokens && tokens.length > 1) {
-          title = UrlbarUtils.strings.formatStringFromName(
-            "bookmarkKeywordSearch",
-            [
-              info.comment,
-              tokens
-                .slice(1)
-                .map(t => t.value)
-                .join(" "),
-            ]
-          );
-        }
-        return new UrlbarResult(
-          UrlbarUtils.RESULT_TYPE.KEYWORD,
-          UrlbarUtils.RESULT_SOURCE.BOOKMARKS,
-          ...UrlbarResult.payloadAndSimpleHighlights(tokens, {
-            title: [title, UrlbarUtils.HIGHLIGHT.TYPED],
-            url: [action.params.url, UrlbarUtils.HIGHLIGHT.TYPED],
-            keyword: [info.firstToken.value, UrlbarUtils.HIGHLIGHT.TYPED],
-            input: [action.params.input],
-            postData: [action.params.postData],
-            icon: info.icon,
-          })
-        );
-      }
       case "switchtab":
         return new UrlbarResult(
           UrlbarUtils.RESULT_TYPE.TAB_SWITCH,

toolkit/components/places/UnifiedComplete.jsm

@@ -525,10 +525,7 @@ function Search(
     // The heuristic token is the first filtered search token, but only when it's
     // actually the first thing in the search string.  If a prefix or restriction
     // character occurs first, then the heurstic token is null.  We use the
-    // heuristic token to help determine the heuristic result.  It may be a Places
-    // keyword, a search engine alias, or simply a URL or part of the search
-    // string the user has typed.  We won't know until we create the heuristic
-    // result.
+    // heuristic token to help determine the heuristic result.
     let firstToken = !!this._searchTokens.length && this._searchTokens[0].value;
     this._heuristicToken =
       firstToken && this._trimmedOriginalSearchString.startsWith(firstToken)
@@ -715,13 +712,9 @@ Search.prototype = {
     };
 
     // For any given search, we run many queries/heuristics:
-    // 1) Places keywords
-    // 2) open pages not supported by history (this._switchToTabQuery)
-    // 3) query based on match behavior
-    // 4) Preloaded sites (currently disabled)
-    //
-    // (1) only gets run if we get any filtered tokens, since if there are no
-    // tokens, there is nothing to match.
+    // 1) open pages not supported by history (this._switchToTabQuery)
+    // 2) query based on match behavior
+    // 3) Preloaded sites (currently disabled)
 
     // Check for Preloaded Sites Expiry before Autofill
     await this._checkPreloadedSitesExpiry();
@@ -897,12 +890,25 @@ Search.prototype = {
       return false;
     }
 
-    // TODO Bug 1662172: Also check if the first token is a bookmark keyword.
     let aliasEngine = await UrlbarSearchUtils.engineForAlias(
       this._heuristicToken,
       this._originalSearchString
     );
-    return !!aliasEngine;
+
+    if (aliasEngine) {
+      return true;
+    }
+
+    let { entry } = await KeywordUtils.getBindableKeyword(
+      this._heuristicToken,
+      this._originalSearchString
+    );
+    if (entry) {
+      this._filterOnHost = entry.url.host;
+      return true;
+    }
+
+    return false;
   },
 
   async _matchFirstHeuristicResult(conn) {
@@ -913,16 +919,8 @@ Search.prototype = {
 
     // We always try to make the first result a special "heuristic" result.  The
     // heuristics below determine what type of result it will be, if any.
-    if (this.pending && this._heuristicToken) {
-      // It may be a Places keyword.
-      let matched = await this._matchPlacesKeyword(this._heuristicToken);
-      if (matched) {
-        return true;
-      }
-    }
 
     let shouldAutofill = this._shouldAutofill;
-
     if (this.pending && shouldAutofill) {
       let matched = this._matchPreloadedSiteForAutofill();
       if (matched) {
@@ -934,61 +932,6 @@ Search.prototype = {
     return false;
   },
 
-  async _matchPlacesKeyword(keyword) {
-    let entry = await PlacesUtils.keywords.fetch(keyword);
-    if (!entry) {
-      return false;
-    }
-
-    let searchString = UrlbarUtils.substringAfter(
-      this._originalSearchString,
-      keyword
-    ).trim();
-
-    let url = null;
-    let postData = null;
-    try {
-      [url, postData] = await KeywordUtils.parseUrlAndPostData(
-        entry.url.href,
-        entry.postData,
-        searchString
-      );
-    } catch (ex) {
-      // It's not possible to bind a param to this keyword.
-      return false;
-    }
-
-    let style = "keyword";
-    let value = url;
-    if (this._enableActions) {
-      style = "action " + style;
-      value = makeActionUrl("keyword", {
-        url,
-        keyword,
-        input: this._originalSearchString,
-        postData,
-      });
-    }
-
-    let match = {
-      value,
-      // Don't use the url with replaced strings, since the icon doesn't change
-      // but the string does, it may cause pointless icon flicker on typing.
-      icon: iconHelper(entry.url),
-      style,
-      frecency: Infinity,
-    };
-    // If there is a query string, the title will be "host: queryString".
-    if (this._searchTokens.length > 1) {
-      match.comment = entry.url.host;
-    }
-
-    this._firstTokenIsKeyword = true;
-    this._filterOnHost = entry.url.host;
-    this._addMatch(match);
-    return true;
-  },
-
   /**
    * Adds a search engine match.
    *

toolkit/modules/KeywordUtils.jsm

@@ -91,4 +91,30 @@ var KeywordUtils = {
     }
     return [url, postData];
   },
+
+  /**
+   * Returns a set of parameters if a keyword is registered and the search
+   * string can be bound to it.
+   *
+   * @param {string} keyword The typed keyword.
+   * @param {string} searchString The full search string, including the keyword.
+   * @returns { entry, url, postData }
+   */
+  async getBindableKeyword(keyword, searchString) {
+    let entry = await PlacesUtils.keywords.fetch(keyword);
+    if (!entry) {
+      return {};
+    }
+
+    try {
+      let [url, postData] = await this.parseUrlAndPostData(
+        entry.url.href,
+        entry.postData,
+        searchString
+      );
+      return { entry, url, postData };
+    } catch (ex) {
+      return {};
+    }
+  },
 };