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

options.js

Index: options.js
===================================================================
--- a/options.js
+++ b/options.js
@@ -44,6 +44,15 @@
       table.removeChild(placeholder);
   }
 
+  Collection.prototype._createElementQuery = function(item)
+  {
+    var access = (item.url || item.text).replace(/'/g, "\\'");
+    return function(container)
+    {
+      return container.querySelector("[data-access='" + access + "']");
+    };
+  };
+
   Collection.prototype.addItems = function() 
   {
     var length = Array.prototype.push.apply(this.items, arguments);
@@ -72,6 +81,7 @@
         if (text)
           listItem.setAttribute("data-search", text.toLowerCase());
 
+        updateBlockingList(listItem, item);
         var control = listItem.querySelector(".control");
         if (control)
         {
@@ -81,7 +91,10 @@
 
         this._setEmpty(table, null);
         if (table.hasChildNodes())
-          table.insertBefore(listItem, table.childNodes[this.items.indexOf(item)]);
+        {
+          table.insertBefore(listItem, 
+                              table.childNodes[this.items.indexOf(item)]);
+        }
         else
           table.appendChild(listItem);
       }
@@ -96,10 +109,11 @@
       return;
 
     this.items.splice(index, 1);
+    var getListElement = this._createElementQuery(item);
     for (var i = 0; i < this.details.length; i++)
     {
       var table = E(this.details[i].id);
-      var element = table.childNodes[index];
+      var element = getListElement(table);
       element.parentElement.removeChild(element);
       if (this.items.length == 0)
         this._setEmpty(table, this.details[i].emptyText);
@@ -112,17 +126,69 @@
     for (var i = 0; i < this.details.length; i++)
     {
       var table = E(this.details[i].id);
-      var template = table.querySelector("template");
-      table.innerHTML = "";
-      table.appendChild(template);
+      var element = table.firstChild;
+      while (element)
+      {
+        if ((element.tagName == "LI" && !element.classList.contains("static")) ||
+            element.nodeType == 3)

[Thomas Greiner, 2016/01/27 17:17:00]

Again, checking for the "static" class when using `Element.nextElementSibling`
should be sufficient or not?

[saroyanm, 2016/01/28 17:00:14]

Done.

+          table.removeChild(element);
+        element = element.nextSibling
+      }
+
       this._setEmpty(table, this.details[i].emptyText);
     }
   };
 
-  function onToggleSubscriptionClick(e)
+  Collection.prototype.hasId = function(id) 
+  {
+    for (var i = 0; i < this.details.length; i++)
+      if (this.details[i].id == id)
+        return true;
+      
+    return false;
+  };
+
+  function updateBlockingList(listItem, subscription)
+  {
+    var dateElement = listItem.querySelector(".date");
+    var timeElement = listItem.querySelector(".time");
+
+    if(dateElement && timeElement)
+    {
+      if (subscription.downloadStatus && 
+          subscription.downloadStatus != "synchronize_ok")
+      {
+        var map =
+        {
+          "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 (subscription.downloadStatus in map)
+          timeElement.textContent = ext.i18n.getMessage(map[subscription.downloadStatus]);
+        else
+          timeElement.textContent = subscription.downloadStatus;
+      }
+      else if (subscription.lastDownload > 0)
+      {
+        var dateTime = i18n_formatDateTime(subscription.lastDownload * 1000);
+        dateElement.textContent = dateTime[0];
+        timeElement.textContent = dateTime[1];
+      }
+    }
+    var websiteElement = listItem.querySelector(".context-menu .website");
+    var sourceElement = listItem.querySelector(".context-menu .source");
+    if (websiteElement && subscription.homepage)
+      websiteElement.setAttribute("href", subscription.homepage);
+    if (sourceElement)
+      sourceElement.setAttribute("href", subscription.url);
+  }
+
+  function toggleRemoveSubscription(e)
   {
     e.preventDefault();
-    var subscriptionUrl = e.target.parentNode.getAttribute("data-access");
+    var subscriptionUrl = findParentData(e.target, "access", false);
     if (!e.target.checked)
     {
       ext.backgroundPage.sendMessage({
@@ -134,16 +200,28 @@
       addEnableSubscription(subscriptionUrl);
   }
 
+  function toggleDisableSubscription(e)
+  {
+    e.preventDefault();
+    var subscriptionUrl = findParentData(e.target, "access", false);
+    ext.backgroundPage.sendMessage(
+    {
+      type: "subscriptions.toggle",
+      keepInstalled: true,
+      url: subscriptionUrl
+    });
+  }
+
   function onAddLanguageSubscriptionClick(e)
   {
     e.preventDefault();
-    var url = this.parentNode.getAttribute("data-access");
+    var url = findParentData(this, "access", false);
     addEnableSubscription(url);
   }
 
   function onRemoveFilterClick()
   {
-    var filter = this.parentNode.getAttribute("data-access");
+    var filter = findParentData(this, "access", false);
     ext.backgroundPage.sendMessage(
     {
       type: "filters.remove",
@@ -155,7 +233,7 @@
   [
     {
       id: "recommend-list-table",
-      onClick: onToggleSubscriptionClick
+      onClick: toggleRemoveSubscription
     }
   ]);
   collections.langs = new Collection(
@@ -163,7 +241,7 @@
     {
       id: "blocking-languages-table",
       emptyText: "options_dialog_language_added_empty",
-      onClick: onToggleSubscriptionClick
+      onClick: toggleRemoveSubscription
     },
     {
       id: "blocking-languages-dialog-table",
@@ -182,14 +260,14 @@
   [
     {
       id: "acceptableads-table",
-      onClick: onToggleSubscriptionClick
+      onClick: toggleRemoveSubscription
     }
   ]);
   collections.custom = new Collection(
   [
     {
       id: "custom-list-table",
-      onClick: onToggleSubscriptionClick
+      onClick: toggleRemoveSubscription
     }
   ]);
   collections.whitelist = new Collection(
@@ -207,31 +285,39 @@
       emptyText: "options_customFilters_empty"
     }
   ]);
+  collections.filterLists = new Collection(
+  [
+    {
+      id: "all-filter-lists-table",
+      onClick: toggleDisableSubscription
+    }
+  ]);
 
-  function updateSubscription(subscription)
+  function observeSubscription(subscription)
   {
-    var subscriptionUrl = subscription.url;
-    var knownSubscription = subscriptionsMap[subscriptionUrl];
-    if (knownSubscription)
-      knownSubscription.disabled = subscription.disabled;
-    else
+    function onObjectChanged(change)
     {
-      getAcceptableAdsURL(function(acceptableAdsUrl)
+      for (var i = 0; i < change.length; i++)
       {
-        function onObjectChanged()
+        var property = change[i].name;
+        if (property == "disabled")
         {
-          var access = (subscriptionUrl || subscription.text).replace(/'/g, "\\'");
+          var access = (subscription.url || 
+                        subscription.text).replace(/'/g, "\\'");
           var elements = document.querySelectorAll("[data-access='" + access + "']");
           for (var i = 0; i < elements.length; i++)
           {
             var element = elements[i];
+            var tableId = element.parentElement ? element.parentElement.id : "";
             var control = element.querySelector(".control");
-            if (control.localName == "input")
+            if (control && control.localName == "input")
               control.checked = subscription.disabled == false;
-            if (subscriptionUrl in recommendationsMap)
+            if (subscription.url in recommendationsMap)
             {
-              var recommendation = recommendationsMap[subscriptionUrl];
-              if (recommendation.type == "ads")
+              var recommendation = recommendationsMap[subscription.url];
+              if (recommendation.type == "ads" && 
+                 (collections.langs.hasId(tableId) ||
+                 collections.allLangs.hasId(tableId)))
               {
                 if (subscription.disabled == false)
                 {
@@ -247,33 +333,60 @@
             }
           }
         }
+        else 
+        {
+          var blockingListId = collections.filterLists.details[0].id;
+          var blockingList = document.getElementById(blockingListId);
+          var listItem = blockingList.querySelector("[data-access='" +
+                          subscription.url + "']");
+          if (listItem)
+            updateBlockingList(listItem, subscription);
+        }
+      }
+    }
 
-        if (!Object.observe)
+    if (!Object.observe)
+    {
+      ["disabled", "lastDownload"].forEach(function(property)
+      {
+        subscription["$" + property] = subscription[property];
+        Object.defineProperty(subscription, property,
         {
-          // Currently only "disabled" property of subscription used for observation
-          // but with Advanced tab implementation we should also add more properties.
-          ["disabled"].forEach(function(property)
+          get: function()
           {
-            subscription["$" + property] = subscription[property];
-            Object.defineProperty(subscription, property,
-            {
-              get: function()
-              {
-                return this["$" + property];
-              },
-              set: function(value)
-              {
-                this["$" + property] = value;
-                onObjectChanged();
-              }
-            });
-          });
-        }
-        else
-        {
-          Object.observe(subscription, onObjectChanged);
-        }
+            return this["$" + property];
+          },
+          set: function(value)
+          {
+            this["$" + property] = value;
+            var change = Object.create(null);
+            change.name = property;
+            onObjectChanged([change]);
+          }
+        });
+      });
+    }
+    else
+    {
+      Object.observe(subscription, onObjectChanged);
+    }
+  }
 
+  function updateSubscription(subscription)
+  {
+    var subscriptionUrl = subscription.url;
+    var knownSubscription = subscriptionsMap[subscriptionUrl];
+    if (knownSubscription)
+    {
+      for (var property in subscription)
+        if (property != "title")
+          knownSubscription[property] = subscription[property];
+    }
+    else
+    {
+      observeSubscription(subscription);
+      getAcceptableAdsURL(function(acceptableAdsUrl)
+      {
         var collection = null;
         if (subscriptionUrl in recommendationsMap)
         {
@@ -331,7 +444,6 @@
           subscription.disabled = null;
           subscription.downloadStatus = null;
           subscription.homepage = null;
-          subscription.lastSuccess = null;
           var recommendation = Object.create(null);
           recommendation.type = element.getAttribute("type");
           var prefix = element.getAttribute("prefixes");
@@ -352,13 +464,30 @@
       });
   }
 
+  function findParentData(element, dataName, returnElement)
+  {
+    while (element)
+    {
+      if (element.hasAttribute("data-" + dataName))
+        return returnElement ? element : element.getAttribute("data-" + dataName);
+
+      element = element.parentElement;
+    }
+    return null;
+  }
+
   function onClick(e)
   {
     var element = e.target;
     while (true)
     {
       if (!element)
+      {
+        var context = document.querySelector(".context");

[Thomas Greiner, 2016/01/27 17:17:00]

Detail: This class name is not very descriptive of its effects so it'd be great
if you could call it something like "show-context" or "show-context-menu"
instead.

[saroyanm, 2016/01/28 17:00:15]

Done.

+        if (context)
+          context.classList.remove("context");
         return;

[Thomas Greiner, 2016/01/27 17:17:00]

The context menu is also supposed to go away whenever you click on an element
that has an action attached to it so it probably makes sense to move this
outside of the switch-statement to the bottom of the click handler.

e.g.

// close context menu

if (action == "open-context-menu")
{
  // open context menu unless it is the same one that we just closed above
}

[saroyanm, 2016/01/28 17:00:14]

Not really sure that I understood what you mean, but changed the implementation,
please let me know if it's still needs to be changed. 

[Thomas Greiner, 2016/01/29 17:48:09]

Here's a more detailed and fleshed-out version:

function onClick(e)
{
  ...
  var context = document.querySelector(".show-context-menu");
  if (context)
    context.classList.remove("show-context-menu");

  for (...)
  {
    switch (actions[i])
    {
      ...
      case "open-context-menu":
        var listItem = findParentData(element, "access", true);
        if (listItem != context)
          listItem.classList.add("show-context-menu");
        break;
      ...
    }
  }
  ...
}

That way you should be able to ensure that the context menu always goes away and
only comes up when clicking on a different subscription title.

[saroyanm, 2016/01/29 18:56:17]

Well, that's what I've had implemented as well after your note, beside of the
part for checking the context, because I wasn't sure whether the context needs
to be hidden after second click on the list item, but yes also added that part.

+      }
 
       if (element.hasAttribute("data-action"))
         break;
@@ -419,21 +548,43 @@
           document.body.setAttribute("data-tab",
             element.getAttribute("data-tab"));
           break;
+        case "update-all-subscriptions":
+          ext.backgroundPage.sendMessage(
+          {
+            type: "subscriptions.update"
+          });
+          break;
+        case "open-context-menu":
+          var listItem = findParentData(element, "access", true);
+          var contextMenu = listItem.querySelector(".content");

[Thomas Greiner, 2016/01/27 17:17:00]

Detail: This variable is not being used.

[saroyanm, 2016/01/28 17:00:15]

Done.

+          listItem.classList.add("context");
+          break;
+        case "update-now":
+          ext.backgroundPage.sendMessage(
+          {
+            type: "subscriptions.update",
+            url: findParentData(element, "access")

[Thomas Greiner, 2016/01/27 17:17:00]

Detail: While the last parameter is optional in such cases, you did explicitly
set it in all other cases. I don't mind either way but it should be consistent.

[saroyanm, 2016/01/28 17:00:15]

Done.

+          });
+          break;
+        case "remove-subscription":
+          ext.backgroundPage.sendMessage(
+          {
+            type: "subscriptions.remove",
+            url: findParentData(element, "access")
+          });
+          break;
+        case "close-context-menu":
+          var access = findParentData(element, "access", true);
+          if (access)
+            access.classList.remove("context");
+          break;
       }
     }
   }
 
   function onDOMLoaded()
   {
-    var recommendationTemplate = document.querySelector("#recommend-list-table template");
-    var popularText = ext.i18n.getMessage("options_popular");
-    recommendationTemplate.content.querySelector(".popular").textContent = popularText;
-    var languagesTemplate = document.querySelector("#all-lang-table template");
-    var buttonText = ext.i18n.getMessage("options_button_add");
-    languagesTemplate.content.querySelector(".button-add span").textContent = buttonText;
-
     populateLists();
-
     function onFindLanguageKeyUp()
     {
       var searchStyle = E("search-style");
@@ -705,14 +856,27 @@
     switch (action)
     {
       case "added":
+        updateSubscription(subscription);
+        updateShareLink();
+
+        var knownSubscription = subscriptionsMap[subscription.url];
+        if (knownSubscription)
+          collections.filterLists.addItems(knownSubscription);
+        else
+          collections.filterLists.addItems(subscription);
+        break;
       case "disabled":
         updateSubscription(subscription);
         updateShareLink();
         break;
+      case "lastDownload":
+        updateSubscription(subscription);
+        break;
       case "homepage":
         // TODO: NYI
         break;
       case "removed":
+        var knownSubscription = subscriptionsMap[subscription.url];
         getAcceptableAdsURL(function(acceptableAdsUrl)
         {
           if (subscription.url == acceptableAdsUrl)
@@ -722,7 +886,6 @@
           }
           else
           {
-            var knownSubscription = subscriptionsMap[subscription.url];
             if (subscription.url in recommendationsMap)
               knownSubscription.disabled = true;
             else
@@ -732,6 +895,7 @@
             }
           }
           updateShareLink();
+          collections.filterLists.removeItem(knownSubscription);
         });
         break;
       case "title":
@@ -821,7 +985,7 @@
   ext.backgroundPage.sendMessage(
   {
     type: "subscriptions.listen",
-    filter: ["added", "disabled", "homepage", "removed", "title"]
+    filter: ["added", "disabled", "homepage", "lastDownload", "removed", "title"]
   });
 
   window.addEventListener("DOMContentLoaded", onDOMLoaded, false);