Backed out changeset e5655c6b1c5f (bug 1658646) for failures on browser_oneOffs_searchSuggestions.js. CLOSED TREE

browser/components/urlbar/UrlbarInput.jsm

@@ -1096,9 +1096,7 @@ class UrlbarInput {
 
     if (this.searchMode) {
       options.searchMode = this.searchMode;
-      if (this.searchMode.source) {
-        options.sources = [this.searchMode.source];
-      }
+      options.sources = [this.searchMode.source];
     }
 
     // TODO (Bug 1522902): This promise is necessary for tests, because some
@@ -1195,15 +1193,10 @@ class UrlbarInput {
     this._searchModeLabel.removeAttribute("data-l10n-id");
 
     if (engineName) {
-      this.searchMode = { engineName };
-      if (source) {
-        this.searchMode.source = source;
-      } else if (UrlbarUtils.WEB_ENGINE_NAMES.has(engineName)) {
-        // History results for general-purpose search engines are often not
-        // useful, so we hide them in search mode. See bug 1658646 for
-        // discussion.
-        this.searchMode.source = UrlbarUtils.RESULT_SOURCE.SEARCH;
-      }
+      this.searchMode = {
+        source: UrlbarUtils.RESULT_SOURCE.SEARCH,
+        engineName,
+      };
       this._searchModeIndicatorTitle.textContent = engineName;
       this._searchModeLabel.textContent = engineName;
       this.document.l10n.setAttributes(
@@ -1260,12 +1253,7 @@ class UrlbarInput {
    */
   searchModeShortcut() {
     if (this.view.oneOffsRefresh) {
-      // We restrict to search results when entering search mode from this
-      // shortcut to honor historical behaviour.
-      this.setSearchMode({
-        source: UrlbarUtils.RESULT_SOURCE.SEARCH,
-        engineName: Services.search.defaultEngine.name,
-      });
+      this.setSearchMode({ engineName: Services.search.defaultEngine.name });
       this.search("");
     } else {
       this.search(UrlbarTokenizer.RESTRICT.SEARCH);

browser/components/urlbar/UrlbarProviderSearchSuggestions.jsm

@@ -102,9 +102,9 @@ class ProviderSearchSuggestions extends UrlbarProvider {
   }
 
   /**
-   * Returns whether the user typed a token alias or restriction token, or is in
-   * search mode. We use this value to override the pref to disable search
-   * suggestions in the Urlbar.
+   * Returns whether the user typed a token alias or a restriction token. We use
+   * this value to override the pref to disable search suggestions in the
+   * Urlbar.
    * @param {UrlbarQueryContext} queryContext  The query context object.
    * @returns {boolean} True if the user typed a token alias or search
    *   restriction token.
@@ -116,9 +116,7 @@ class ProviderSearchSuggestions extends UrlbarProvider {
         queryContext.restrictSource == UrlbarUtils.RESULT_SOURCE.SEARCH) ||
       queryContext.tokens.some(
         t => t.type == UrlbarTokenizer.TYPE.RESTRICT_SEARCH
-      ) ||
-      (queryContext.searchMode &&
-        queryContext.sources.includes(UrlbarUtils.RESULT_SOURCE.SEARCH))
+      )
     );
   }
 

browser/components/urlbar/UrlbarProviderTopSites.jsm

@@ -64,11 +64,7 @@ class ProviderTopSites extends UrlbarProvider {
    * @returns {boolean} Whether this provider should be invoked for the search.
    */
   isActive(queryContext) {
-    return (
-      !queryContext.restrictSource &&
-      !queryContext.searchString &&
-      !queryContext.searchMode
-    );
+    return !queryContext.restrictSource && !queryContext.searchString;
   }
 
   /**

browser/components/urlbar/tests/UrlbarTestUtils.jsm

@@ -491,13 +491,10 @@ var UrlbarTestUtils = {
     );
 
     let buttons = oneOffs.getSelectableButtons(true);
-    if (!searchMode) {
-      searchMode = { engineName: buttons[0].engine.name };
-      if (UrlbarUtils.WEB_ENGINE_NAMES.has(searchMode.engineName)) {
-        searchMode.source = UrlbarUtils.RESULT_SOURCE.SEARCH;
-      }
-    }
-
+    searchMode = searchMode || {
+      source: UrlbarUtils.RESULT_SOURCE.SEARCH,
+      engineName: buttons[0].engine.name,
+    };
     let oneOff = buttons.find(o =>
       searchMode.engineName
         ? o.engine.name == searchMode.engineName

browser/components/urlbar/tests/browser/browser_oneOffs.js

@@ -484,6 +484,7 @@ add_task(async function oneOffClick() {
         "Urlbar view is still open."
       );
       UrlbarTestUtils.assertSearchMode(window, {
+        source: UrlbarUtils.RESULT_SOURCE.SEARCH,
         engineName: oneOffs[0].engine.name,
       });
       window.gURLBar.setSearchMode({});
@@ -534,6 +535,7 @@ add_task(async function oneOffReturn() {
         "Urlbar view is still open."
       );
       UrlbarTestUtils.assertSearchMode(window, {
+        source: UrlbarUtils.RESULT_SOURCE.SEARCH,
         engineName: oneOffs[0].engine.name,
       });
       window.gURLBar.setSearchMode({});

browser/components/urlbar/tests/browser/browser_placeholder.js

@@ -230,7 +230,10 @@ add_task(async function test_search_mode_engine_web() {
 
 add_task(async function test_search_mode_engine_other() {
   await doSearchModeTest(
-    { engineName: extraEngine.name },
+    {
+      source: UrlbarUtils.RESULT_SOURCE.SEARCH,
+      engineName: extraEngine.name,
+    },
     { id: "urlbar-placeholder-search-mode-other", args: null }
   );
 });

browser/components/urlbar/tests/browser/browser_searchModeIndicator.js

@@ -197,6 +197,7 @@ add_task(async function escape() {
     window
   ).getSelectableButtons(true);
   UrlbarTestUtils.assertSearchMode(window, {
+    source: UrlbarUtils.RESULT_SOURCE.SEARCH,
     engineName: oneOffs[0].engine.name,
   });
 
@@ -372,6 +373,7 @@ add_task(async function tab_switch() {
     window
   ).getSelectableButtons(true);
   UrlbarTestUtils.assertSearchMode(window, {
+    source: UrlbarUtils.RESULT_SOURCE.SEARCH,
     engineName: oneOffs[0].engine.name,
   });
 
@@ -409,6 +411,7 @@ add_task(async function tab_switch() {
   await BrowserTestUtils.switchTab(gBrowser, tabs[0]);
   await searchPromise;
   UrlbarTestUtils.assertSearchMode(window, {
+    source: UrlbarUtils.RESULT_SOURCE.SEARCH,
     engineName: oneOffs[0].engine.name,
   });
 
@@ -432,6 +435,7 @@ add_task(async function tab_switch() {
   await BrowserTestUtils.switchTab(gBrowser, tabs[0]);
   await searchPromise;
   UrlbarTestUtils.assertSearchMode(window, {
+    source: UrlbarUtils.RESULT_SOURCE.SEARCH,
     engineName: oneOffs[0].engine.name,
   });
 
@@ -447,6 +451,7 @@ add_task(async function tab_switch() {
   await BrowserTestUtils.switchTab(gBrowser, tabs[0]);
   await searchPromise;
   UrlbarTestUtils.assertSearchMode(window, {
+    source: UrlbarUtils.RESULT_SOURCE.SEARCH,
     engineName: oneOffs[0].engine.name,
   });
 

browser/components/urlbar/tests/browser/browser_searchMode_alias_replacement.js

@@ -82,6 +82,7 @@ add_task(async function replaced_on_space() {
   await searchPromise;
 
   UrlbarTestUtils.assertSearchMode(window, {
+    source: UrlbarUtils.RESULT_SOURCE.SEARCH,
     engineName: aliasEngine.name,
   });
   Assert.ok(!gURLBar.value, "The Urlbar value should be cleared.");

browser/components/urlbar/tests/browser/browser_searchMode_no_results.js

@@ -261,6 +261,7 @@ add_task(async function spaceToEnterSearchMode() {
       "Panel has no results, therefore should have noresults attribute"
     );
     UrlbarTestUtils.assertSearchMode(win, {
+      source: UrlbarUtils.RESULT_SOURCE.SEARCH,
       engineName: engine.name,
     });
     this.Assert.equal(

browser/components/urlbar/tests/browser/browser_searchMode_suggestions.js

@@ -142,52 +142,6 @@ add_task(async function nonEmptySearch_nonMatching() {
   });
 });
 
-add_task(async function nonEmptySearch_withHistory() {
-  // URLs with the same host as the search engine.
-  await PlacesTestUtils.addVisits([
-    "http://mochi.test/ciao",
-    "http://mochi.test/ciao1",
-  ]);
-
-  await BrowserTestUtils.withNewTab("about:robots", async function(browser) {
-    await UrlbarTestUtils.promiseAutocompleteResultPopup({
-      window,
-      value: "ciao",
-    });
-
-    await UrlbarTestUtils.enterSearchMode(window);
-    Assert.equal(gURLBar.value, "ciao", "Urlbar value should be set.");
-    await checkResults([
-      {
-        isSearchHistory: false,
-        suggestion: undefined,
-      },
-      {
-        isSearchHistory: false,
-        suggestion: "ciaofoo",
-      },
-      {
-        isSearchHistory: false,
-        suggestion: "ciaobar",
-      },
-      {
-        isSearchHistory: false,
-        suggestion: undefined,
-        historyUrl: "http://mochi.test/ciao1",
-      },
-      {
-        isSearchHistory: false,
-        suggestion: undefined,
-        historyUrl: "http://mochi.test/ciao",
-      },
-    ]);
-
-    await UrlbarTestUtils.exitSearchMode(window, { clickClose: true });
-  });
-
-  await PlacesUtils.history.clear();
-});
-
 async function checkResults(resultsDetails) {
   Assert.equal(
     UrlbarTestUtils.getResultCount(window),
@@ -196,29 +150,20 @@ async function checkResults(resultsDetails) {
   );
   for (let i = 0; i < resultsDetails.length; ++i) {
     let result = await UrlbarTestUtils.getDetailsOfResultAt(window, i);
-    if (result.searchParams) {
-      Assert.equal(
-        result.searchParams?.engine,
-        suggestionsEngine.name,
-        "It should be a search result for our suggestion engine."
-      );
-      Assert.equal(
-        result.searchParams?.isSearchHistory,
-        resultsDetails[i].isSearchHistory,
-        "Check if it should be a local suggestion result."
-      );
-      Assert.equal(
-        result.searchParams?.suggestion,
-        resultsDetails[i].suggestion,
-        "Check the suggestion value"
-      );
-    }
-    if (resultsDetails[i].historyUrl) {
-      Assert.equal(
-        result.url,
-        resultsDetails[i].historyUrl,
-        "The history result should have the correct URL."
-      );
-    }
+    Assert.equal(
+      result.searchParams.engine,
+      suggestionsEngine.name,
+      "It should be a search result for our suggestion engine."
+    );
+    Assert.equal(
+      result.searchParams.isSearchHistory,
+      resultsDetails[i].isSearchHistory,
+      "Check if it should be a local suggestion result."
+    );
+    Assert.equal(
+      result.searchParams.suggestion,
+      resultsDetails[i].suggestion,
+      "Check the suggestion value"
+    );
   }
 }

toolkit/components/places/UnifiedComplete.jsm

@@ -492,7 +492,6 @@ function Search(
     this._maxResults = queryContext.maxResults;
     this._userContextId = queryContext.userContextId;
     this._currentPage = queryContext.currentPage;
-    this._engineName = queryContext.searchMode?.engineName;
   } else {
     let params = new Set(searchParam.split(" "));
     this._enableActions = params.has("enable-actions");
@@ -550,6 +549,10 @@ function Search(
     let behavior = sourceToBehaviorMap.get(queryContext.restrictSource);
     this.setBehavior(behavior);
 
+    if (behavior == "search" && queryContext.engineName) {
+      this._engineName = queryContext.engineName;
+    }
+
     // When we are in restrict mode, all the tokens are valid for searching, so
     // there is no _heuristicToken.
     this._heuristicToken = null;
@@ -787,16 +790,6 @@ Search.prototype = {
       return;
     }
 
-    // this._engineName is set if the user is in search mode. We fetch only
-    // local results with the same host as the search mode engine.
-    if (this._engineName && !this._keywordSubstitute) {
-      let engine = Services.search.getEngineByName(this._engineName);
-      this._keywordSubstitute = {
-        host: engine.getResultDomain(),
-        keyword: null,
-      };
-    }
-
     // Add the first heuristic result, if any.  Set _addingHeuristicResult
     // to true so that when the result is added, "heuristic" can be included in
     // its style.
@@ -1092,10 +1085,7 @@ Search.prototype = {
 
     this._addMatch(match);
     if (!this._keywordSubstitute) {
-      this._keywordSubstitute = {
-        host: entry.url.host,
-        keyword,
-      };
+      this._keywordSubstitute = entry.url.host;
     }
     return true;
   },
@@ -1114,10 +1104,7 @@ Search.prototype = {
     };
     this._addSearchEngineMatch(this._searchEngineAliasMatch);
     if (!this._keywordSubstitute) {
-      this._keywordSubstitute = {
-        host: engine.getResultDomain(),
-        keyword: alias,
-      };
+      this._keywordSubstitute = engine.getResultDomain();
     }
     return true;
   },
@@ -1708,10 +1695,7 @@ Search.prototype = {
   get _keywordSubstitutedSearchString() {
     let tokens = this._searchTokens.map(t => t.value);
     if (this._keywordSubstitute) {
-      tokens = [
-        this._keywordSubstitute.host,
-        ...tokens.slice(this._keywordSubstitute.keyword ? 1 : 0),
-      ];
+      tokens = [this._keywordSubstitute, ...tokens.slice(1)];
     }
     return tokens.join(" ");
   },