Issue 2824 - Only consider ads subscriptions in chooseFilterSubscription

lib/subscriptionInit.js

Index: lib/subscriptionInit.js
diff --git a/lib/subscriptionInit.js b/lib/subscriptionInit.js
index 9cd17b48988b41eb21d071d7efcd7506557f0375..e7abaceac1234b9c1455ae04dc12b2b6d9d1cbad 100644
--- a/lib/subscriptionInit.js
+++ b/lib/subscriptionInit.js
@@ -81,6 +81,59 @@ function shouldAddDefaultSubscription()
 }
 
 /**
+ * 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[]}
@@ -119,7 +172,7 @@ function getSubscriptions()
         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");