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

messageResponder.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
- * Copyright (C) 2006-2015 Eyeo GmbH
+ * Copyright (C) 2006-2016 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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
@@ skipping 26 matching lines @@
 
   function convertObject(keys, obj)
   {
     var result = {};
     for (var i = 0; i < keys.length; i++)
       result[keys[i]] = obj[keys[i]];
     return result;
   }
 
   var convertSubscription = convertObject.bind(null, ["disabled",
-    "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.

+    "downloadStatus", "homepage", "lastDownload", "title", "url"]);
   var convertFilter = convertObject.bind(null, ["text"]);
 
   var changeListeners = null;
   var messageTypes = {
     "app": "app.listen",
     "filter": "filters.listen",
     "subscription": "subscriptions.listen"
   };
 
   function sendMessage(type, action, args, page)
@@ skipping 267 matching lines @@
         break;
       case "subscriptions.remove":
         var subscription = Subscription.fromURL(message.url);
         if (subscription.url in FilterStorage.knownSubscriptions)
           FilterStorage.removeSubscription(subscription);
         break;
       case "subscriptions.toggle":
         var subscription = Subscription.fromURL(message.url);
         if (subscription.url in FilterStorage.knownSubscriptions)
         {
-          if (subscription.disabled)
+          if (subscription.disabled || message.keepInstalled)
           {
             subscription.disabled = !subscription.disabled;
             FilterNotifier.triggerListeners("subscription.disabled",
                                             subscription);
           }
           else
             FilterStorage.removeSubscription(subscription);
         }
         else
         {
           subscription.disabled = false;
           subscription.title = message.title;
           subscription.homepage = message.homepage;
           FilterStorage.addSubscription(subscription);
           if (!subscription.lastDownload)
             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];
+      case "subscriptions.update":
+        var subscriptions = message.url ? [Subscription.fromURL(message.url)] : 
+                            FilterStorage.subscriptions;
+        for (var i = 0; i < subscriptions.length; i++)
+        {
+          var subscription = 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);
+      case "subscriptions.isDownloading":
+        callback(Synchronizer.isExecuting(message.url));
         break;
     }
   });
 })(this);