Issue 2375 - Implement "Blocking lists" section in new 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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 "use strict";
 
 (function()
 {
   var subscriptionsMap = Object.create(null);
   var recommendationsMap = Object.create(null);
   var filtersMap = Object.create(null);
   var collections = Object.create(null);
   var maxLabelId = 0;
+  var getMessage = ext.i18n.getMessage;
+  var filterErrors =
+  {
+    "synchronize_invalid_url": "options_filterList_lastDownload_invalidURL",
+    "synchronize_connection_error": "options_filterList_lastDownload_connectionError",
+    "synchronize_invalid_data": "options_filterList_lastDownload_invalidData",
+    "synchronize_checksum_mismatch": "options_filterList_lastDownload_checksumMismatch"
+  };
 
   function Collection(details)
   {
     this.details = details;
     this.items = [];
   }
 
   Collection.prototype._setEmpty = function(table, text)
   {
     var placeholder = table.querySelector(".empty-placeholder");
     if (text && !placeholder)
     {
       placeholder = document.createElement("li");
       placeholder.className = "empty-placeholder";
-      placeholder.textContent = ext.i18n.getMessage(text);
+      placeholder.textContent = getMessage(text);
       table.appendChild(placeholder);
     }
     else if (placeholder)
       table.removeChild(placeholder);
   }
 
   Collection.prototype._createElementQuery = function(item)
   {
     var access = (item.url || item.text).replace(/'/g, "\\'");
     return function(container)
@@ skipping 93 matching lines @@
   Collection.prototype.updateItem = function(item)
   {
     var access = (item.url || item.text).replace(/'/g, "\\'");
     for (var i = 0; i < this.details.length; i++)
     {
       var table = E(this.details[i].id);
       var element = table.querySelector("[data-access='" + access + "']");
       if (!element)
         continue;
 
-      var text = item.title || item.url || item.text;
-      element.querySelector(".display").textContent = text;
-      if (text)
-        element.setAttribute("data-search", text.toLowerCase());
+      var title = item.title || item.url || item.text;
+      element.querySelector(".display").textContent = title;
+      if (title)
+        element.setAttribute("data-search", title.toLowerCase());
       var control = element.querySelector(".control[role='checkbox']");
       if (control)
         control.setAttribute("aria-checked", item.disabled == false);
 
+      var downloadStatus = item.downloadStatus;
       var dateElement = element.querySelector(".date");
       var timeElement = element.querySelector(".time");
       if(dateElement && timeElement)
       {
-        if (item.downloadStatus && 
-            item.downloadStatus != "synchronize_ok")
-        {
-          var map =

[Thomas Greiner, 2016/02/01 18:52:36]

Detail: This content is static so it should be defined outside to avoid
declaring it each time when the function gets called.

[saroyanm, 2016/02/03 14:04:26]

Done, also made some other function/variable assignments to improve readability.

-          {
-            "synchronize_invalid_url": "options_subscription_lastDownload_invalidURL",
-            "synchronize_connection_error": "options_subscription_lastDownload_connectionError",
-            "synchronize_invalid_data": "options_subscription_lastDownload_invalidData",
-            "synchronize_checksum_mismatch": "options_subscription_lastDownload_checksumMismatch"
-          };
-          if (item.downloadStatus in map)
-            timeElement.textContent = ext.i18n.getMessage(map[item.downloadStatus]);
-          else
-            timeElement.textContent = item.downloadStatus;
-        }
-        else if (item.lastDownload > 0)
-        {
-          var dateTime = i18n_formatDateTime(item.lastDownload * 1000);
-          dateElement.textContent = dateTime[0];
-          timeElement.textContent = dateTime[1];
-        }

[Thomas Greiner, 2016/02/01 18:52:35]

What about the else-case here? Currently, this will cause both fields to be
empty whenever a new subscription is added until the download finishes.

If you look at the current options page it is indicating "Downloading…" which is
also shown when updating a subscription. The latter one is completely missing
from the new options page so we should fix that as well.

[saroyanm, 2016/02/03 14:04:26]

Good point, sorry for that, Done.

+        var message = element.querySelector(".message");
+        ext.backgroundPage.sendMessage(
+        {
+          type: "subscriptions.isDownloading",
+          url: item.url
+        },
+        function(isDownloading)
+        {
+          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 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);
     }
   };
 
@@ skipping 159 matching lines @@
               collections.allLangs.addItems(subscription);
               collections.langs.removeItem(subscription);
             }
           }
         }
         for (var i in collections)
           collections[i].updateItem(subscription);
       }
     }
 
     if (!Object.observe)

[Thomas Greiner, 2016/02/01 18:52:35]

Since `Object.observe()` is not implemented in Safari and older Chrome versions
and is going to be removed at some point, have you tested whether the polyfill
still works?

[saroyanm, 2016/02/03 14:04:26]

Thanks for pointing that out, apparently we have to check also for the old value
of the Object property when using define property, otherwise the set event will
be called also in case the value of the property is same. Done.

     {
       ["disabled", "lastDownload"].forEach(function(property)
       {
         subscription["$" + property] = subscription[property];
         Object.defineProperty(subscription, property,
         {
           get: function()
           {
             return this["$" + property];
           },
-          set: function(value)
-          {
-            this["$" + property] = value;
-            var change = Object.create(null);
-            change.name = property;
-            onObjectChanged([change]);
+          set: function(newValue)
+          {
+            var oldValue = this["$" + property];
+            if (oldValue != newValue)
+            {
+              this["$" + property] = newValue;
+              var change = Object.create(null);
+              change.name = property;
+              onObjectChanged([change]);
+            }
           }
         });
       });
     }
     else
     {
       Object.observe(subscription, onObjectChanged);
     }
   }
 
@@ skipping 68 matching lines @@
           subscription.url = element.getAttribute("url");
           subscription.disabled = null;
           subscription.downloadStatus = null;
           subscription.homepage = null;
           var recommendation = Object.create(null);
           recommendation.type = element.getAttribute("type");
           var prefix = element.getAttribute("prefixes");
           if (prefix)
           {
             prefix = prefix.replace(/\W/g, "_");
-            subscription.title = ext.i18n.getMessage("options_language_" + prefix);
+            subscription.title = getMessage("options_language_" + prefix);
           }
           else
           {
             var type = recommendation.type.replace(/\W/g, "_");
-            subscription.title = ext.i18n.getMessage("common_feature_" + type + "_title");
+            subscription.title = getMessage("common_feature_" + type + "_title");
           }
 
           recommendationsMap[subscription.url] = recommendation;
           updateSubscription(subscription);
         }
       });
   }
 
   function findParentData(element, dataName, returnElement)
   {
@@ skipping 82 matching lines @@
           ext.backgroundPage.sendMessage(
           {
             type: "subscriptions.update"
           });
           break;
         case "open-context-menu":
           var listItem = findParentData(element, "access", true);
           if (listItem != context)
             listItem.classList.add("show-context-menu");
           break;
-        case "update-now":

[Thomas Greiner, 2016/02/01 18:52:36]

Detail: For consistency I'd suggest renaming this to "update-subscription". Also
because "update-now" could be confused with "update-all-subscriptions".

[saroyanm, 2016/02/03 14:04:27]

Fare enough, done.

+        case "update-subscription":
           ext.backgroundPage.sendMessage(
           {
             type: "subscriptions.update",
             url: findParentData(element, "access", false)
           });
           break;
         case "remove-subscription":
           ext.backgroundPage.sendMessage(
           {
             type: "subscriptions.remove",
@@ skipping 46 matching lines @@
 
     getDocLink("contribute", function(link)
     {
       document.querySelector("#tab-contribute a").setAttribute("href", link);
     });
 
     updateShareLink();
 
     // Initialize interactive UI elements
     document.body.addEventListener("click", onClick, false);
-    var placeholderValue  = ext.i18n.getMessage("options_dialog_language_find");
+    var placeholderValue  = getMessage("options_dialog_language_find");
     E("find-language").setAttribute("placeholder", placeholderValue);
     E("find-language").addEventListener("keyup", onFindLanguageKeyUp, false);
     E("whitelisting-textbox").addEventListener("keypress", function(e)
     {
       if (getKey(e) == "Enter")
         addWhitelistedDomain();
     }, false);
 
     // Advanced tab
     var filterTextbox = document.querySelector("#custom-filters-add input");
-    placeholderValue = ext.i18n.getMessage("options_customFilters_textbox_placeholder");
+    placeholderValue = getMessage("options_customFilters_textbox_placeholder");
     filterTextbox.setAttribute("placeholder", placeholderValue);
     function addCustomFilters()
     {
       var filterText = filterTextbox.value;
       ext.backgroundPage.sendMessage(
       {
         type: "filters.add",
         text: filterText
       });
       filterTextbox.value = "";
@@ skipping 87 matching lines @@
             updateFilter(filters[i]);
         });
       }
     });
     loadRecommendations();
     getAcceptableAdsURL(function(acceptableAdsUrl)
     {
       var subscription = Object.create(null);
       subscription.url = acceptableAdsUrl;
       subscription.disabled = true;
-      subscription.title = ext.i18n.getMessage("options_acceptableAds_description");
+      subscription.title = getMessage("options_acceptableAds_description");
       updateSubscription(subscription);
 
       // Load user subscriptions
       ext.backgroundPage.sendMessage(
       {
         type: "subscriptions.get",
         downloadable: true
       }, 
       function(subscriptions)
       {
@@ skipping 219 matching lines @@
     filter: ["added", "loaded", "removed"]
   });
   ext.backgroundPage.sendMessage(
   {
     type: "subscriptions.listen",
     filter: ["added", "disabled", "homepage", "lastDownload", "removed", "title"]
   });
 
   window.addEventListener("DOMContentLoaded", onDOMLoaded, false);
 })();