Issue 3915 - Properly handle special subscriptions

options.js

Index: options.js
diff --git a/options.js b/options.js
index 2d01e713331036259ac4423439a4b31d600d6e64..b5656b0ad0f0d363be84bad1d9690eeb68e88056 100644
--- a/options.js
+++ b/options.js
@@ -192,6 +192,21 @@ function loadOptions()
 }
 $(loadOptions);
 
+function convertSpecialSubscription(subscription)
+{
+  getFilters(subscription.url, function(filters)
+  {
+    for (var j = 0; j < filters.length; j++)
+    {
+      var filter = filters[j].text;
+      if (whitelistedDomainRegexp.test(filter))
+        appendToListBox("excludedDomainsBox", RegExp.$1);

[Sebastian Noack, 2016/04/12 12:26:06]

RegExp.$* is deprecated. However, since this is code is matter to be replaced
with the new options page anyway, I leave it up to you whether to address this.

[kzar, 2016/04/12 12:31:49]

I would rather not mess with it. (Note this is not new code, I simply moved it
into a function.)

[Sebastian Noack, 2016/04/12 12:43:21]

I know that the logic here isn't new. But this would be a good opportunity to
clean it up. In particular since we have to test the whole options page anyway
after the recent changes. However, as this code is thrown away anyway when we
have the new options page, fine with me.

+      else
+        appendToListBox("userFiltersBox", filter);
+    }
+  });
+}
+
 // Reloads the displayed subscriptions and filters
 function reloadFilters()
 {
@@ -219,17 +234,7 @@ function reloadFilters()
     clearListBox("excludedDomainsBox");
 
     for (var i = 0; i < subscriptions.length; i++)
-      getFilters(subscriptions[i].url, function(filters)
-      {
-        for (var j = 0; j < filters.length; j++)
-        {
-          var filter = filters[j].text;
-          if (whitelistedDomainRegexp.test(filter))
-            appendToListBox("excludedDomainsBox", RegExp.$1);
-          else
-            appendToListBox("userFiltersBox", filter);
-        }
-      });
+      convertSpecialSubscription(subscriptions[i]);
   });
 }
 
@@ -469,7 +474,9 @@ function onSubscriptionMessage(action, subscription)
         updateSubscriptionInfo(element, subscription);
       break;
     case "added":
-      if (subscription.url == acceptableAdsUrl)
+      if (subscription.special)

[Sebastian Noack, 2016/04/12 12:26:06]

If we add this property it should be isSpecial.

[kzar, 2016/04/12 12:31:49]

I disagree as this way it's consistent with the `subscriptions.get` message. I
guess let's leave this up to Thomas to decide.

[Sebastian Noack, 2016/04/12 12:43:21]

What does have the semantics of query parameters have to do with properties
describing an object? While speaking of consistency, also note that there is
already isDownloading.

[kzar, 2016/04/13 08:15:13]

Fair enough, I renamed these in the new review.

+        convertSpecialSubscription(subscription);
+      else if (subscription.url == acceptableAdsUrl)
         $("#acceptableAds").prop("checked", true);
       else if (!element)
         addSubscriptionEntry(subscription);