Bug 1527389 - The aliases of hidden engines shouldn't show up in the urlbar r=adw
authorOmkar Konaraddi <okonaraddi@mozilla.com>
Mon, 01 Jul 2019 23:03:41 +0000
changeset 540498 351504c92d1354cb2e02c171de2c29cfd985a97a
parent 540497 024f3ba5e1232eccba5cbe8b5eabc02e3b5cce19
child 540499 6385b8616d54c62539ec4b4bb9325e9d4abfe721
push id11529
push userarchaeopteryx@coole-files.de
push dateThu, 04 Jul 2019 15:22:33 +0000
treeherdermozilla-beta@ebb510a784b8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs1527389
milestone69.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 1527389 - The aliases of hidden engines shouldn't show up in the urlbar r=adw Differential Revision: https://phabricator.services.mozilla.com/D36532
browser/components/urlbar/tests/browser/browser_urlbarTokenAlias.js
toolkit/components/places/PlacesSearchAutocompleteProvider.jsm
toolkit/components/places/tests/unifiedcomplete/test_PlacesSearchAutocompleteProvider.js
toolkit/components/places/tests/unifiedcomplete/test_search_engine_alias.js
--- a/browser/components/urlbar/tests/browser/browser_urlbarTokenAlias.js
+++ b/browser/components/urlbar/tests/browser/browser_urlbarTokenAlias.js
@@ -432,8 +432,97 @@ function assertHighlighted(highlighted, 
     index >= 0,
     `gURLBar.textValue="${gURLBar.textValue}" expectedAlias="${expectedAlias}"`
   );
   let range = selection.getRangeAt(0);
   Assert.ok(range);
   Assert.equal(range.startOffset, index);
   Assert.equal(range.endOffset, index + expectedAlias.length);
 }
+
+
+/**
+ * This test checks that if an engine is marked as hidden then
+ * it should not appear in the popup when using the "@" token alias in the search bar.
+ */
+add_task(async function hiddenEngine() {
+  await promiseAutocompleteResultPopup("@", window, true);
+
+  const defaultEngine = await Services.search.getDefault();
+
+  let foundDefaultEngineInPopup = false;
+
+  // Checks that the default engine appears in the urlbar's popup.
+  for (let i = 0; i < UrlbarTestUtils.getResultCount(window); i++) {
+    let details = await UrlbarTestUtils.getDetailsOfResultAt(window, i);
+    if (defaultEngine.name == details.searchParams.engine) {
+      foundDefaultEngineInPopup = true;
+      break;
+    }
+  }
+  Assert.ok(foundDefaultEngineInPopup, "Default engine appears in the popup.");
+
+  await UrlbarTestUtils.promisePopupClose(window,
+    () => EventUtils.synthesizeKey("KEY_Escape"));
+
+  // Checks that a hidden default engine (i.e. an engine removed from
+  // a user's search settings) does not appear in the urlbar's popup.
+  defaultEngine.hidden = true;
+  await promiseAutocompleteResultPopup("@", window, true);
+  foundDefaultEngineInPopup = false;
+  for (let i = 0; i < UrlbarTestUtils.getResultCount(window); i++) {
+    let details = await UrlbarTestUtils.getDetailsOfResultAt(window, i);
+    if (defaultEngine.name == details.searchParams.engine) {
+      foundDefaultEngineInPopup = true;
+      break;
+    }
+  }
+  Assert.ok(!foundDefaultEngineInPopup, "Hidden default engine does not appear in the popup");
+
+  await UrlbarTestUtils.promisePopupClose(window,
+    () => EventUtils.synthesizeKey("KEY_Escape"));
+
+  defaultEngine.hidden = false;
+});
+
+/**
+ * This test checks that if an engine is marked as hidden then
+ * it should not appear in the popup when using the "@" token alias in the search bar.
+ */
+add_task(async function hiddenEngine() {
+  await promiseAutocompleteResultPopup("@", window, true);
+
+  const defaultEngine = await Services.search.getDefault();
+
+  let foundDefaultEngineInPopup = false;
+
+  // Checks that the default engine appears in the urlbar's popup.
+  for (let i = 0; i < UrlbarTestUtils.getResultCount(window); i++) {
+    let details = await UrlbarTestUtils.getDetailsOfResultAt(window, i);
+    if (defaultEngine.name == details.searchParams.engine) {
+      foundDefaultEngineInPopup = true;
+      break;
+    }
+  }
+  Assert.ok(foundDefaultEngineInPopup, "Default engine appears in the popup.");
+
+  await UrlbarTestUtils.promisePopupClose(window,
+    () => EventUtils.synthesizeKey("KEY_Escape"));
+
+  // Checks that a hidden default engine (i.e. an engine removed from
+  // a user's search settings) does not appear in the urlbar's popup.
+  defaultEngine.hidden = true;
+  await promiseAutocompleteResultPopup("@", window, true);
+  foundDefaultEngineInPopup = false;
+  for (let i = 0; i < UrlbarTestUtils.getResultCount(window); i++) {
+    let details = await UrlbarTestUtils.getDetailsOfResultAt(window, i);
+    if (defaultEngine.name == details.searchParams.engine) {
+      foundDefaultEngineInPopup = true;
+      break;
+    }
+  }
+  Assert.ok(!foundDefaultEngineInPopup, "Hidden default engine does not appear in the popup");
+
+  await UrlbarTestUtils.promisePopupClose(window,
+    () => EventUtils.synthesizeKey("KEY_Escape"));
+
+  defaultEngine.hidden = false;
+});
--- a/toolkit/components/places/PlacesSearchAutocompleteProvider.jsm
+++ b/toolkit/components/places/PlacesSearchAutocompleteProvider.jsm
@@ -71,18 +71,22 @@ const SearchAutocompleteProviderInternal
     this.tokenAliasEngines = [];
 
     // The search engines will always be processed in the order returned by the
     // search service, which can be defined by the user.
     (await Services.search.getEngines()).forEach(e => this._addEngine(e));
   },
 
   _addEngine(engine) {
+    if (engine.hidden) {
+      return;
+    }
+
     let domain = engine.getResultDomain();
-    if (domain && !engine.hidden) {
+    if (domain) {
       this.enginesByDomain.set(domain, engine);
     }
 
     let aliases = [];
     if (engine.alias) {
       aliases.push(engine.alias);
     }
     aliases.push(...engine.wrappedJSObject._internalAliases);
--- a/toolkit/components/places/tests/unifiedcomplete/test_PlacesSearchAutocompleteProvider.js
+++ b/toolkit/components/places/tests/unifiedcomplete/test_PlacesSearchAutocompleteProvider.js
@@ -36,16 +36,21 @@ add_task(async function hide_search_engi
   let domain = engine.getResultDomain();
   let token = domain.substr(0, 1);
   let promiseTopic = promiseSearchTopic("engine-changed");
   await Promise.all([Services.search.removeEngine(engine), promiseTopic]);
   Assert.ok(engine.hidden);
   let matchedEngine =
     await PlacesSearchAutocompleteProvider.engineForDomainPrefix(token);
   Assert.ok(!matchedEngine || matchedEngine.getResultDomain() != domain);
+  engine.hidden = false;
+  await TestUtils.waitForCondition(() => PlacesSearchAutocompleteProvider.engineForDomainPrefix(token));
+  let matchedEngine2 =
+    await PlacesSearchAutocompleteProvider.engineForDomainPrefix(token);
+  Assert.ok(matchedEngine2);
 });
 
 add_task(async function add_search_engine_match() {
   let promiseTopic = promiseSearchTopic("engine-added");
   Assert.equal(
     null,
     await PlacesSearchAutocompleteProvider.engineForDomainPrefix("bacon")
   );
@@ -143,20 +148,28 @@ add_task(async function test_parseSubmis
   Assert.equal(result.engineName, engine.name);
   Assert.equal(result.terms, "terms");
 
   result = PlacesSearchAutocompleteProvider.parseSubmissionURL("http://example.org/");
   Assert.equal(result, null);
 });
 
 add_task(async function test_builtin_aliased_search_engine_match() {
+  let engine = await PlacesSearchAutocompleteProvider.engineForAlias("@google");
+  Assert.ok(engine);
+  Assert.equal(engine.name, "Google");
+  let promiseTopic = promiseSearchTopic("engine-changed");
+  await Promise.all([Services.search.removeEngine(engine), promiseTopic]);
   let matchedEngine =
     await PlacesSearchAutocompleteProvider.engineForAlias("@google");
-  Assert.ok(matchedEngine);
-  Assert.equal(matchedEngine.name, "Google");
+  Assert.ok(!matchedEngine);
+  engine.hidden = false;
+  await TestUtils.waitForCondition(() => PlacesSearchAutocompleteProvider.engineForAlias("@google"));
+  engine = await PlacesSearchAutocompleteProvider.engineForAlias("@google");
+  Assert.ok(engine);
 });
 
 function promiseSearchTopic(expectedVerb) {
   return new Promise(resolve => {
     Services.obs.addObserver( function observe(subject, topic, verb) {
       info("browser-search-engine-modified: " + verb);
       if (verb == expectedVerb) {
         Services.obs.removeObserver(observe, "browser-search-engine-modified");
--- a/toolkit/components/places/tests/unifiedcomplete/test_search_engine_alias.js
+++ b/toolkit/components/places/tests/unifiedcomplete/test_search_engine_alias.js
@@ -121,17 +121,16 @@ add_task(async function basicGetAndPost(
             engineName: "MozSearch",
             searchQuery: search,
             heuristic: true,
           }),
         ],
       });
     }
   }
-
   await cleanup();
 });
 
 
 // Uses an engine that provides search suggestions.
 add_task(async function engineWithSuggestions() {
   let engine = await addTestSuggestionsEngine();
 
@@ -240,16 +239,25 @@ add_task(async function engineWithSugges
   engine.alias = "";
   await cleanup();
 });
 
 
 // When the search is simply "@", the results should be a list of all the "@"
 // alias engines.
 add_task(async function tokenAliasEngines() {
+  await Services.search.init();
+  // Tell the search service we are running in the US.  This also has the
+  // desired side-effect of preventing our geoip lookup.
+  Services.prefs.setCharPref("browser.search.region", "US");
+  Services.prefs.setBoolPref("browser.search.geoSpecificDefaults", false);
+
+  Services.search.restoreDefaultEngines();
+  Services.search.resetToOriginalDefaultEngine();
+
   let tokenEngines = [];
   for (let engine of await Services.search.getEngines()) {
     let aliases = [];
     if (engine.alias) {
       aliases.push(engine.alias);
     }
     aliases.push(...engine.wrappedJSObject._internalAliases);
     let tokenAliases = aliases.filter(a => a.startsWith("@"));