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

options.js

Index: options.js
===================================================================
--- a/options.js
+++ b/options.js
@@ -201,42 +201,35 @@
           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)
@@ -418,24 +411,21 @@
 
     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}]);
             }
           }
         });
@@ -985,6 +975,8 @@
         updateShareLink();
         break;
       case "lastDownload":
+      case "downloadStatus":
+      case "downloading":
         updateSubscription(subscription);
         break;
       case "homepage":
@@ -1173,7 +1165,7 @@
   {
     type: "subscriptions.listen",
     filter: ["added", "disabled", "homepage", "lastDownload", "removed",
-        "title"]
+        "title", "downloadStatus", "downloading"]
   });
 
   window.addEventListener("DOMContentLoaded", onDOMLoaded, false);