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.

+ * Finds the element for the default ad blocking filter subscription based
+ * on the user's locale.
  *
  * @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 prefixes = subscription.getAttribute("prefixes");
+    let prefix = prefixes && prefixes.split(",").find(
+      lang => new RegExp("^" + lang + "\\b").test(Utils.appLocale)
     );
 
     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 (prefix && subscriptionType == "ads")
     {
       if (!selectedPrefix || selectedPrefix.length < prefix.length)
       {
         selectedItem = subscription;
         selectedPrefix = prefix;
         matchCount = 1;
       }
       else if (selectedPrefix && selectedPrefix.length == prefix.length)
       {
         matchCount++;
@@ skipping 110 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;
 };