Issue 2375 - Implement "Blocking lists" section in new options page

messageResponder.js

Index: messageResponder.js
===================================================================
--- a/messageResponder.js
+++ b/messageResponder.js
@@ -47,7 +47,7 @@
   }
 
   var convertSubscription = convertObject.bind(null, ["disabled",
-    "downloadStatus", "homepage", "lastSuccess", "title", "url"]);
+    "downloadStatus", "lastDownload", "homepage", "lastSuccess", "title", "url"]);

[Thomas Greiner, 2016/01/19 11:27:27]

Do we want "lastDownload", "lastSuccess" or both?

[Thomas Greiner, 2016/01/19 11:27:27]

Detail: What about keeping it alphabetically ordered and considering the 80
character line limitation?

[saroyanm, 2016/01/22 09:55:05]

Done.

[saroyanm, 2016/01/22 09:55:06]

In current option page, seems like we are using lastDownload, so I'll keep
lastDownload for now, in case we will need lastSuccess in future we can always
add.

[Thomas Greiner, 2016/01/25 15:40:27]

There's already a ticket requesting for this information to be shown
additionally in case of an error (see
https://issues.adblockplus.org/ticket/2848) so we can work on it separately.

[saroyanm, 2016/01/26 18:36:14]

Acknowledged.

   var convertFilter = convertObject.bind(null, ["text"]);
 
   var changeListeners = null;
@@ -333,8 +333,17 @@
         break;
       case "subscriptions.toggle":
         var subscription = Subscription.fromURL(message.url);
-        if (subscription.url in FilterStorage.knownSubscriptions && !subscription.disabled)
-          FilterStorage.removeSubscription(subscription);
+        if (subscription.url in FilterStorage.knownSubscriptions)
+        {
+          if (subscription.disabled)
+          {
+            subscription.disabled = !subscription.disabled;
+            FilterNotifier.triggerListeners("subscription.disabled",
+                                            subscription);
+          }
+          else
+            FilterStorage.removeSubscription(subscription);
+        }
         else
         {
           subscription.disabled = false;
@@ -345,6 +354,33 @@
             Synchronizer.execute(subscription);
         }
         break;
+      case "subscriptions.toggleState":

[Thomas Greiner, 2016/01/19 11:27:27]

I'd rather add this to "subscriptions.toggle" to avoid duplication. You can
introduce the flag "keepInstalled" to indicate whether it should be
added/removed or merely en-/disabled.

[saroyanm, 2016/01/22 09:55:06]

Done.

+        var subscription = Subscription.fromURL(message.url);
+        if (subscription.url in FilterStorage.knownSubscriptions)
+        {
+          subscription.disabled = !subscription.disabled;
+          FilterNotifier.triggerListeners("subscription.disabled", 
+                                          subscription);
+        }
+        break;
+      case "subscriptions.updateAll":

[Thomas Greiner, 2016/01/19 11:27:27]

Why not add this to "subscriptions.update"? If `message.url` is not given we
could update all.

[saroyanm, 2016/01/22 09:55:06]

Done.

+        for (var i = 0; i < FilterStorage.subscriptions.length; i++)
+        {
+          var subscription = FilterStorage.subscriptions[i];
+          if (subscription instanceof DownloadableSubscription)
+            Synchronizer.execute(subscription, true);
+        }
+        break;
+      case "subscriptions.update":
+        var subscription = Subscription.fromURL(message.url);
+        if (subscription instanceof DownloadableSubscription)
+            Synchronizer.execute(subscription, true);
+        break;
+      case "subscriptions.website":

[Thomas Greiner, 2016/01/19 11:27:27]

This belongs into "app.open".

However, I'd suggest going with regular hyperlinks for the subscription homepage
and source. Or is there a need for opening them this way?

[saroyanm, 2016/01/22 09:55:05]

Done.

+        var subscription = Subscription.fromURL(message.url);
+        if(subscription.homepage)
+          window.open(subscription.homepage);
+        break;
     }
   });
 })(this);