Bug 1768529 - Exclude URLs with a particular search param from appearing as tiles on the new-tab page. r=nanj

This modifies `ActivityStreamProvider.getTopFrecentSites()` so it excludes URLs
with a given search param. There are two questions I considered when writing
this patch:

* The top-sites code is a little complex and there are multiple places where
  this logic could be implemented. I chose the bottommost layer, where the top
  sites are fetched from Places and some other exclusion logic already exists,
  because we don't want these URLs to appear in any list of top sites in Firefox
  AFAIK -- not on the new-tab page and not in the urlbar.
* How should we encode this new exclusion logic? We don't want to hardcode a
  specific search param. We may not even want to base it on search params at all
  but instead make it more general somehow. For now, I did the simple thing and
  went ahead and based it on a search param, and I added a new option to
  `getTopFrecentSites()` that specifies the param and that falls back to a new
  pref. Callers can override the pref fallback by passing in an option.

The pref value and option to `getTopFrecentSites()` supports the following
forms:

* `""` (empty) - Disable this feature
* `"key"` - Search param named "key" with any or no value
* `"key="` - Search param named "key" with no value
* `"key=value"` - Search param named "key" with value "value"

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

browser/app/profile/firefox.js

@@ -1586,6 +1586,14 @@ pref("browser.newtabpage.activity-stream.improvesearch.handoffToAwesomebar", tru
 
 pref("browser.newtabpage.activity-stream.logowordmark.alwaysVisible", true);
 
+// URLs from the user's history that contain this search param will be hidden
+// from the top sites. The value is a string with one of the following forms:
+// - "" (empty) - Disable this feature
+// - "key" - Search param named "key" with any or no value
+// - "key=" - Search param named "key" with no value
+// - "key=value" - Search param named "key" with value "value"
+pref("browser.newtabpage.activity-stream.hideTopSitesWithSearchParam", "");
+
 // Used to display triplet cards on newtab
 pref("trailhead.firstrun.newtab.triplets", "");
 // Separate about welcome

toolkit/modules/NewTabUtils.jsm

@@ -1185,6 +1185,15 @@ var ActivityStreamProvider = {
    *   {int}  topsiteFrecency: Minimum amount of frecency for a site.
    *   {bool} onePerDomain: Dedupe the resulting list.
    *   {bool} includeFavicon: Include favicons if available.
+   *   {string} hideWithSearchParam: URLs that contain this search param will be
+   *     excluded from the returned links. This value should be either undefined
+   *     or a string with one of the following forms:
+   *     - undefined: Fall back to the value of pref
+   *       `browser.newtabpage.activity-stream.hideTopSitesWithSearchParam`
+   *     - "" (empty) - Disable this feature
+   *     - "key" - Search param named "key" with any or no value
+   *     - "key=" - Search param named "key" with no value
+   *     - "key=value" - Search param named "key" with value "value"
    *
    * @returns {Promise} Returns a promise with the array of links as payload.
    */
@@ -1196,6 +1205,9 @@ var ActivityStreamProvider = {
         topsiteFrecency: ACTIVITY_STREAM_DEFAULT_FRECENCY,
         onePerDomain: true,
         includeFavicon: true,
+        hideWithSearchParam: Services.prefs.getCharPref(
+          "browser.newtabpage.activity-stream.hideTopSitesWithSearchParam"
+        ),
       },
       aOptions || {}
     );
@@ -1283,6 +1295,20 @@ var ActivityStreamProvider = {
       });
     }
 
+    // Remove links that contain the hide-with search param.
+    if (options.hideWithSearchParam) {
+      let [key, value] = options.hideWithSearchParam.split("=");
+      links = links.filter(link => {
+        try {
+          let { searchParams } = new URL(link.url);
+          return value === undefined
+            ? !searchParams.has(key)
+            : !searchParams.getAll(key).includes(value);
+        } catch (error) {}
+        return true;
+      });
+    }
+
     // Remove any blocked links.
     if (!options.ignoreBlocked) {
       links = links.filter(link => !BlockedLinks.isBlocked(link));

toolkit/modules/tests/xpcshell/test_NewTabUtils.js

@@ -1206,6 +1206,101 @@ add_task(async function getTopFrecentSites_order() {
   );
 });
 
+add_task(async function getTopFrecentSites_hideWithSearchParam() {
+  await setUpActivityStreamTest();
+
+  let pref = "browser.newtabpage.activity-stream.hideTopSitesWithSearchParam";
+  let provider = NewTabUtils.activityStreamLinks;
+
+  // This maps URL search params to objects describing whether a URL with those
+  // params is expected to be included in the returned links. Each object maps
+  // from effective hide-with search params to whether the URL is expected to be
+  // included.
+  let tests = {
+    "": {
+      "": true,
+      test: true,
+      "test=": true,
+      "test=hide": true,
+      nomatch: true,
+      "nomatch=": true,
+      "nomatch=hide": true,
+    },
+    test: {
+      "": true,
+      test: false,
+      "test=": false,
+      "test=hide": true,
+      nomatch: true,
+      "nomatch=": true,
+      "nomatch=hide": true,
+    },
+    "test=hide": {
+      "": true,
+      test: false,
+      "test=": true,
+      "test=hide": false,
+      nomatch: true,
+      "nomatch=": true,
+      "nomatch=hide": true,
+    },
+    "test=foo&test=hide": {
+      "": true,
+      test: false,
+      "test=": true,
+      "test=hide": false,
+      nomatch: true,
+      "nomatch=": true,
+      "nomatch=hide": true,
+    },
+  };
+
+  for (let [urlParams, expected] of Object.entries(tests)) {
+    for (let prefValue of Object.keys(expected)) {
+      info(
+        "Running test: " + JSON.stringify({ urlParams, prefValue, expected })
+      );
+
+      // Add a visit to a URL with search params `urlParams`.
+      let url = new URL("http://example.com/");
+      url.search = urlParams;
+      await PlacesTestUtils.addVisits(url);
+
+      // Set the pref to `prefValue`.
+      Services.prefs.setCharPref(pref, prefValue);
+
+      // Call `getTopSites()` with all the test values for `hideWithSearchParam`
+      // plus undefined. When `hideWithSearchParam` is undefined, the pref value
+      // should be used. Otherwise it should override the pref.
+      for (let hideWithSearchParam of [undefined, ...Object.keys(expected)]) {
+        info(
+          "Calling getTopSites() with hideWithSearchParam: " +
+            JSON.stringify(hideWithSearchParam)
+        );
+
+        let options = { topsiteFrecency: 100 };
+        if (hideWithSearchParam !== undefined) {
+          options = { ...options, hideWithSearchParam };
+        }
+        let links = await provider.getTopSites(options);
+
+        let effectiveHideWithParam =
+          hideWithSearchParam === undefined ? prefValue : hideWithSearchParam;
+        if (expected[effectiveHideWithParam]) {
+          Assert.equal(links.length, 1, "One link returned");
+          Assert.equal(links[0].url, url.toString(), "Expected link returned");
+        } else {
+          Assert.equal(links.length, 0, "No links returned");
+        }
+      }
+
+      await PlacesUtils.history.clear();
+    }
+  }
+
+  Services.prefs.clearUserPref(pref);
+});
+
 add_task(async function activitySteamProvider_deleteHistoryLink() {
   await setUpActivityStreamTest();