Issue 6783 - opt-in to anti circumvention filter list on upgrade

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 40 matching lines @@
 {
   firstRun = FilterStorage.subscriptions.length == 0;
 
   if (firstRun && (!FilterStorage.firstRun || Prefs.currentVersion))
     reinitialized = true;
 
   Prefs.currentVersion = info.addonVersion;
 }
 
 /**
- * Determines whether to add the default ad blocking subscription.

[kzar, 2018/07/16 13:39:23]

Please can you change "subscription" to "subscriptions" and otherwise adjust
this comment? Also the function name itself.

+ * Determines whether to add the default ad blocking subscriptions.
  * Returns true, if there are no filter subscriptions besides those
  * other subscriptions added automatically, and no custom filters.
  *
  * On first run, this logic should always result in true since there
  * is no data and therefore no subscriptions. But it also causes the
- * default ad blocking subscription to be added again after some
+ * default ad blocking subscriptions to be added again after some
  * data corruption or misconfiguration.
  *
  * @return {boolean}
  */
-function shouldAddDefaultSubscription()
+function shouldAddDefaultSubscriptions()
 {
   for (let subscription of FilterStorage.subscriptions)
   {
     if (subscription instanceof DownloadableSubscription &&
         subscription.url != Prefs.subscriptions_exceptionsurl &&
         subscription.url != Prefs.subscriptions_antiadblockurl)
       return false;
 
     if (subscription instanceof SpecialSubscription &&
         subscription.filters.length > 0)
@@ skipping 120 matching lines @@
     if (supportsNotificationsWithButtons())
     {
       let antiAdblockSubscription = Subscription.fromURL(
         Prefs.subscriptions_antiadblockurl
       );
       antiAdblockSubscription.disabled = true;
       subscriptions.push(antiAdblockSubscription);
     }
   }
 
-  // Add default ad blocking subscription (e.g. EasyList)

[kzar, 2018/07/16 13:39:23]

Please could you fix this comment?

[hub, 2018/07/16 15:50:06]

Done.

-  // Ensure the anti-circumvention subscription is added on upgrade.

[kzar, 2018/07/16 13:39:23]

Nit: Please could you move this line of the comment that you added above the
code you added line 234?

[hub, 2018/07/16 15:50:06]

Done (and edited).

-  let addDefaultSubscription = shouldAddDefaultSubscription();
-  if (addDefaultSubscription || !Prefs.subscriptions_checkedanticv)
+  // Add default ad blocking subscriptions (e.g. EasyList, Anti-Circumvention)
+  let addDefaultSubscription = shouldAddDefaultSubscriptions();
+  if (addDefaultSubscription || !Prefs.subscriptions_addedanticv)
   {
     return fetch("subscriptions.xml")
       .then(response => response.text())
       .then(text =>
       {
         let doc = new DOMParser().parseFromString(text, "application/xml");
         let nodes = doc.getElementsByTagName("subscription");
 
         let defaultSubscriptions = chooseFilterSubscriptions(nodes);
         if (defaultSubscriptions)
         {
           for (let name in defaultSubscriptions)
           {
             let node = defaultSubscriptions[name];
             if (!node)
               continue;
 
-            let type = node.getAttribute("type");

[kzar, 2018/07/16 13:39:23]

I wonder why we do this before the `url` check, since our circumvention filter
will always have a URL right?

[hub, 2018/07/16 15:50:06]

True that.
Moved inside the if (url)

-            if (!addDefaultSubscription && type != "circumvention")
-              continue;
-
             let url = node.getAttribute("url");
             if (url)
             {
+              // Make sure that we don't add Easylist again if we want
+              // to just add the Anti-Circumvention subscription.
+              let type = node.getAttribute("type");
+              if (!addDefaultSubscription && type != "circumvention")
+                continue;
+
               let subscription = Subscription.fromURL(url);
               subscription.disabled = false;
               subscription.title = node.getAttribute("title");
               subscription.homepage = node.getAttribute("homepage");
               subscription.type = type;
               subscriptions.push(subscription);
               if (subscription.type == "circumvention")
-                Prefs.subscriptions_checkedanticv = true;
+                Prefs.subscriptions_addedanticv = true;
             }
           }
         }
 
         return subscriptions;
       });
   }
 
   return subscriptions;
 }
@@ skipping 73 matching lines @@
  *
  * @param {function} callback
  */
 exports.setSubscriptionsCallback = callback =>
 {
   subscriptionsCallback = callback;
 };
 
 // Exports for tests only
 exports.chooseFilterSubscriptions = chooseFilterSubscriptions;