Bug 1465692 - search for keyword domains instead of literal keyword draft
authorPeter Simonyi <pts@petersimonyi.ca>
Sat, 02 Jun 2018 17:48:17 -0400
changeset 803307 ee504973568ae78d240680c3d495a75209bd158c
parent 803222 8c926373039374cd1a47d92215e9efb4d5557983
push id112066
push userbmo:pts+bmo@petersimonyi.ca
push dateSat, 02 Jun 2018 21:48:54 +0000
bugs1465692
milestone62.0a1
Bug 1465692 - search for keyword domains instead of literal keyword MozReview-Commit-ID: JFSUt7Rwtl6
toolkit/components/places/PlacesSearchAutocompleteProvider.jsm
toolkit/components/places/UnifiedComplete.js
toolkit/components/places/tests/unifiedcomplete/test_keyword_search.js
toolkit/components/places/tests/unifiedcomplete/test_keyword_search_actions.js
toolkit/components/places/tests/unifiedcomplete/test_search_engine_alias.js
--- a/toolkit/components/places/PlacesSearchAutocompleteProvider.jsm
+++ b/toolkit/components/places/PlacesSearchAutocompleteProvider.jsm
@@ -83,25 +83,27 @@ const SearchAutocompleteProviderInternal
     }
 
     // The search engines will always be processed in the order returned by the
     // search service, which can be defined by the user.
     Services.search.getVisibleEngines().forEach(e => this._addEngine(e));
   },
 
   _addEngine(engine) {
+    let domain = engine.getResultDomain();
+
     if (engine.alias) {
       this.aliasMatches.push({
         alias: engine.alias,
         engineName: engine.name,
         iconUrl: engine.iconURI ? engine.iconURI.spec : null,
+        resultDomain: domain,
       });
     }
 
-    let domain = engine.getResultDomain();
     if (domain) {
       this.priorityMatches.push({
         token: domain,
         // The searchForm property returns a simple URL for the search engine, but
         // we may need an URL which includes an affiliate code (bug 990799).
         url: engine.searchForm,
         engineName: engine.name,
         iconUrl: engine.iconURI ? engine.iconURI.spec : null,
@@ -246,16 +248,18 @@ var PlacesSearchAutocompleteProvider = O
    *        Search string to match exactly a search engine alias.
    *
    * @return An object with the following properties, or undefined if the token
    *         does not match any relevant URL:
    *         {
    *           alias: The matched search engine's alias.
    *           engineName: The display name of the search engine.
    *           iconUrl: Icon associated to the match, or null if not available.
+   *           resultDomain: The domain name for the search engine's results;
+   *                         see nsISearchEngine::getResultDomain.
    *         }
    */
   async findMatchByAlias(searchToken) {
     await this.ensureInitialized();
 
     return SearchAutocompleteProviderInternal.aliasMatches
              .find(m => m.alias.toLocaleLowerCase() == searchToken.toLocaleLowerCase());
   },
--- a/toolkit/components/places/UnifiedComplete.js
+++ b/toolkit/components/places/UnifiedComplete.js
@@ -886,16 +886,17 @@ function Search(searchString, searchPara
 
   let userContextId = searchParam.match(REGEXP_USER_CONTEXT_ID);
   this._userContextId = userContextId ?
                           parseInt(userContextId[1], 10) :
                           Ci.nsIScriptSecurityManager.DEFAULT_USER_CONTEXT_ID;
 
   this._searchTokens =
     this.filterTokens(getUnfilteredSearchTokens(this._searchString));
+  this._keywordSubstitute = null;
 
   this._prohibitSearchSuggestions = prohibitSearchSuggestions;
 
   this._listener = autocompleteListener;
   this._autocompleteSearch = autocompleteSearch;
 
   // Create a new result to add eventual matches.  Note we need a result
   // regardless having matches.
@@ -1107,37 +1108,27 @@ Search.prototype = {
     // 3) inline completion for origins (this._originQuery) or urls (this._urlQuery)
     // 4) directly typed in url (ie, can be navigated to as-is)
     // 5) submission for the current search engine
     // 6) Places keywords
     // 7) adaptive learning (this._adaptiveQuery)
     // 8) open pages not supported by history (this._switchToTabQuery)
     // 9) query based on match behavior
     //
-    // (6) only gets ran if we get any filtered tokens, since if there are no
-    // tokens, there is nothing to match. This is the *first* query we check if
-    // we want to run, but it gets queued to be run later.
+    // (6) only gets run if we get any filtered tokens, since if there are no
+    // tokens, there is nothing to match.
     //
     // (1), (4), (5) only get run if actions are enabled. When actions are
     // enabled, the first result is always a special result (resulting from one
     // of the queries between (1) and (6) inclusive). As such, the UI is
     // expected to auto-select the first result when actions are enabled. If the
     // first result is an inline completion result, that will also be the
     // default result and therefore be autofilled (this also happens if actions
     // are not enabled).
 
-    // Get the final query, based on the tokens found in the search string.
-    let queries = [];
-    // "openpage" behavior is supported by the default query.
-    // _switchToTabQuery instead returns only pages not supported by history.
-    if (this.hasBehavior("openpage")) {
-      queries.push(this._switchToTabQuery);
-    }
-    queries.push(this._searchQuery);
-
     // Check for Preloaded Sites Expiry before Autofill
     await this._checkPreloadedSitesExpiry();
 
     // Add the first heuristic result, if any.  Set _addingHeuristicFirstMatch
     // to true so that when the result is added, "heuristic" can be included in
     // its style.
     this._addingHeuristicFirstMatch = true;
     let hasHeuristic = await this._matchFirstHeuristicResult(conn);
@@ -1204,16 +1195,26 @@ Search.prototype = {
 
     // Then fetch remote tabs.
     if (this._enableActions && this.hasBehavior("openpage")) {
       await this._matchRemoteTabs();
       if (!this.pending)
         return;
     }
 
+    // Get the final query, based on the tokens found in the search string and
+    // the keyword substitution, if any.
+    let queries = [];
+    // "openpage" behavior is supported by the default query.
+    // _switchToTabQuery instead returns only pages not supported by history.
+    if (this.hasBehavior("openpage")) {
+      queries.push(this._switchToTabQuery);
+    }
+    queries.push(this._searchQuery);
+
     // Finally run all the other queries.
     for (let [query, params] of queries) {
       await conn.executeCached(query, params, this._onResultRow.bind(this));
       if (!this.pending)
         return;
     }
 
     // If we have some unused adaptive matches, add them now.
@@ -1494,17 +1495,17 @@ Search.prototype = {
       return true;
     }
     return false;
   },
 
   async _matchPlacesKeyword() {
     // The first word could be a keyword, so that's what we'll search.
     let keyword = this._searchTokens[0];
-    let entry = await PlacesUtils.keywords.fetch(this._searchTokens[0]);
+    let entry = await PlacesUtils.keywords.fetch(keyword);
     if (!entry)
       return false;
 
     let searchString = this._trimmedOriginalSearchString.substr(keyword.length + 1);
 
     let url = null, postData = null;
     try {
       [url, postData] =
@@ -1533,16 +1534,19 @@ Search.prototype = {
       value,
       comment,
       // 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 (!this._keywordSubstitute) {
+      this._keywordSubstitute = entry.url.host;
+    }
     return true;
   },
 
   async _matchSearchEngineDomain() {
     if (!Prefs.get("autoFill.searchEngines")) {
       return false;
     }
     if (!this._searchString) {
@@ -1609,16 +1613,19 @@ Search.prototype = {
     let match = await PlacesSearchAutocompleteProvider.findMatchByAlias(alias);
     if (!match)
       return false;
 
     match.engineAlias = alias;
     let query = this._trimmedOriginalSearchString.substr(alias.length + 1);
 
     this._addSearchEngineMatch(match, query);
+    if (!this._keywordSubstitute) {
+      this._keywordSubstitute = match.resultDomain;
+    }
     return true;
   },
 
   async _matchCurrentSearchEngine() {
     let match = await PlacesSearchAutocompleteProvider.getDefaultMatch();
     if (!match)
       return false;
 
@@ -2242,16 +2249,31 @@ Search.prototype = {
       conditions.push("tags NOTNULL");
     }
 
     return conditions.length ? defaultQuery("AND " + conditions.join(" AND "))
                              : defaultQuery();
   },
 
   /**
+   * Get the search string with the keyword substitution applied.
+   * If the user-provided string starts with a keyword that gave a heuristic
+   * result, it can provide a substitute string (e.g. the domain that keyword
+   * will search) so that the history/bookmark results we show will correspond
+   * to the keyword search rather than searching for the literal keyword.
+   */
+  get _keywordSubstitutedSearchString() {
+    let tokens = this._searchTokens;
+    if (this._keywordSubstitute) {
+      tokens = [this._keywordSubstitute, ...this._searchTokens.slice(1)];
+    }
+    return tokens.join(" ");
+  },
+
+  /**
    * Obtains the search query to be used based on the previously set search
    * preferences (accessed by this.hasBehavior).
    *
    * @return an array consisting of the correctly optimized query to search the
    *         database with and an object containing the params to bound.
    */
   get _searchQuery() {
     let query = this._suggestionPrefQuery;
@@ -2260,17 +2282,17 @@ Search.prototype = {
       query,
       {
         parent: PlacesUtils.tagsFolderId,
         query_type: QUERYTYPE_FILTERED,
         matchBehavior: this._matchBehavior,
         searchBehavior: this._behavior,
         // We only want to search the tokens that we are left with - not the
         // original search string.
-        searchString: this._searchTokens.join(" "),
+        searchString: this._keywordSubstitutedSearchString,
         userContextId: this._userContextId,
         // Limit the query to the the maximum number of desired results.
         // This way we can avoid doing more work than needed.
         maxResults: Prefs.get("maxRichResults")
       }
     ];
   },
 
@@ -2284,17 +2306,17 @@ Search.prototype = {
     return [
       SQL_SWITCHTAB_QUERY,
       {
         query_type: QUERYTYPE_FILTERED,
         matchBehavior: this._matchBehavior,
         searchBehavior: this._behavior,
         // We only want to search the tokens that we are left with - not the
         // original search string.
-        searchString: this._searchTokens.join(" "),
+        searchString: this._keywordSubstitutedSearchString,
         userContextId: this._userContextId,
         maxResults: Prefs.get("maxRichResults")
       }
     ];
   },
 
   /**
    * Obtains the query to search for adaptive results.
--- a/toolkit/components/places/tests/unifiedcomplete/test_keyword_search.js
+++ b/toolkit/components/places/tests/unifiedcomplete/test_keyword_search.js
@@ -10,19 +10,21 @@
  *
  * Also test for bug 249468 by making sure multiple keyword bookmarks with the
  * same keyword appear in the list.
  */
 
 add_task(async function test_keyword_searc() {
   let uri1 = NetUtil.newURI("http://abc/?search=%s");
   let uri2 = NetUtil.newURI("http://abc/?search=ThisPageIsInHistory");
+  let uri3 = NetUtil.newURI("http://somedomain.example/key");
   await PlacesTestUtils.addVisits([
     { uri: uri1, title: "Generic page title" },
-    { uri: uri2, title: "Generic page title" }
+    { uri: uri2, title: "Generic page title" },
+    { uri: uri3, title: "This page shouldn't be suggested" },
   ]);
   await addBookmark({ uri: uri1, title: "Bookmark title", keyword: "key"});
 
   info("Plain keyword query");
   await check_autocomplete({
     search: "key term",
     matches: [ { uri: NetUtil.newURI("http://abc/?search=term"), title: "abc", style: ["keyword", "heuristic"] } ]
   });
@@ -46,32 +48,49 @@ add_task(async function test_keyword_sea
   });
 
   info("Unescaped term in query");
   await check_autocomplete({
     search: "key ユニコード",
     matches: [ { uri: NetUtil.newURI("http://abc/?search=ユニコード"), title: "abc", style: ["keyword", "heuristic"] } ]
   });
 
-  info("Keyword that happens to match a page");
+  info("Keyword with query that happens to match a page");
   await check_autocomplete({
     search: "key ThisPageIsInHistory",
     matches: [ { uri: NetUtil.newURI("http://abc/?search=ThisPageIsInHistory"), title: "abc", style: ["keyword", "heuristic"] } ]
   });
 
+  info("Keyword with query that partially matches a page");
+  await check_autocomplete({
+    search: "key ThisPage",
+    matches: [
+      { uri: NetUtil.newURI("http://abc/?search=ThisPage"), title: "abc", style: ["keyword", "heuristic"] },
+      { uri: NetUtil.newURI("http://abc/?search=ThisPageIsInHistory"), title: "Generic page title" },
+    ],
+  });
+
   info("Keyword without query (without space)");
   await check_autocomplete({
     search: "key",
-    matches: [ { uri: NetUtil.newURI("http://abc/?search="), title: "abc", style: ["keyword", "heuristic"] } ]
+    matches: [
+      { uri: NetUtil.newURI("http://abc/?search="), title: "abc", style: ["keyword", "heuristic"] },
+      { uri: NetUtil.newURI("http://abc/?search=%s"), title: "Bookmark title", style: ["bookmark"] },
+      { uri: NetUtil.newURI("http://abc/?search=ThisPageIsInHistory"), title: "Generic page title" },
+    ]
   });
 
   info("Keyword without query (with space)");
   await check_autocomplete({
     search: "key ",
-    matches: [ { uri: NetUtil.newURI("http://abc/?search="), title: "abc", style: ["keyword", "heuristic"] } ]
+    matches: [
+      { uri: NetUtil.newURI("http://abc/?search="), title: "abc", style: ["keyword", "heuristic"] },
+      { uri: NetUtil.newURI("http://abc/?search=%s"), title: "Bookmark title", style: ["bookmark"] },
+      { uri: NetUtil.newURI("http://abc/?search=ThisPageIsInHistory"), title: "Generic page title" },
+    ]
   });
 
   info("Bug 1228111 - Keyword with a space in front");
   await check_autocomplete({
     search: " key test",
     matches: [ { uri: NetUtil.newURI("http://abc/?search=test"), title: "abc", style: ["keyword", "heuristic"] } ]
   });
 
--- a/toolkit/components/places/tests/unifiedcomplete/test_keyword_search_actions.js
+++ b/toolkit/components/places/tests/unifiedcomplete/test_keyword_search_actions.js
@@ -12,25 +12,27 @@
  * same keyword appear in the list.
  */
 
 add_task(async function test_keyword_search() {
   let uri1 = NetUtil.newURI("http://abc/?search=%s");
   let uri2 = NetUtil.newURI("http://abc/?search=ThisPageIsInHistory");
   let uri3 = NetUtil.newURI("http://abc/?search=%s&raw=%S");
   let uri4 = NetUtil.newURI("http://abc/?search=%s&raw=%S&mozcharset=ISO-8859-1");
+  let uri5 = NetUtil.newURI("http://def/?search=%s");
   await PlacesTestUtils.addVisits([{ uri: uri1 },
                                    { uri: uri2 },
                                    { uri: uri3 }]);
   await addBookmark({ uri: uri1, title: "Keyword", keyword: "key"});
   await addBookmark({ uri: uri1, title: "Post", keyword: "post", postData: "post_search=%s"});
   await addBookmark({ uri: uri3, title: "Encoded", keyword: "encoded"});
   await addBookmark({ uri: uri4, title: "Charset", keyword: "charset"});
   await addBookmark({ uri: uri2, title: "Noparam", keyword: "noparam"});
   await addBookmark({ uri: uri2, title: "Noparam-Post", keyword: "post_noparam", postData: "noparam=1"});
+  await addBookmark({ uri: uri5, title: "Keyword", keyword: "key2"});
 
   info("Plain keyword query");
   await check_autocomplete({
     search: "key term",
     searchParam: "enable-actions",
     matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=term", input: "key term"}),
                  title: "abc", style: [ "action", "keyword", "heuristic" ] } ]
   });
@@ -72,30 +74,49 @@ add_task(async function test_keyword_sea
   info("Keyword that happens to match a page");
   await check_autocomplete({
     search: "key ThisPageIsInHistory",
     searchParam: "enable-actions",
     matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=ThisPageIsInHistory", input: "key ThisPageIsInHistory"}),
                  title: "abc", style: [ "action", "keyword", "heuristic" ] } ]
   });
 
+  info("Keyword with partial page match");
+  await check_autocomplete({
+    search: "key ThisPage",
+    searchParam: "enable-actions",
+    matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=ThisPage", input: "key ThisPage"}),
+                 title: "abc", style: [ "action", "keyword", "heuristic" ] },
+               // Only the most recent bookmark for the URL:
+               { value: "http://abc/?search=ThisPageIsInHistory",
+                 title: "Noparam-Post", style: ["bookmark"] },
+    ]
+  });
+
+  // For the keyword with no query terms (with or without space after), the
+  // domain is different from the other tests because otherwise all the other
+  // test bookmarks and history entries would be matches.
   info("Keyword without query (without space)");
   await check_autocomplete({
-    search: "key",
+    search: "key2",
     searchParam: "enable-actions",
-    matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=", input: "key"}),
-                 title: "abc", style: [ "action", "keyword", "heuristic" ] } ]
+    matches: [ { uri: makeActionURI("keyword", {url: "http://def/?search=", input: "key2"}),
+                 title: "def", style: [ "action", "keyword", "heuristic" ] },
+               { uri: uri5, title: "Keyword", style: [ "bookmark" ] },
+    ]
   });
 
   info("Keyword without query (with space)");
   await check_autocomplete({
-    search: "key ",
+    search: "key2 ",
     searchParam: "enable-actions",
-    matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=", input: "key "}),
-                 title: "abc", style: [ "action", "keyword", "heuristic" ] } ]
+    matches: [ { uri: makeActionURI("keyword", {url: "http://def/?search=", input: "key2 "}),
+                 title: "def", style: [ "action", "keyword", "heuristic" ] },
+               { uri: uri5, title: "Keyword", style: [ "bookmark" ] },
+    ]
   });
 
   info("POST Keyword");
   await check_autocomplete({
     search: "post foo",
     searchParam: "enable-actions",
     matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=foo", input: "post foo", postData: "post_search=foo"}),
                  title: "abc", style: [ "action", "keyword", "heuristic" ] } ]
--- a/toolkit/components/places/tests/unifiedcomplete/test_search_engine_alias.js
+++ b/toolkit/components/places/tests/unifiedcomplete/test_search_engine_alias.js
@@ -4,30 +4,45 @@
 
 add_task(async function() {
   // Note that head_autocomplete.js has already added a MozSearch engine.
   // Here we add another engine with a search alias.
   Services.search.addEngineWithDetails("AliasedGETMozSearch", "", "get", "",
                                        "GET", "http://s.example.com/search");
   Services.search.addEngineWithDetails("AliasedPOSTMozSearch", "", "post", "",
                                        "POST", "http://s.example.com/search");
+  let histURI = NetUtil.newURI("http://s.example.com/search?q=firefox");
+  await PlacesTestUtils.addVisits([{ uri: histURI, title: "History entry" }]);
 
   for (let alias of ["get", "post"]) {
     await check_autocomplete({
       search: alias,
       searchParam: "enable-actions",
       matches: [ makeSearchMatch(alias, { engineName: `Aliased${alias.toUpperCase()}MozSearch`,
-                                          searchQuery: "", alias, heuristic: true }) ]
+                                          searchQuery: "", alias, heuristic: true }),
+        { uri: histURI, title: "History entry" },
+      ]
     });
 
     await check_autocomplete({
       search: `${alias} `,
       searchParam: "enable-actions",
       matches: [ makeSearchMatch(`${alias} `, { engineName: `Aliased${alias.toUpperCase()}MozSearch`,
-                                                searchQuery: "", alias, heuristic: true }) ]
+                                                searchQuery: "", alias, heuristic: true }),
+        { uri: histURI, title: "History entry" },
+      ]
+    });
+
+    await check_autocomplete({
+      search: `${alias} fire`,
+      searchParam: "enable-actions",
+      matches: [ makeSearchMatch(`${alias} fire`, { engineName: `Aliased${alias.toUpperCase()}MozSearch`,
+                                                    searchQuery: "fire", alias, heuristic: true }),
+        { uri: histURI, title: "History entry" },
+      ]
     });
 
     await check_autocomplete({
       search: `${alias} mozilla`,
       searchParam: "enable-actions",
           matches: [ makeSearchMatch(`${alias} mozilla`, { engineName: `Aliased${alias.toUpperCase()}MozSearch`,
                                                            searchQuery: "mozilla", alias, heuristic: true }) ]
     });