Bug 1900822 - Remove SearchEngine.searchForm use from new tab search shortcuts code. r=Standard8,home-newtab-reviewers,nbarrett

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

browser/components/newtab/lib/SearchShortcuts.sys.mjs

@@ -50,14 +50,6 @@ export function getSearchProvider(candidateShortURL) {
   );
 }
 
-// Get the search form URL for a given search keyword. This allows us to pick
-// different tippytop icons for the different variants. Sush as yandex.com vs. yandex.ru.
-// See more details in bug 1643523.
-export async function getSearchFormURL(keyword) {
-  const engine = await Services.search.getEngineByAlias(keyword);
-  return engine?.wrappedJSObject._searchForm;
-}
-
 // Check topsite against predefined list of valid search engines
 // https://searchfox.org/mozilla-central/rev/ca869724246f4230b272ed1c8b9944596e80d920/toolkit/components/search/nsSearchService.js#939
 export async function checkHasSearchEngine(keyword) {

browser/components/newtab/lib/ShortURL.sys.mjs

@@ -50,6 +50,28 @@ export function getETLD(host) {
   }
 }
 
+/**
+ * shortHostname - Creates a short version of a hostname, used for display purposes
+ *            e.g. "www.foosite.com"  =>  "foosite"
+ *
+ * @param {string} hostname The full hostname
+ * @returns {string} The shortened hostname
+ */
+export function shortHostname(hostname) {
+  if (!hostname) {
+    return "";
+  }
+
+  const newHostname = hostname.replace(/^www\./i, "").toLowerCase();
+
+  // Remove the eTLD (e.g., com, net) and the preceding period from the hostname
+  const eTLD = getETLD(newHostname);
+  const eTLDExtra =
+    eTLD.length && newHostname.endsWith(eTLD) ? -(eTLD.length + 1) : Infinity;
+
+  return handleIDNHost(newHostname.slice(0, eTLDExtra) || newHostname);
+}
+
 /**
  * shortURL - Creates a short version of a link's url, used for display purposes
  *            e.g. {url: http://www.foosite.com}  =>  "foosite"
@@ -72,17 +94,6 @@ export function shortURL({ url }) {
     return url;
   }
 
-  // Clean up the url (lowercase hostname via URL and remove www.)
-  const hostname = parsed.hostname.replace(/^www\./i, "");
-
-  // Remove the eTLD (e.g., com, net) and the preceding period from the hostname
-  const eTLD = getETLD(hostname);
-  const eTLDExtra = eTLD.length ? -(eTLD.length + 1) : Infinity;
-
   // Ideally get the short eTLD-less host but fall back to longer url parts
-  return (
-    handleIDNHost(hostname.slice(0, eTLDExtra) || hostname) ||
-    parsed.pathname ||
-    parsed.href
-  );
+  return shortHostname(parsed.hostname) || parsed.pathname || parsed.href;
 }

browser/components/newtab/lib/TopSitesFeed.sys.mjs

@@ -12,7 +12,10 @@ import {
   TOP_SITES_MAX_SITES_PER_ROW,
 } from "resource://activity-stream/common/Reducers.sys.mjs";
 import { Dedupe } from "resource://activity-stream/common/Dedupe.sys.mjs";
-import { shortURL } from "resource://activity-stream/lib/ShortURL.sys.mjs";
+import {
+  shortURL,
+  shortHostname,
+} from "resource://activity-stream/lib/ShortURL.sys.mjs";
 import { getDefaultOptions } from "resource://activity-stream/lib/ActivityStreamStorage.sys.mjs";
 
 import {
@@ -22,7 +25,6 @@ import {
   SEARCH_SHORTCUTS_HAVE_PINNED_PREF,
   checkHasSearchEngine,
   getSearchProvider,
-  getSearchFormURL,
 } from "resource://activity-stream/lib/SearchShortcuts.sys.mjs";
 
 const lazy = {};
@@ -130,9 +132,8 @@ const DISPLAY_FAIL_REASON_OVERSOLD = "oversold";
 const DISPLAY_FAIL_REASON_DISMISSED = "dismissed";
 const DISPLAY_FAIL_REASON_UNRESOLVED = "unresolved";
 
-function getShortURLForCurrentSearch() {
-  const url = shortURL({ url: Services.search.defaultEngine.searchForm });
-  return url;
+function getShortHostnameForCurrentSearch() {
+  return shortHostname(Services.search.defaultEngine.searchUrlDomain);
 }
 
 class TopSitesTelemetry {
@@ -555,7 +556,7 @@ export class TopSitesFeed {
     ChromeUtils.defineLazyGetter(
       this,
       "_currentSearchHostname",
-      getShortURLForCurrentSearch
+      getShortHostnameForCurrentSearch
     );
     this.dedupe = new Dedupe(this._dedupeKey);
     this.frecentCache = new lazy.LinksCache(
@@ -625,7 +626,7 @@ export class TopSitesFeed {
           this.store.getState().Prefs.values[FILTER_DEFAULT_SEARCH_PREF]
         ) {
           delete this._currentSearchHostname;
-          this._currentSearchHostname = getShortURLForCurrentSearch();
+          this._currentSearchHostname = getShortHostnameForCurrentSearch();
         }
         this.refresh({ broadcast: true });
         break;
@@ -1302,7 +1303,7 @@ export class TopSitesFeed {
         if (link.customScreenshotURL) {
           this._fetchScreenshot(link, link.customScreenshotURL, isStartup);
         } else if (link.searchTopSite && !link.isDefault) {
-          await this._attachTippyTopIconForSearchShortcut(link, link.label);
+          this._tippyTopProvider.processSite(link);
         } else {
           this._fetchIcon(link, isStartup);
         }
@@ -1434,30 +1435,6 @@ export class TopSitesFeed {
     return sponsored;
   }
 
-  /**
-   * Attach TippyTop icon to the given search shortcut
-   *
-   * Note that it queries the search form URL from search service For Yandex,
-   * and uses it to choose the best icon for its shortcut variants.
-   *
-   * @param {Object} link A link object with a `url` property
-   * @param {string} keyword Search keyword
-   */
-  async _attachTippyTopIconForSearchShortcut(link, keyword) {
-    if (
-      ["@\u044F\u043D\u0434\u0435\u043A\u0441", "@yandex"].includes(keyword)
-    ) {
-      let site = { url: link.url };
-      site.url = (await getSearchFormURL(keyword)) || site.url;
-      this._tippyTopProvider.processSite(site);
-      link.tippyTopIcon = site.tippyTopIcon;
-      link.smallFavicon = site.smallFavicon;
-      link.backgroundColor = site.backgroundColor;
-    } else {
-      this._tippyTopProvider.processSite(link);
-    }
-  }
-
   /**
    * Refresh the top sites data for content.
    * @param {bool} options.broadcast Should the update be broadcasted.
@@ -1554,7 +1531,7 @@ export class TopSitesFeed {
       );
       if (shortcut) {
         let clone = { ...shortcut };
-        await this._attachTippyTopIconForSearchShortcut(clone, clone.keyword);
+        this._tippyTopProvider.processSite(clone);
         searchShortcuts.push(clone);
       }
     }

browser/components/newtab/test/unit/lib/ShortUrl.test.js

@@ -1,5 +1,5 @@
 import { GlobalOverrider } from "test/unit/utils";
-import { shortURL } from "lib/ShortURL.sys.mjs";
+import { shortURL, shortHostname } from "lib/ShortURL.sys.mjs";
 
 const puny = "xn--kpry57d";
 const idn = "台灣";
@@ -102,3 +102,71 @@ describe("shortURL", () => {
     assert.equal(shortURL({ url: "about:" }), "about:");
   });
 });
+
+describe("shortHostname", () => {
+  let globals;
+  let IDNStub;
+  let getPublicSuffixFromHostStub;
+
+  beforeEach(() => {
+    IDNStub = sinon.stub().callsFake(host => host.replace(puny, idn));
+    getPublicSuffixFromHostStub = sinon.stub().returns("com");
+
+    globals = new GlobalOverrider();
+    globals.set("IDNService", { convertToDisplayIDN: IDNStub });
+    globals.set("Services", {
+      eTLD: { getPublicSuffixFromHost: getPublicSuffixFromHostStub },
+    });
+  });
+
+  afterEach(() => {
+    globals.restore();
+  });
+
+  it("should return a blank string if hostname is empty", () => {
+    assert.equal(shortHostname(""), "");
+  });
+
+  it("should return the input if it is not a hostname", () => {
+    const checkInvalid = url => assert.equal(shortHostname(url), url);
+    checkInvalid("foo.com/bar");
+    checkInvalid("something/something");
+    checkInvalid("http:");
+    checkInvalid("http::double");
+    checkInvalid("http://foo.com/");
+  });
+
+  it("should remove the eTLD", () => {
+    assert.equal(shortHostname("com.blah.com"), "com.blah");
+  });
+
+  it("should convert host to idn when calling shortHostname", () => {
+    assert.equal(shortHostname(`${puny}.blah.com`), `${idn}.blah`);
+  });
+
+  it("should not strip out www if not first subdomain", () => {
+    assert.equal(shortHostname("foo.www.com"), "foo.www");
+  });
+
+  it("should convert to lowercase", () => {
+    assert.equal(shortHostname("FOO.COM"), "foo");
+  });
+
+  it("should return hostname for ip address", () => {
+    getPublicSuffixFromHostStub.throws("host is ip address");
+
+    assert.equal(shortHostname("127.0.0.1"), "127.0.0.1");
+  });
+
+  it("should return etld for www.gov.uk (www-only non-etld)", () => {
+    getPublicSuffixFromHostStub.returns("gov.uk");
+
+    assert.equal(shortHostname("www.gov.uk"), "gov.uk");
+  });
+
+  it("should return idn etld for www-only non-etld", () => {
+    getPublicSuffixFromHostStub.returns(puny);
+
+    assert.equal(shortHostname(`www.${puny}`), idn);
+  });
+});

browser/components/newtab/test/xpcshell/test_TopSitesFeed.js

@@ -94,7 +94,7 @@ function getTopSitesFeedForTest(sandbox) {
 add_setup(async () => {
   let sandbox = sinon.createSandbox();
   sandbox.stub(SearchService.prototype, "defaultEngine").get(() => {
-    return { identifier: "ddg", searchForm: "https://duckduckgo.com" };
+    return { identifier: "ddg", searchUrlDomain: "duckduckgo.com" };
   });
 
   gGetTopSitesStub = sandbox

browser/components/topsites/TopSites.sys.mjs

@@ -8,13 +8,15 @@ import {
   TOP_SITES_MAX_SITES_PER_ROW,
 } from "resource://activity-stream/common/Reducers.sys.mjs";
 import { Dedupe } from "resource://activity-stream/common/Dedupe.sys.mjs";
-import { shortURL } from "resource://activity-stream/lib/ShortURL.sys.mjs";
+import {
+  shortURL,
+  shortHostname,
+} from "resource://activity-stream/lib/ShortURL.sys.mjs";
 
 import {
   CUSTOM_SEARCH_SHORTCUTS,
   checkHasSearchEngine,
   getSearchProvider,
-  getSearchFormURL,
 } from "resource://activity-stream/lib/SearchShortcuts.sys.mjs";
 
 const lazy = {};
@@ -72,8 +74,8 @@ const DEFAULT_SITES_OVERRIDE_PREF =
   "browser.newtabpage.activity-stream.default.sites";
 const DEFAULT_SITES_EXPERIMENTS_PREF_BRANCH = "browser.topsites.experiment.";
 
-function getShortURLForCurrentSearch() {
-  const url = shortURL({ url: Services.search.defaultEngine.searchForm });
+function getShortHostnameForCurrentSearch() {
+  const url = shortHostname(Services.search.defaultEngine.searchUrlDomain);
   return url;
 }
 
@@ -93,7 +95,7 @@ class _TopSites {
     ChromeUtils.defineLazyGetter(
       this,
       "_currentSearchHostname",
-      getShortURLForCurrentSearch
+      getShortHostnameForCurrentSearch
     );
     this.dedupe = new Dedupe(this._dedupeKey);
     this.frecentCache = new lazy.LinksCache(
@@ -207,7 +209,7 @@ class _TopSites {
           Services.prefs.getBoolPref(NO_DEFAULT_SEARCH_TILE_PREF, true)
         ) {
           delete this._currentSearchHostname;
-          this._currentSearchHostname = getShortURLForCurrentSearch();
+          this._currentSearchHostname = getShortHostnameForCurrentSearch();
         }
         this.refresh({ broadcast: true });
         break;
@@ -750,7 +752,7 @@ class _TopSites {
     for (const link of withPinned) {
       if (link) {
         if (link.searchTopSite && !link.isDefault) {
-          await this._attachTippyTopIconForSearchShortcut(link, link.label);
+          this._tippyTopProvider.processSite(link);
         } else {
           this._fetchIcon(link);
         }
@@ -768,30 +770,6 @@ class _TopSites {
     return withPinned;
   }
 
-  /**
-   * Attach TippyTop icon to the given search shortcut
-   *
-   * Note that it queries the search form URL from search service For Yandex,
-   * and uses it to choose the best icon for its shortcut variants.
-   *
-   * @param {object} link A link object with a `url` property
-   * @param {string} keyword Search keyword
-   */
-  async _attachTippyTopIconForSearchShortcut(link, keyword) {
-    if (
-      ["@\u044F\u043D\u0434\u0435\u043A\u0441", "@yandex"].includes(keyword)
-    ) {
-      let site = { url: link.url };
-      site.url = (await getSearchFormURL(keyword)) || site.url;
-      this._tippyTopProvider.processSite(site);
-      link.tippyTopIcon = site.tippyTopIcon;
-      link.smallFavicon = site.smallFavicon;
-      link.backgroundColor = site.backgroundColor;
-    } else {
-      this._tippyTopProvider.processSite(link);
-    }
-  }
-
   /**
    * Refresh the top sites data for content.
    *
@@ -841,7 +819,7 @@ class _TopSites {
       );
       if (shortcut) {
         let clone = { ...shortcut };
-        await this._attachTippyTopIconForSearchShortcut(clone, clone.keyword);
+        this._tippyTopProvider.processSite(clone);
         searchShortcuts.push(clone);
       }
     }

browser/components/topsites/test/unit/test_top_sites.js

@@ -109,7 +109,7 @@ add_setup(async () => {
 
   let sandbox = sinon.createSandbox();
   sandbox.stub(SearchService.prototype, "defaultEngine").get(() => {
-    return { identifier: "ddg", searchForm: "https://duckduckgo.com" };
+    return { identifier: "ddg", searchUrlDomain: "duckduckgo.com" };
   });
 
   gGetTopSitesStub = sandbox