Issue 3890 - Fix "Downloading..." indication for subscriptions on the options page

options.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * 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
@@ skipping 183 matching lines @@
       if (title)
         element.setAttribute("data-search", title.toLowerCase());
       var control = element.querySelector(".control[role='checkbox']");
       if (control)
       {
         control.setAttribute("aria-checked", item.disabled == false);
         if (this._isControlDisabled(item, i))
           control.setAttribute("disabled", true);
       }
 
-      var downloadStatus = item.downloadStatus;
       var dateElement = element.querySelector(".date");
       var timeElement = element.querySelector(".time");
-      if(dateElement && timeElement)
+      if (dateElement && timeElement)
       {
         var message = element.querySelector(".message");
-        ext.backgroundPage.sendMessage(
+        if (item.isDownloading)
         {
-          type: "subscriptions.isDownloading",
-          url: item.url
-        },
-        function(isDownloading)
+          var text = getMessage("options_filterList_lastDownload_inProgress");
+          message.textContent = text;
+          element.classList.add("show-message");
+        }
+        else if (item.downloadStatus != "synchronize_ok")
         {
-          if (isDownloading)
-          {
-            var text = getMessage("options_filterList_lastDownload_inProgress");
-            message.textContent = text;
-            element.classList.add("show-message");
-          }
-          else if (downloadStatus && downloadStatus != "synchronize_ok")
-          {
-            if (downloadStatus in filterErrors)
-              message.textContent = getMessage(filterErrors[downloadStatus]);
-            else
-              message.textContent = item.downloadStatus;
-            element.classList.add("show-message");
-          }
-          else if (item.lastDownload > 0)
-          {
-            var dateTime = i18n_formatDateTime(item.lastDownload * 1000);
-            dateElement.textContent = dateTime[0];
-            timeElement.textContent = dateTime[1];
-            element.classList.remove("show-message");
-          }
-        });
+          var error = filterErrors[item.downloadStatus];
+          if (error)
+            message.textContent = getMessage(error);
+          else
+            message.textContent = item.downloadStatus;
+          element.classList.add("show-message");
+        }
+        else if (item.lastDownload > 0)
+        {
+          var dateTime = i18n_formatDateTime(item.lastDownload * 1000);
+          dateElement.textContent = dateTime[0];
+          timeElement.textContent = dateTime[1];
+          element.classList.remove("show-message");
+        }
       }
+
       var websiteElement = element.querySelector(".context-menu .website");
       var sourceElement = element.querySelector(".context-menu .source");
       if (websiteElement && item.homepage)
         websiteElement.setAttribute("href", item.homepage);
       if (sourceElement)
         sourceElement.setAttribute("href", item.url);
     }
   };
 
   Collection.prototype.clearAll = function()
@@ skipping 161 matching lines @@
             }
           }
         }
         for (var i in collections)
           collections[i].updateItem(subscription);
       }
     }
 
     if (!Object.observe)
     {
-      ["disabled", "lastDownload"].forEach(function(property)
+      Object.keys(subscription).forEach(function(property)

[Sebastian Noack, 2016/04/05 18:38:49]

We would have to add "isDownloading" and "downloadStatus" to the list. However,
if we simply observe all properties, we don't need any hard-coded list, and the
behavior is also more consistent with Object.observe().

       {
-        subscription["$" + property] = subscription[property];
+        var value = subscription[property];

[Sebastian Noack, 2016/04/05 18:38:49]

This is unrelated, but why using a prefixed key, if we can just have closure
variable. That simplifies the logic below quite a bit, and minimizes potential
side effects.

         Object.defineProperty(subscription, property,
         {
           get: function()
           {
-            return this["$" + property];
+            return value;
           },
           set: function(newValue)
           {
-            var oldValue = this["$" + property];
-            if (oldValue != newValue)
+            if (value != newValue)
             {
-              this["$" + property] = newValue;
-              var change = Object.create(null);
-              change.name = property;
-              onObjectChanged([change]);
+              value = newValue;
+              onObjectChanged([{name: property}]);
             }
           }
         });
       });
     }
     else
     {
       Object.observe(subscription, onObjectChanged);
     }
   }
@@ skipping 529 matching lines @@
         if (knownSubscription)
           collections.filterLists.addItems(knownSubscription);
         else
           collections.filterLists.addItems(subscription);
         break;
       case "disabled":
         updateSubscription(subscription);
         updateShareLink();
         break;
       case "lastDownload":
+      case "downloadStatus":
+      case "downloading":
         updateSubscription(subscription);
         break;
       case "homepage":
         // TODO: NYI
         break;
       case "removed":
         var knownSubscription = subscriptionsMap[subscription.url];
         getAcceptableAdsURL(function(acceptableAdsUrl)
         {
           if (subscription.url == acceptableAdsUrl)
@@ skipping 168 matching lines @@
   {
     type: "prefs.listen",
     filter: ["notifications_ignoredcategories", "notifications_showui",
         "safari_contentblocker", "show_devtools_panel",
         "shouldShowBlockElementMenu"]
   });
   ext.backgroundPage.sendMessage(
   {
     type: "subscriptions.listen",
     filter: ["added", "disabled", "homepage", "lastDownload", "removed",
-        "title"]
+        "title", "downloadStatus", "downloading"]
   });
 
   window.addEventListener("DOMContentLoaded", onDOMLoaded, false);
 })();