Bug 1588469 - Show Search in a Private Window even if search suggestions in address bar are disabled. r=Standard8
☠☠ backed out by 4a6da6cc293d ☠ ☠
authorMarco Bonardo <mbonardo@mozilla.com>
Wed, 16 Oct 2019 22:07:14 +0000
changeset 559275 b85a791759739f49d378ff310f9132e71ea49660
parent 559274 7c046b2cdab468155e42adf35456d8dd3f7f92de
child 559276 c5bff62e1640bcd05351d8f24cf34ef121d1197d
push id12175
push userccoroiu@mozilla.com
push dateThu, 17 Oct 2019 19:29:09 +0000
treeherdermozilla-beta@d333b6ef1fd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersStandard8
bugs1588469
milestone71.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1588469 - Show Search in a Private Window even if search suggestions in address bar are disabled. r=Standard8 Split the SEARCH source into SEARCH_NETWORK or SEARCH_LOCAL to distinguish search results coming from a remote source, from those generated locally. browser.urlbar.suggest.searches will only affect SEARCH_NETWORK results. Differential Revision: https://phabricator.services.mozilla.com/D49172
browser/components/extensions/test/xpcshell/test_ext_urlbar.js
browser/components/urlbar/UrlbarProviderExtension.jsm
browser/components/urlbar/UrlbarProviderPrivateSearch.jsm
browser/components/urlbar/UrlbarProviderUnifiedComplete.jsm
browser/components/urlbar/UrlbarProvidersManager.jsm
browser/components/urlbar/UrlbarUtils.jsm
browser/components/urlbar/tests/browser/browser_urlbar_separatePrivateDefault.js
browser/components/urlbar/tests/unit/test_providersManager_filtering.js
--- a/browser/components/extensions/test/xpcshell/test_ext_urlbar.js
+++ b/browser/components/extensions/test/xpcshell/test_ext_urlbar.js
@@ -262,33 +262,33 @@ add_task(async function test_onProviderR
   Assert.ok(!provider.isRestricting(context));
 
   // Check the results.
   let expectedResults = [
     // The first result should be a search result returned by the
     // UnifiedComplete provider.
     {
       type: UrlbarUtils.RESULT_TYPE.SEARCH,
-      source: UrlbarUtils.RESULT_SOURCE.SEARCH,
+      source: UrlbarUtils.RESULT_SOURCE.SEARCH_LOCAL,
       title: "test",
       heuristic: true,
       payload: {
         query: "test",
         engine: "Test engine",
         suggestion: undefined,
         keyword: undefined,
         icon: "",
         keywordOffer: false,
       },
     },
     // The second result should be our search suggestion result since the
     // default muxer sorts search suggestion results before other types.
     {
       type: UrlbarUtils.RESULT_TYPE.SEARCH,
-      source: UrlbarUtils.RESULT_SOURCE.SEARCH,
+      source: UrlbarUtils.RESULT_SOURCE.SEARCH_NETWORK,
       title: "Test search-search result",
       heuristic: false,
       payload: {
         engine: "Test engine",
         suggestion: "Test search-search result",
       },
     },
     // The rest of the results should appear in the order we returned them
@@ -481,45 +481,45 @@ add_task(async function test_onProviderR
   await controller.startQuery(context);
 
   // Check the results.  The first several are valid and should include "Test
   // engine" as the engine.  The others don't specify an engine and are
   // therefore invalid, so they should be ignored.
   let expectedResults = [
     {
       type: UrlbarUtils.RESULT_TYPE.SEARCH,
-      source: UrlbarUtils.RESULT_SOURCE.SEARCH,
+      source: UrlbarUtils.RESULT_SOURCE.SEARCH_NETWORK,
       engine: "Test engine",
       title: "engine specified",
       heuristic: false,
     },
     {
       type: UrlbarUtils.RESULT_TYPE.SEARCH,
-      source: UrlbarUtils.RESULT_SOURCE.SEARCH,
+      source: UrlbarUtils.RESULT_SOURCE.SEARCH_NETWORK,
       engine: "Test engine",
       title: "keyword specified",
       heuristic: false,
     },
     {
       type: UrlbarUtils.RESULT_TYPE.SEARCH,
-      source: UrlbarUtils.RESULT_SOURCE.SEARCH,
+      source: UrlbarUtils.RESULT_SOURCE.SEARCH_NETWORK,
       engine: "Test engine",
       title: "url specified",
       heuristic: false,
     },
     {
       type: UrlbarUtils.RESULT_TYPE.SEARCH,
-      source: UrlbarUtils.RESULT_SOURCE.SEARCH,
+      source: UrlbarUtils.RESULT_SOURCE.SEARCH_NETWORK,
       engine: "Test engine",
       title: "engine, keyword, and url specified",
       heuristic: false,
     },
     {
       type: UrlbarUtils.RESULT_TYPE.SEARCH,
-      source: UrlbarUtils.RESULT_SOURCE.SEARCH,
+      source: UrlbarUtils.RESULT_SOURCE.SEARCH_NETWORK,
       engine: "Test engine",
       title: "keyword and url specified",
       heuristic: false,
     },
   ];
 
   let actualResults = context.results.map(r => ({
     type: r.type,
--- a/browser/components/urlbar/UrlbarProviderExtension.jsm
+++ b/browser/components/urlbar/UrlbarProviderExtension.jsm
@@ -339,13 +339,13 @@ UrlbarProviderExtension.RESULT_TYPES = {
 };
 
 // Maps extension source type enums to internal source types.
 UrlbarProviderExtension.SOURCE_TYPES = {
   bookmarks: UrlbarUtils.RESULT_SOURCE.BOOKMARKS,
   history: UrlbarUtils.RESULT_SOURCE.HISTORY,
   local: UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL,
   network: UrlbarUtils.RESULT_SOURCE.OTHER_NETWORK,
-  search: UrlbarUtils.RESULT_SOURCE.SEARCH,
+  search: UrlbarUtils.RESULT_SOURCE.SEARCH_NETWORK,
   tabs: UrlbarUtils.RESULT_SOURCE.TABS,
 };
 
 UrlbarProviderExtension.notificationTimeout = DEFAULT_NOTIFICATION_TIMEOUT;
--- a/browser/components/urlbar/UrlbarProviderPrivateSearch.jsm
+++ b/browser/components/urlbar/UrlbarProviderPrivateSearch.jsm
@@ -105,17 +105,17 @@ class ProviderPrivateSearch extends Urlb
     let isPrivateEngine =
       separatePrivateDefault && engine != (await Services.search.getDefault());
     logger.info(`isPrivateEngine: ${isPrivateEngine}`);
 
     addCallback(
       this,
       new UrlbarResult(
         UrlbarUtils.RESULT_TYPE.SEARCH,
-        UrlbarUtils.RESULT_SOURCE.SEARCH,
+        UrlbarUtils.RESULT_SOURCE.SEARCH_LOCAL,
         ...UrlbarResult.payloadAndSimpleHighlights(queryContext.tokens, {
           engine: [engine.name, UrlbarUtils.HIGHLIGHT.TYPED],
           query: [
             queryContext.searchString.trim(),
             UrlbarUtils.HIGHLIGHT.TYPED,
           ],
           icon: [engine.iconURI ? engine.iconURI.spec : null],
           inPrivateWindow: true,
--- a/browser/components/urlbar/UrlbarProviderUnifiedComplete.jsm
+++ b/browser/components/urlbar/UrlbarProviderUnifiedComplete.jsm
@@ -264,19 +264,22 @@ function makeUrlbarResult(tokens, info) 
           action.params.alias &&
           !action.params.searchQuery.trim() &&
           action.params.alias.startsWith("@")
         ) {
           keywordOffer = info.isHeuristic
             ? UrlbarUtils.KEYWORD_OFFER.HIDE
             : UrlbarUtils.KEYWORD_OFFER.SHOW;
         }
+        let source = action.params.searchSuggestion
+          ? UrlbarUtils.RESULT_SOURCE.SEARCH_NETWORK
+          : UrlbarUtils.RESULT_SOURCE.SEARCH_LOCAL;
         return new UrlbarResult(
           UrlbarUtils.RESULT_TYPE.SEARCH,
-          UrlbarUtils.RESULT_SOURCE.SEARCH,
+          source,
           ...UrlbarResult.payloadAndSimpleHighlights(tokens, {
             engine: [action.params.engineName, UrlbarUtils.HIGHLIGHT.TYPED],
             suggestion: [
               action.params.searchSuggestion,
               UrlbarUtils.HIGHLIGHT.SUGGESTED,
             ],
             keyword: [action.params.alias, UrlbarUtils.HIGHLIGHT.TYPED],
             query: [
@@ -369,17 +372,17 @@ function makeUrlbarResult(tokens, info) 
         Cu.reportError(`Unexpected action type: ${action.type}`);
         return null;
     }
   }
 
   if (info.style.includes("priority-search")) {
     return new UrlbarResult(
       UrlbarUtils.RESULT_TYPE.SEARCH,
-      UrlbarUtils.RESULT_SOURCE.SEARCH,
+      UrlbarUtils.RESULT_SOURCE.SEARCH_LOCAL,
       ...UrlbarResult.payloadAndSimpleHighlights(tokens, {
         engine: [info.comment, UrlbarUtils.HIGHLIGHT.TYPED],
         icon: [info.icon],
       })
     );
   }
 
   // This is a normal url/title tuple.
--- a/browser/components/urlbar/UrlbarProvidersManager.jsm
+++ b/browser/components/urlbar/UrlbarProvidersManager.jsm
@@ -447,24 +447,32 @@ function getAcceptableMatchSources(conte
       case UrlbarUtils.RESULT_SOURCE.HISTORY:
         if (
           restrictTokenType === UrlbarTokenizer.TYPE.RESTRICT_HISTORY ||
           (!restrictTokenType && UrlbarPrefs.get("suggest.history"))
         ) {
           acceptedSources.push(source);
         }
         break;
-      case UrlbarUtils.RESULT_SOURCE.SEARCH:
+      case UrlbarUtils.RESULT_SOURCE.SEARCH_NETWORK:
         if (
           restrictTokenType === UrlbarTokenizer.TYPE.RESTRICT_SEARCH ||
           (!restrictTokenType && UrlbarPrefs.get("suggest.searches"))
         ) {
           acceptedSources.push(source);
         }
         break;
+      case UrlbarUtils.RESULT_SOURCE.SEARCH_LOCAL:
+        if (
+          restrictTokenType === UrlbarTokenizer.TYPE.RESTRICT_SEARCH ||
+          !restrictTokenType
+        ) {
+          acceptedSources.push(source);
+        }
+        break;
       case UrlbarUtils.RESULT_SOURCE.TABS:
         if (
           restrictTokenType === UrlbarTokenizer.TYPE.RESTRICT_OPENPAGE ||
           (!restrictTokenType && UrlbarPrefs.get("suggest.openpage"))
         ) {
           acceptedSources.push(source);
         }
         break;
--- a/browser/components/urlbar/UrlbarUtils.jsm
+++ b/browser/components/urlbar/UrlbarUtils.jsm
@@ -99,20 +99,21 @@ var UrlbarUtils = {
   // can return results from more than one source. This is used by the
   // ProvidersManager to decide which providers must be queried and which
   // results can be returned.
   // If you add new source types, consider checking if consumers of
   // "urlbar-user-start-navigation" need update as well.
   RESULT_SOURCE: {
     BOOKMARKS: 1,
     HISTORY: 2,
-    SEARCH: 3,
-    TABS: 4,
-    OTHER_LOCAL: 5,
-    OTHER_NETWORK: 6,
+    SEARCH_LOCAL: 3,
+    SEARCH_NETWORK: 4,
+    TABS: 5,
+    OTHER_LOCAL: 6,
+    OTHER_NETWORK: 7,
   },
 
   // This defines icon locations that are commonly used in the UI.
   ICON: {
     // DEFAULT is defined lazily so it doesn't eagerly initialize PlacesUtils.
     SEARCH_GLASS: "chrome://browser/skin/search-glass.svg",
     TIP: "chrome://browser/skin/tip.svg",
   },
--- a/browser/components/urlbar/tests/browser/browser_urlbar_separatePrivateDefault.js
+++ b/browser/components/urlbar/tests/browser/browser_urlbar_separatePrivateDefault.js
@@ -106,16 +106,32 @@ add_task(async function test_search() {
   await UrlbarTestUtils.promiseAutocompleteResultPopup({
     window,
     waitForFocus,
     value: "unique198273982173",
   });
   await AssertPrivateResult(window, await Services.search.getDefault(), false);
 });
 
+add_task(async function test_search_disabled_suggestions() {
+  info(
+    "Test that 'Search in a Private Window' appears if suggestions are disabled"
+  );
+  await SpecialPowers.pushPrefEnv({
+    set: [["browser.urlbar.suggest.searches", false]],
+  });
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: "unique198273982173",
+  });
+  await AssertPrivateResult(window, await Services.search.getDefault(), false);
+  await SpecialPowers.popPrefEnv();
+});
+
 add_task(async function test_oneoff_selected_keyboard() {
   info(
     "Test that 'Search in a Private Window' with keyboard opens the selected one-off engine if there's no private engine"
   );
   await UrlbarTestUtils.promiseAutocompleteResultPopup({
     window,
     waitForFocus,
     value: "unique198273982173",
--- a/browser/components/urlbar/tests/unit/test_providersManager_filtering.js
+++ b/browser/components/urlbar/tests/unit/test_providersManager_filtering.js
@@ -311,34 +311,35 @@ add_task(async function test_nofilter_re
     ),
     new UrlbarResult(
       UrlbarUtils.RESULT_TYPE.URL,
       UrlbarUtils.RESULT_SOURCE.HISTORY,
       { url: "http://mozilla.org/foo_history/" }
     ),
     new UrlbarResult(
       UrlbarUtils.RESULT_TYPE.SEARCH,
-      UrlbarUtils.RESULT_SOURCE.SEARCH,
+      UrlbarUtils.RESULT_SOURCE.SEARCH_NETWORK,
       { engine: "noengine" }
     ),
   ];
   /**
    * A test provider.
    */
   class TestProvider extends UrlbarProvider {
     get name() {
       return "MyProvider";
     }
     get type() {
       return UrlbarUtils.PROVIDER_TYPE.IMMEDIATE;
     }
     isActive(context) {
-      Assert.equal(
-        context.acceptableSources.length,
-        1,
+      // For most restrictions there is only one acceptable source, but for
+      // some like SEARCH it may be 2 (LOCAL and NETWORK).
+      Assert.ok(
+        context.acceptableSources.length <= 2,
         "Check acceptableSources"
       );
       return true;
     }
     isRestricting(context) {
       return false;
     }
     async startQuery(context, add) {
@@ -352,17 +353,17 @@ add_task(async function test_nofilter_re
   }
   let provider = new TestProvider();
   UrlbarProvidersManager.registerProvider(provider);
 
   let typeToPropertiesMap = new Map([
     ["HISTORY", { source: "HISTORY", pref: "history" }],
     ["BOOKMARK", { source: "BOOKMARKS", pref: "bookmark" }],
     ["OPENPAGE", { source: "TABS", pref: "openpage" }],
-    ["SEARCH", { source: "SEARCH", pref: "searches" }],
+    ["SEARCH", { source: "SEARCH_NETWORK", pref: "searches" }],
   ]);
   for (let [type, token] of Object.entries(UrlbarTokenizer.RESTRICT)) {
     let properties = typeToPropertiesMap.get(type);
     if (!properties) {
       continue;
     }
     info("Restricting on " + type);
     let context = createContext(token + " foo", {