Bug 1675045 - Add search mode hostname check to the muxer. r=mak

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

browser/components/urlbar/UrlbarMuxerUnifiedComplete.jsm

@@ -404,6 +404,27 @@ class MuxerUnifiedComplete extends UrlbarMuxer {
       }
     }
 
+    // When in an engine search mode, discard URL results whose hostnames don't
+    // include the root domain of the search mode engine.
+    if (state.context.searchMode?.engineName && result.payload.url) {
+      let engine = Services.search.getEngineByName(
+        state.context.searchMode.engineName
+      );
+      if (engine) {
+        let searchModeRootDomain = UrlbarSearchUtils.getRootDomainFromEngine(
+          engine
+        );
+        let resultUrl = new URL(result.payload.url);
+        // Add a trailing "." to increase the stringency of the check. This
+        // check covers most general cases. Some edge cases are not covered,
+        // like `resultUrl` being ebay.mydomain.com, which would escape this
+        // check if `searchModeRootDomain` was "ebay".
+        if (!resultUrl.hostname.includes(`${searchModeRootDomain}.`)) {
+          return false;
+        }
+      }
+    }
+
     // Include the result.
     return true;
   }

browser/components/urlbar/UrlbarSearchUtils.jsm

@@ -146,6 +146,32 @@ class SearchUtils {
     return tokenAliasEngines;
   }
 
+  /**
+   * @param {nsISearchEngine} engine
+   * @returns {string}
+   *   The root domain of a search engine. e.g. If `engine` has the domain
+   *   www.subdomain.rootdomain.com, `rootdomain` is returned. Returns the
+   *   engine's domain if the engine's URL does not have a valid TLD.
+   */
+  getRootDomainFromEngine(engine) {
+    let domain = engine.getResultDomain();
+    let suffix = engine.searchUrlPublicSuffix;
+    if (!suffix) {
+      if (domain.endsWith(".test")) {
+        suffix = "test";
+      } else {
+        return domain;
+      }
+    }
+    domain = domain.substr(
+      0,
+      // -1 to remove the trailing dot.
+      domain.length - suffix.length - 1
+    );
+    let domainParts = domain.split(".");
+    return domainParts.pop();
+  }
+
   getDefaultEngine(isPrivate = false) {
     return this.separatePrivateDefaultUIEnabled &&
       this.separatePrivateDefault &&

browser/components/urlbar/tests/UrlbarTestUtils.jsm

@@ -23,6 +23,7 @@ XPCOMUtils.defineLazyModuleGetters(this, {
   UrlbarController: "resource:///modules/UrlbarController.jsm",
   UrlbarPrefs: "resource:///modules/UrlbarPrefs.jsm",
   UrlbarProvider: "resource:///modules/UrlbarUtils.jsm",
+  UrlbarSearchUtils: "resource:///modules/UrlbarSearchUtils.jsm",
   UrlbarUtils: "resource:///modules/UrlbarUtils.jsm",
 });
 
@@ -509,11 +510,12 @@ var UrlbarTestUtils = {
           let engine = Services.search.getEngineByName(
             expectedSearchMode.engineName
           );
-          let engineHost = engine.getResultDomain();
+          let engineRootDomain = UrlbarSearchUtils.getRootDomainFromEngine(
+            engine
+          );
           let resultUrl = new URL(result.url);
-          // Use `includes` to allow results from engine subdomains.
           this.Assert.ok(
-            resultUrl.host.includes(engineHost),
+            resultUrl.hostname.includes(engineRootDomain),
             "Search mode result matches engine host."
           );
         }

browser/components/urlbar/tests/browser/browser.ini

@@ -155,6 +155,7 @@ support-files =
 support-files =
   dummy_page.html
 [browser_searchMode_engineRemoval.js]
+[browser_searchMode_excludeResults.js]
 [browser_searchMode_heuristic.js]
 [browser_searchMode_indicator.js]
 support-files =

browser/components/urlbar/tests/browser/browser_searchMode_excludeResults.js

@@ -0,0 +1,212 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Tests that results with hostnames other than the search mode engine are not
+ * shown.
+ */
+
+"use strict";
+
+XPCOMUtils.defineLazyModuleGetters(this, {
+  SyncedTabs: "resource://services-sync/SyncedTabs.jsm",
+});
+
+add_task(async function setup() {
+  await SpecialPowers.pushPrefEnv({
+    set: [
+      ["browser.urlbar.update2", true],
+      ["browser.urlbar.update2.oneOffsRefresh", true],
+      ["browser.urlbar.suggest.searches", false],
+      ["browser.urlbar.autoFill", false],
+      // Special prefs for remote tabs.
+      ["services.sync.username", "fake"],
+      ["services.sync.syncedTabs.showRemoteTabs", true],
+    ],
+  });
+
+  await PlacesUtils.history.clear();
+  await PlacesUtils.bookmarks.eraseEverything();
+
+  let oldDefaultEngine = await Services.search.getDefault();
+  // Note that the result domain is subdomain.example.ca. We still expect to
+  // match with example.com results because we ignore subdomains and the public
+  // suffix in this check.
+  let engine = await Services.search.addEngineWithDetails("Test", {
+    template: `http://subdomain.example.ca/?search={searchTerms}`,
+  });
+  await Services.search.setDefault(engine);
+  await Services.search.moveEngine(engine, 0);
+
+  const REMOTE_TAB = {
+    id: "7cqCr77ptzX3",
+    type: "client",
+    lastModified: 1492201200,
+    name: "Nightly on MacBook-Pro",
+    clientType: "desktop",
+    tabs: [
+      {
+        type: "tab",
+        title: "Test Remote",
+        url: "http://example.com",
+        icon: UrlbarUtils.ICON.DEFAULT,
+        client: "7cqCr77ptzX3",
+        lastUsed: 1452124677,
+      },
+      {
+        type: "tab",
+        title: "Test Remote 2",
+        url: "http://example-2.com",
+        icon: UrlbarUtils.ICON.DEFAULT,
+        client: "7cqCr77ptzX3",
+        lastUsed: 1452124677,
+      },
+    ],
+  };
+
+  const sandbox = sinon.createSandbox();
+
+  let originalSyncedTabsInternal = SyncedTabs._internal;
+  SyncedTabs._internal = {
+    isConfiguredToSyncTabs: true,
+    hasSyncedThisSession: true,
+    getTabClients() {
+      return Promise.resolve([]);
+    },
+    syncTabs() {
+      return Promise.resolve();
+    },
+  };
+
+  // Tell the Sync XPCOM service it is initialized.
+  let weaveXPCService = Cc["@mozilla.org/weave/service;1"].getService(
+    Ci.nsISupports
+  ).wrappedJSObject;
+  let oldWeaveServiceReady = weaveXPCService.ready;
+  weaveXPCService.ready = true;
+
+  sandbox
+    .stub(SyncedTabs._internal, "getTabClients")
+    .callsFake(() => Promise.resolve(Cu.cloneInto([REMOTE_TAB], {})));
+
+  // Reset internal cache in PlacesRemoteTabsAutocompleteProvider.
+  Services.obs.notifyObservers(null, "weave:engine:sync:finish", "tabs");
+
+  registerCleanupFunction(async function() {
+    sandbox.restore();
+    weaveXPCService.ready = oldWeaveServiceReady;
+    SyncedTabs._internal = originalSyncedTabsInternal;
+    await Services.search.setDefault(oldDefaultEngine);
+    await Services.search.removeEngine(engine);
+    await PlacesUtils.history.clear();
+  });
+});
+
+add_task(async function basic() {
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    value: "example",
+  });
+  Assert.equal(
+    UrlbarTestUtils.getResultCount(window),
+    3,
+    "We have three results"
+  );
+  let firstResult = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+  Assert.equal(
+    firstResult.type,
+    UrlbarUtils.RESULT_TYPE.SEARCH,
+    "The first result is the heuristic search result."
+  );
+  let secondResult = await UrlbarTestUtils.getDetailsOfResultAt(window, 1);
+  Assert.equal(
+    secondResult.type,
+    UrlbarUtils.RESULT_TYPE.REMOTE_TAB,
+    "The second result is a remote tab."
+  );
+  let thirdResult = await UrlbarTestUtils.getDetailsOfResultAt(window, 1);
+  Assert.equal(
+    thirdResult.type,
+    UrlbarUtils.RESULT_TYPE.REMOTE_TAB,
+    "The third result is a remote tab."
+  );
+
+  await UrlbarTestUtils.enterSearchMode(window);
+
+  Assert.equal(
+    UrlbarTestUtils.getResultCount(window),
+    2,
+    "We have two results. The second remote tab result is excluded despite matching the search string."
+  );
+  firstResult = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+  Assert.equal(
+    firstResult.type,
+    UrlbarUtils.RESULT_TYPE.SEARCH,
+    "The first result is the heuristic search result."
+  );
+  secondResult = await UrlbarTestUtils.getDetailsOfResultAt(window, 1);
+  Assert.equal(
+    secondResult.type,
+    UrlbarUtils.RESULT_TYPE.REMOTE_TAB,
+    "The second result is a remote tab."
+  );
+
+  await UrlbarTestUtils.exitSearchMode(window);
+  await UrlbarTestUtils.promisePopupClose(window);
+});
+
+// For engines with an invalid TLD, we filter on the entire domain.
+add_task(async function malformedEngine() {
+  let badEngine = await Services.search.addEngineWithDetails("TestMalformed", {
+    template: `http://example.foobar/?search={searchTerms}`,
+  });
+
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    value: "example",
+  });
+  Assert.equal(
+    UrlbarTestUtils.getResultCount(window),
+    3,
+    "We have three results"
+  );
+  let firstResult = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+  Assert.equal(
+    firstResult.type,
+    UrlbarUtils.RESULT_TYPE.SEARCH,
+    "The first result is the heuristic search result."
+  );
+  let secondResult = await UrlbarTestUtils.getDetailsOfResultAt(window, 1);
+  Assert.equal(
+    secondResult.type,
+    UrlbarUtils.RESULT_TYPE.REMOTE_TAB,
+    "The second result is a remote tab."
+  );
+  let thirdResult = await UrlbarTestUtils.getDetailsOfResultAt(window, 1);
+  Assert.equal(
+    thirdResult.type,
+    UrlbarUtils.RESULT_TYPE.REMOTE_TAB,
+    "The third result is a remote tab."
+  );
+
+  await UrlbarTestUtils.enterSearchMode(window, {
+    engineName: badEngine.name,
+  });
+
+  Assert.equal(
+    UrlbarTestUtils.getResultCount(window),
+    1,
+    "We only have one result."
+  );
+  firstResult = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+  Assert.ok(firstResult.heuristic, "The first result is heuristic.");
+  Assert.equal(
+    firstResult.type,
+    UrlbarUtils.RESULT_TYPE.SEARCH,
+    "The first result is the heuristic search result."
+  );
+
+  await UrlbarTestUtils.exitSearchMode(window);
+  await UrlbarTestUtils.promisePopupClose(window);
+  await Services.search.removeEngine(badEngine);
+});

browser/components/urlbar/tests/browser/browser_view_resultTypes_display.js

@@ -43,9 +43,6 @@ add_task(async function setup() {
       ["browser.urlbar.matchBuckets", "general:5,suggestion:4"],
       // Turn autofill off.
       ["browser.urlbar.autoFill", false],
-      // Special prefs for remote tabs.
-      ["services.sync.username", "fake"],
-      ["services.sync.syncedTabs.showRemoteTabs", true],
     ],
   });
 
@@ -237,6 +234,12 @@ add_task(async function test_omnibox_result() {
 });
 
 add_task(async function test_remote_tab_result() {
+  await SpecialPowers.pushPrefEnv({
+    set: [
+      ["services.sync.username", "fake"],
+      ["services.sync.syncedTabs.showRemoteTabs", true],
+    ],
+  });
   // Clear history so that history added by previous tests doesn't mess up this
   // test when it selects results in the urlbar.
   await PlacesUtils.history.clear();
@@ -312,4 +315,5 @@ add_task(async function test_remote_tab_result() {
       type: UrlbarUtils.RESULT_TYPE.REMOTE_TAB,
     });
   });
+  await SpecialPowers.popPrefEnv();
 });

browser/components/urlbar/tests/unit/test_UrlbarSearchUtils.jsm

@@ -227,6 +227,38 @@ add_task(async function test_serps_are_equivalent() {
   Assert.ok(UrlbarSearchUtils.serpsAreEquivalent(url1, url2, ["abc", "foo"]));
 });
 
+add_task(async function test_get_root_domain_from_engine() {
+  let engine = await Services.search.addEngineWithDetails("TestEngine2", {
+    template: "http://example.com",
+  });
+  Assert.equal(UrlbarSearchUtils.getRootDomainFromEngine(engine), "example");
+  await Services.search.removeEngine(engine);
+
+  engine = await Services.search.addEngineWithDetails("TestEngine", {
+    template: "http://www.subdomain.othersubdomain.example.com",
+  });
+  Assert.equal(UrlbarSearchUtils.getRootDomainFromEngine(engine), "example");
+  await Services.search.removeEngine(engine);
+
+  // We let engines with URL ending in .test through even though its not a valid
+  // TLD.
+  engine = await Services.search.addEngineWithDetails("TestMalformed", {
+    template: `http://mochi.test/?search={searchTerms}`,
+  });
+  Assert.equal(UrlbarSearchUtils.getRootDomainFromEngine(engine), "mochi");
+  await Services.search.removeEngine(engine);
+
+  // We return the domain for engines with a malformed URL.
+  engine = await Services.search.addEngineWithDetails("TestMalformed", {
+    template: `http://subdomain.foobar/?search={searchTerms}`,
+  });
+  Assert.equal(
+    UrlbarSearchUtils.getRootDomainFromEngine(engine),
+    "subdomain.foobar"
+  );
+  await Services.search.removeEngine(engine);
+});
+
 function promiseSearchTopic(expectedVerb) {
   return new Promise(resolve => {
     Services.obs.addObserver(function observe(subject, topic, verb) {