Issue 2824 - Only consider ads subscriptions in chooseFilterSubscription

lib/subscriptionInit.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 63 matching lines @@
 
     if (subscription instanceof SpecialSubscription &&
         subscription.filters.length > 0)
       return false;
   }
 
   return true;
 }
 
 /**
+ * Returns the default filter subscriptions to add.

[Sebastian Noack, 2017/10/03 00:24:40]

I wouldn't insist on adding comments for functions that are not exported. But if
you feel like adding a comment for this function, at least make sure that it
contains any more information that its name already does:

  Finds the element for the default ad blocking filter subscription based
  on the UI language.

[wspee, 2017/10/03 09:26:54]

Done.

+ *
+ * @param {HTMLCollection} subscriptions
+ * @return {Element}
+ */
+function chooseFilterSubscription(subscriptions)
+{
+  let selectedItem = null;
+  let selectedPrefix = null;
+  let matchCount = 0;
+  for (let subscription of subscriptions)
+  {
+    if (!selectedItem)
+      selectedItem = subscription;
+
+    let prefix = Utils.checkLocalePrefixMatch(

[Sebastian Noack, 2017/10/03 00:24:40]

Since this seems to be the only call to that function in the whole code base, we
should move it into this module as well, or perhaps even inline the logic:

  let prefixes = subscription.getAttribute("prefixes");
  let prefix = prefixes && prefixes.split(",").find(
    prefix => new RegExp("^" + prefix + "\\b").test(Utils.appLocale)
  );

[wspee, 2017/10/03 09:26:55]

Done.

+      subscription.getAttribute("prefixes")
+    );
+
+    let subscriptionType = subscription.getAttribute("type");
+    if (subscriptionType != "ads")

[Sebastian Noack, 2017/10/03 00:24:40]

This could be combined with the check below:

  if (prefix && subscriptionType != "ads")
  {
    ...
  }

[wspee, 2017/10/03 09:26:54]

Done.

+    {
+      continue;
+    }
+
+    if (prefix)
+    {
+      if (!selectedPrefix || selectedPrefix.length < prefix.length)
+      {
+        selectedItem = subscription;
+        selectedPrefix = prefix;
+        matchCount = 1;
+      }
+      else if (selectedPrefix && selectedPrefix.length == prefix.length)
+      {
+        matchCount++;
+
+        // If multiple items have a matching prefix of the same length:
+        // Select one of the items randomly, probability should be the same
+        // for all items. So we replace the previous match here with
+        // probability 1/N (N being the number of matches).
+        if (Math.random() * matchCount < 1)
+        {
+          selectedItem = subscription;
+          selectedPrefix = prefix;
+        }
+      }
+    }
+  }
+  return selectedItem;
+}
+
+/**
  * Gets the filter subscriptions to be added when the extnesion is loaded.
  *
  * @return {Promise|Subscription[]}
  */
 function getSubscriptions()
 {
   let subscriptions = [];
 
   // Add pre-configured subscriptions
   for (let url of Prefs.additional_subscriptions)
@@ skipping 18 matching lines @@
   // Add default ad blocking subscription (e.g. EasyList)
   if (shouldAddDefaultSubscription())
   {
     return fetch("subscriptions.xml")
       .then(response => response.text())
       .then(text =>
       {
         let doc = new DOMParser().parseFromString(text, "application/xml");
         let nodes = doc.getElementsByTagName("subscription");
 
-        let node = Utils.chooseFilterSubscription(nodes);
+        let node = chooseFilterSubscription(nodes);
         if (node)
         {
           let url = node.getAttribute("url");
           if (url)
           {
             let subscription = Subscription.fromURL(url);
             subscription.disabled = false;
             subscription.title = node.getAttribute("title");
             subscription.homepage = node.getAttribute("homepage");
             subscriptions.push(subscription);
@@ skipping 44 matching lines @@
  * Sets a callback that is called with an array of subscriptions to be added
  * during initialization. The callback must return an array of subscriptions
  * that will effectively be added.
  *
  * @param {function} callback
  */
 exports.setSubscriptionsCallback = callback =>
 {
   subscriptionsCallback = callback;
 };