Bug 1560228 - Strip only leading question marks from search string. r=adw

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

--HG--
extra : moz-landing-system : lando

browser/components/urlbar/tests/browser/browser_keyword.js

@@ -206,7 +206,7 @@ add_task(async function test_keyword_with_question_mark() {
     UrlbarUtils.RESULT_TYPE.SEARCH,
     "Result should be a search"
   );
-  Assert.equal(result.searchParams.query, "question", "Check search query");
+  Assert.equal(result.searchParams.query, "question?", "Check search query");
 
   result = await promise_first_result("question? something");
   Assert.equal(

toolkit/components/places/UnifiedComplete.jsm

@@ -647,7 +647,6 @@ function Search(
 
   // This allows to handle leading or trailing restriction characters specially.
   this._leadingRestrictionToken = null;
-  this._trailingRestrictionToken = null;
   if (tokens.length > 0) {
     if (
       UrlbarTokenizer.isRestrictionToken(tokens[0]) &&
@@ -656,13 +655,6 @@ function Search(
     ) {
       this._leadingRestrictionToken = tokens[0].value;
     }
-    if (
-      UrlbarTokenizer.isRestrictionToken(tokens[tokens.length - 1]) &&
-      (tokens.length > 1 ||
-        tokens[tokens.length - 1].type == UrlbarTokenizer.TYPE.RESTRICT_SEARCH)
-    ) {
-      this._trailingRestrictionToken = tokens[tokens.length - 1].value;
-    }
 
     // Check if the first token has a strippable prefix and remove it, but don't
     // create an empty token.
@@ -1739,17 +1731,14 @@ Search.prototype = {
     if (!engine || !this.pending) {
       return false;
     }
-    // Strip a leading or trailing restriction char.
+    // Strip a leading search restriction char, because we prepend it to text
+    // when the search shortcut is used and it's not user typed. Don't strip
+    // other restriction chars, so that it's possible to search for things
+    // including one of those (e.g. "c#").
     let query = this._trimmedOriginalSearchString;
-    if (this._leadingRestrictionToken) {
+    if (this._leadingRestrictionToken === UrlbarTokenizer.RESTRICT.SEARCH) {
       query = substringAfter(query, this._leadingRestrictionToken).trim();
     }
-    if (this._trailingRestrictionToken) {
-      query = query.substring(
-        0,
-        query.lastIndexOf(this._trailingRestrictionToken)
-      );
-    }
     this._addSearchEngineMatch({ engine, query });
     return true;
   },

toolkit/components/places/tests/unifiedcomplete/test_restrict_searchstring.js

@@ -0,0 +1,32 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+/**
+ * Test that restriction tokens are not removed from the search string, apart
+ * from a leading search restriction token.
+ */
+
+add_task(async function test_searchstring() {
+  for (let token of Object.values(UrlbarTokenizer.RESTRICT)) {
+    for (let search of [`${token} query`, `query ${token}`]) {
+      let searchQuery =
+        token == UrlbarTokenizer.RESTRICT.SEARCH &&
+        search.startsWith(UrlbarTokenizer.RESTRICT.SEARCH)
+          ? search.substring(2)
+          : search;
+      info(`Searching for "${search}", expecting "${searchQuery}"`);
+      await check_autocomplete({
+        search,
+        searchParam: "enable-actions",
+        matches: [
+          makeSearchMatch(search, {
+            engineName: "MozSearch",
+            searchQuery,
+            heuristic: true,
+          }),
+        ],
+      });
+    }
+  }
+});

toolkit/components/places/tests/unifiedcomplete/test_search_engine_alias.js

@@ -111,15 +111,20 @@ add_task(async function basicGetAndPost() {
     // be recognized.  It should be treated as part of the search string.  Try
     // all the restriction tokens to test that.  We should get a single "search
     // with" heuristic result without an alias.
-    for (let restrictToken in UrlbarTokenizer.RESTRICT) {
-      let search = `${restrictToken} ${alias} query string`;
+    for (let token of Object.values(UrlbarTokenizer.RESTRICT)) {
+      let search = `${token} ${alias} query string`;
+      let searchQuery =
+        token == UrlbarTokenizer.RESTRICT.SEARCH &&
+        search.startsWith(UrlbarTokenizer.RESTRICT.SEARCH)
+          ? search.substring(2)
+          : search;
       await check_autocomplete({
         search,
         searchParam: "enable-actions",
         matches: [
           makeSearchMatch(search, {
             engineName: "MozSearch",
-            searchQuery: search,
+            searchQuery,
             heuristic: true,
           }),
         ],

toolkit/components/places/tests/unifiedcomplete/test_tab_matches.js

@@ -224,7 +224,7 @@ add_task(async function test_tab_matches() {
     matches: [
       makeSearchMatch(UrlbarTokenizer.RESTRICT.OPENPAGE + " abc", {
         heuristic: true,
-        searchQuery: "abc",
+        searchQuery: UrlbarTokenizer.RESTRICT.OPENPAGE + " abc",
       }),
       makeSwitchToTabMatch("http://abc.com/", { title: "ABC rocks" }),
     ],
@@ -247,7 +247,7 @@ add_task(async function test_tab_matches() {
     matches: [
       makeSearchMatch(UrlbarTokenizer.RESTRICT.OPENPAGE + " mozilla", {
         heuristic: true,
-        searchQuery: "mozilla",
+        searchQuery: UrlbarTokenizer.RESTRICT.OPENPAGE + " mozilla",
       }),
       makeSwitchToTabMatch("about:mozilla"),
     ],

toolkit/components/places/tests/unifiedcomplete/xpcshell.ini

@@ -43,6 +43,7 @@ skip-if = appname == "thunderbird"
 [test_query_url.js]
 [test_remote_tab_matches.js]
 skip-if = !sync
+[test_restrict_searchstring.js]
 [test_search_engine_alias.js]
 [test_search_engine_default.js]
 [test_search_engine_host.js]