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-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.
  *
  * 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)
+    {
+      return container.querySelector("[data-access='" + access + "']");
+    };
+  };
 
   Collection.prototype.addItems = function() 
   {
     var length = Array.prototype.push.apply(this.items, arguments);
     if (length == 0)
       return;
 
     this.items.sort(function(a, b)
     {
       var aValue = (a.title || a.text || a.url).toLowerCase();
       var bValue = (b.title || b.text || b.url).toLowerCase();
       return aValue.localeCompare(bValue);
     });
 
     for (var j = 0; j < this.details.length; j++) 
     {
       var table = E(this.details[j].id);
       var template = table.querySelector("template");
       for (var i = 0; i < arguments.length; i++) 
       {
         var item = arguments[i];
-        var text = item.title || item.url || item.text;
         var listItem = document.createElement("li");
         listItem.appendChild(document.importNode(template.content, true));
         listItem.setAttribute("data-access", item.url || item.text);
-        listItem.querySelector(".display").textContent = text;
-        if (text)
-          listItem.setAttribute("data-search", text.toLowerCase());
-
-        updateTimeDate(listItem, item);
+
+        var labelId = "label-" + (++maxLabelId);
+        listItem.querySelector(".display").setAttribute("id", labelId);
         var control = listItem.querySelector(".control");
         if (control)
         {
+          // We use aria-labelledby to avoid triggering the control when
+          // interacting with the label
+          control.setAttribute("aria-labelledby", labelId);
           control.addEventListener("click", this.details[j].onClick, false);
-          control.checked = item.disabled == false;
         }
 
         this._setEmpty(table, null);
         if (table.hasChildNodes())
         {
-          var index = this.items.indexOf(item);
-          if (table.firstChild.classList.contains("head"))
-            index++;
-
-          table.insertBefore(listItem, table.childNodes[index]);
+          table.insertBefore(listItem, 
+              table.childNodes[this.items.indexOf(item)]);
         }
         else
           table.appendChild(listItem);
+        this.updateItem(item);
       }
     }
     return length;
   };
 
   Collection.prototype.removeItem = function(item)
   {
     var index = this.items.indexOf(item);
     if (index == -1)
       return;
 
     this.items.splice(index, 1);
-    var access = (item.url || item.text).replace(/'/g, "\\'");

[Thomas Greiner, 2016/01/19 11:27:28]

I notice the issues with accessing elements by their index so I understand the
need for using this accessor. However, please extract this into its own function
so that we don't have to implement this logic wherever we need to determine the
access value of an item.

For example:

Collection.prototype._createElementQuery = function(item)
{
  var access = (item.url || item.text).replace(/'/g, "\\'");
  return function(container)
  {
    return container.querySelector("[data-access='" + access + "']");
  };
};

[saroyanm, 2016/01/22 09:55:08]

Done, but wouldn't it be simpler to just have a private method that takes two
parameters and return the result ? 
Like: 
Collection.prototype._getListElement = function(item, container)
{
  return container.querySelector("[data-access='" + (item.url ||
item.text).replace(/'/g, "\\'") + "']");
};

What is the benefit of returning the function ?
Maybe I don't understand your approach.

[Thomas Greiner, 2016/01/25 15:40:28]

By returning a function we only create the "access" string once instead of
everytime we call the function. It's not a big difference overall but my code
was only a suggestion anyway so feel free to implement it either way.

+    var getListElement = this._createElementQuery(item);
     for (var i = 0; i < this.details.length; i++)
     {
       var table = E(this.details[i].id);
-      var element = table.querySelector("[data-access='" + access + "']");
+      var element = getListElement(table);
+
+      // Element gets removed so make sure to handle focus appropriately
+      var control = element.querySelector(".control");
+      if (control && control == document.activeElement)
+      {
+        if (!focusNextElement(element.parentElement, control))
+        {
+          // Fall back to next focusable element within same tab or dialog
+          var focusableElement = element.parentElement;
+          while (focusableElement)
+          {
+            if (focusableElement.classList.contains("tab-content")
+                || focusableElement.classList.contains("dialog-content"))
+              break;
+
+            focusableElement = focusableElement.parentElement;
+          }
+          focusNextElement(focusableElement || document, control);
+        }
+      }
+
       element.parentElement.removeChild(element);
       if (this.items.length == 0)
         this._setEmpty(table, this.details[i].emptyText);
     }
   };
 
+  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 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)
+      {
+        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);
+    }
+  };
+
   Collection.prototype.clearAll = function()
   {
     this.items = [];
     for (var i = 0; i < this.details.length; i++)
     {
       var table = E(this.details[i].id);
-      var template = table.querySelector("template");
-      var staticElements = [];

[Thomas Greiner, 2016/01/19 11:27:29]

This logic is quite complicated. Assuming that you merely want to know at which
position elements should be inserted you could go with the following approach:

1. Create a document fragment: document.createDocumentFragment()
2. Append list elements to the document fragment: fragment.appendChild()
3. Insert the document fragment at the appropriate position:
template.parentNode.insertBefore(fragment, template)

Obviously, that means that you won't be able to clear it with `table.innerHTML =
""` anymore and that you therefore have to remove each element individually but
that should be fine since we're only calling this method in `populateLists()`.

[saroyanm, 2016/01/22 09:55:10]

Currently we are not using createDocumentFragment to add List item, I've updated
current removal implementation to only remove the dynamic list items, I also
need to remove text nodes otherwise it's becoming messy to always determine the
type of the node.

[Thomas Greiner, 2016/01/25 15:40:27]

You could use `Element.nextElementSibling` to avoid other kinds of nodes. So
checking for the "static" class and `<template>` elements should be sufficient.

Alternatively, you could use `table.children` to get a list of all child
elements.

In any case you need to be careful since you're removing the element from the
DOM. The safest approach would be to use
`Array.prototype.slice.call(table.children)` and loop over the items in the
array.

[saroyanm, 2016/01/26 18:36:15]

I assume while you didn't commented under updated code, that mean you are okey
with the implementation in patch set "#2"

[Thomas Greiner, 2016/01/27 17:16:56]

Thanks for reminding me. I wrote down some parts that I still wanted to comment
on but forgot about those so I added them now.

       var element = table.firstChild;
       while (element)
       {
-        if (element.tagName == "TEMPLATE" || 
-            (element.classList && element.classList.contains("static")))
-          staticElements.push(element);
-        element = element.nextSibling
+        if (element.tagName == "LI" && !element.classList.contains("static"))
+          table.removeChild(element);
+        element = element.nextElementSibling;
       }
 
-      table.innerHTML = "";
-      for (var j = 0; j < staticElements.length; j++)
-        table.appendChild(staticElements[j]);
-
       this._setEmpty(table, this.details[i].emptyText);
     }
   };
 
-  Collection.prototype.getTableIds = function() 

[Thomas Greiner, 2016/01/19 11:27:29]

This information is an implementation detail and should therefore stay within
the Collection.

[saroyanm, 2016/01/22 09:55:09]

updated the method to check if the collection has ID.

-  {
-    var ids = [];
-    for (var i = 0; i < this.details.length; i++)
-      ids.push(this.details[i].id)
-    
-    return ids;
-  };
-
-  function updateTimeDate(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 timedate = i18n_timeDateStrings(subscription.lastDownload * 1000);

[Thomas Greiner, 2016/01/19 11:27:29]

This function returns the localized date-time string which is not what you're
looking for because it will differ (e.g. "1970/30/1" or "1970/1/30" or
"1.1.1970") and thereby cause unexpected results and forces you to do all those
modificiations and checks below.

Therefore please don't use this function and instead implement it yourself. Feel
free to add it to "i18n.js" but note that we cannot remove
"i18n_timeDateStrings()" yet because it's still being used in the current
options page.

[saroyanm, 2016/01/22 09:55:10]

I see, I was trying to make it consistent with old code, didn't noticed the
localization part thanks, not really sure if we need separate method for this in
i18n, because it has nothing to do with localization currently, but I have
implemented the logic in current function, I think should be enough for now, but
let me know if you expect this logic to do something else.

[Thomas Greiner, 2016/01/25 15:40:28]

The mere formatting of the string should be generic enough for inclusion in
"i18n.js", all the other parts should stay here.

e.g.

var dateTime = i18n_formatDateTime(date);
dateElement.textContent = dateTime[0];
timeElement.textContent = dateTime[1];

Eventually, we might need something even more generic (e.g.
`i18n_formatDateTime(date, "YYYY-MM-DD hh:mm")`) but that'd probably be too much
effort for this specific use case.

Anyway, please note that the code you implemented formats the date as
"2016-111-1 1:1" instead of the expected "2016-12-01 01:01".

[saroyanm, 2016/01/26 18:36:15]

Done.

-        if (timedate[1])
-          dateElement.textContent = timedate[1].split("/").reverse().join("-");
-        else
-        {
-          var today = new Date();
-          dateElement.textContent = today.getFullYear() + "-" +
-                                    today.getMonth()+1 + "-" + today.getDate();
-        }
-        var time = timedate[0].split(":");
-        time[2] = time[2].split(" ")[1];
-        if (time[2] == "AM" && time[0] == "12")
-          time[0] = 0;
-        else if (time[2] == "PM")
-          time[0] = parseInt(time[0]) + 12;
-        
-        time.pop();
-        timeElement.textContent = time.join(":");
-      }
-    }
-  }
-
-  function onToggleSubscriptionClick(e)

[Thomas Greiner, 2016/01/19 11:27:29]

I'd suggest renaming this function to "toggleRemoveSubscription" and the one
below to "toggleDisableSubscription" to clarify how their behavior differs.

[saroyanm, 2016/01/22 09:55:08]

Done.

+  function focusNextElement(container, currentElement)
+  {
+    var focusables = container.querySelectorAll("a, button, input, .control");
+    focusables = Array.prototype.slice.call(focusables);
+    var index = focusables.indexOf(currentElement);
+    index += (index == focusables.length - 1) ? -1 : 1;
+
+    var nextElement = focusables[index];
+    if (!nextElement)
+      return false;
+
+    nextElement.focus();
+    return true;
+  }
+
+  function toggleRemoveSubscription(e)
   {
     e.preventDefault();
-    var subscriptionUrl = e.target.parentNode.getAttribute("data-access");
-    if (!e.target.checked)
+    var subscriptionUrl = findParentData(e.target, "access", false);
+    if (e.target.getAttribute("aria-checked") == "true")
     {
       ext.backgroundPage.sendMessage({
         type: "subscriptions.remove",
         url: subscriptionUrl
       });
     }
     else
       addEnableSubscription(subscriptionUrl);
   }
 
-  function onToggleSubscriptionStateClick(e)
+  function toggleDisableSubscription(e)
   {
     e.preventDefault();
-    var subscriptionUrl = e.target.parentNode.getAttribute("data-access");

[Thomas Greiner, 2016/01/19 11:27:29]

What about reusing `getParentAccessElement()` since you already created it?

[saroyanm, 2016/01/22 09:55:10]

Done.

[Thomas Greiner, 2016/01/25 15:40:28]

I don't see that you changed anything here.

[saroyanm, 2016/01/26 18:36:15]

Now for sure, thanks.

+    var subscriptionUrl = findParentData(e.target, "access", false);
     ext.backgroundPage.sendMessage(
     {
-      type: "subscriptions.toggleState",
+      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",
       text: filter
     });
   }
 
   collections.popular = new Collection(
   [
     {
       id: "recommend-list-table",
-      onClick: onToggleSubscriptionClick
+      onClick: toggleRemoveSubscription
     }
   ]);
   collections.langs = new Collection(
   [
     {
       id: "blocking-languages-table",
       emptyText: "options_dialog_language_added_empty",
-      onClick: onToggleSubscriptionClick
+      onClick: toggleRemoveSubscription
     },
     {
       id: "blocking-languages-dialog-table",
       emptyText: "options_dialog_language_added_empty"
     }
   ]);
   collections.allLangs = new Collection(
   [
     {
       id: "all-lang-table",
       emptyText: "options_dialog_language_other_empty",
       onClick: onAddLanguageSubscriptionClick
     }
   ]);
   collections.acceptableAds = new Collection(
   [
     {
       id: "acceptableads-table",
-      onClick: onToggleSubscriptionClick
+      onClick: toggleRemoveSubscription
     }
   ]);
   collections.custom = new Collection(
   [
     {
       id: "custom-list-table",
-      onClick: onToggleSubscriptionClick
+      onClick: toggleRemoveSubscription
     }
   ]);
   collections.whitelist = new Collection(
   [
     {
       id: "whitelisting-table",
       emptyText: "options_whitelisted_empty",
       onClick: onRemoveFilterClick
     }
   ]);
   collections.customFilters = new Collection(
   [
     {
       id: "custom-filters-table",
       emptyText: "options_customFilters_empty"
     }
   ]);
-  collections.blockingLists = new Collection(
+  collections.filterLists = new Collection(
   [
     {
-      id: "blocking-lists-table",
-      onClick: onToggleSubscriptionStateClick
+      id: "all-filter-lists-table",
+      onClick: toggleDisableSubscription
     }
   ]);
 
   function observeSubscription(subscription)
   {
     function onObjectChanged(change)
     {
       for (var i = 0; i < change.length; i++)
       {
-        var property = change[i].name;
-        if (property == "disabled")
-        {
-          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 && control.localName == "input")
-              control.checked = subscription.disabled == false;
-            if (subscription.url in recommendationsMap)
+        if (change[i].name == "disabled")
+        {
+          var recommendation = recommendationsMap[subscription.url];
+          if (recommendation && recommendation.type == "ads")
+          {
+            if (subscription.disabled == false)
             {
-              var recommendation = recommendationsMap[subscription.url];
-              var langids = collections.langs.getTableIds();
-              var allLangIds = collections.allLangs.getTableIds();
-              if (recommendation.type == "ads" && 
-                  langids.concat(allLangIds).indexOf(tableId) != -1)
-              {
-                if (subscription.disabled == false)
-                {
-                  collections.allLangs.removeItem(subscription);
-                  collections.langs.addItems(subscription);
-                }
-                else
-                {
-                  collections.allLangs.addItems(subscription);
-                  collections.langs.removeItem(subscription);
-                }
-              }
+              collections.allLangs.removeItem(subscription);
+              collections.langs.addItems(subscription);
             }
+            else
+            {
+              collections.allLangs.addItems(subscription);
+              collections.langs.removeItem(subscription);
+            }
           }
         }
-        else 
-        {
-          var blockingListId = collections.blockingLists.details[0].id;
-          var blockingList = document.getElementById(blockingListId);
-          var listItem = blockingList.querySelector("[data-access='" +
-                          subscription.url + "']");
-          updateTimeDate(listItem, subscription);
-        }
+        for (var i in collections)
+          collections[i].updateItem(subscription);
       }
     }
 
     if (!Object.observe)
     {
       ["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);
     }
   }
 
   function updateSubscription(subscription)
   {
     var subscriptionUrl = subscription.url;
     var knownSubscription = subscriptionsMap[subscriptionUrl];
     if (knownSubscription)
     {
-      knownSubscription.disabled = subscription.disabled;
-      knownSubscription.lastDownload = subscription.lastDownload;
+      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)
         {
           var recommendation = recommendationsMap[subscriptionUrl];
@@ skipping 43 matching lines @@
         var elements = doc.documentElement.getElementsByTagName("subscription");
         for (var i = 0; i < elements.length; i++)
         {
           var element = elements[i];
           var subscription = Object.create(null);
           subscription.title = element.getAttribute("title");
           subscription.url = element.getAttribute("url");
           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");
           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)
+  {
+    while (element)
+    {
+      if (element.hasAttribute("data-" + dataName))
+        return returnElement ? element : element.getAttribute("data-" + dataName);
+
+      element = element.parentElement;
+    }
+    return null;
+  }
+
   function onClick(e)
   {
+    var context = document.querySelector(".show-context-menu");
+    if (context)
+      context.classList.remove("show-context-menu");
+
     var element = e.target;
     while (true)
     {
       if (!element)
         return;
 
       if (element.hasAttribute("data-action"))
         break;
 
       element = element.parentElement;
-    }
-
-    function getParentAccessElement()

[Thomas Greiner, 2016/01/19 11:27:30]

Please avoid declaring functions inside other functions or otherwise they're
being created everytime when the function is called.

[saroyanm, 2016/01/22 09:55:09]

Good point.

-    {
-      var elementCopy = element;
-      while (!elementCopy.dataset.access)

[Thomas Greiner, 2016/01/19 11:27:29]

We can't use `Element.dataset` due to older Safari versions so please use
`Element.getAttribute()`.

[saroyanm, 2016/01/22 09:55:09]

Right, done.

-        elementCopy = elementCopy.parentNode;
-
-      return elementCopy;
     }
 
     var actions = element.getAttribute("data-action").split(",");
     for (var i = 0; i < actions.length; i++)
     {
       switch (actions[i])
       {
         case "add-domain-exception":
           addWhitelistedDomain();
           break;
@@ skipping 38 matching lines @@
           });
           E("custom-filters").classList.remove("mode-edit");
           break;
         case "switch-tab":
           document.body.setAttribute("data-tab",
             element.getAttribute("data-tab"));
           break;
         case "update-all-subscriptions":
           ext.backgroundPage.sendMessage(
           {
-            type: "subscriptions.updateAll"
+            type: "subscriptions.update"
           });
           break;
         case "open-context-menu":

[Thomas Greiner, 2016/01/19 11:27:29]

We should improve this mechanism because it requires quite a bit of extra work
to get the UX right.

My suggestion would be to listen to clicks:
- click on subscription title:
  - if context menu hidden: show context menu
  - else: hide context menu
- click outside of context menu: hide context menu
- click on action item in context menu: hide context menu
- click somewhere else in context menu: ignore

That would also be consistent with other context menus (e.g. right-click context
menu).

[saroyanm, 2016/01/22 09:55:09]

Done, but bit hacky, not sure if it's what you mean.

[Thomas Greiner, 2016/01/27 17:16:56]

Better but what about replacing it with "toggle-context-menu"? Because right now
it doesn't close again when clicking on the link or arrow again.

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

Done.

-          var listItem = getParentAccessElement();
-          var contextMenu = listItem.querySelector(".content");
-          listItem.classList.add("context");
-
-          function mouseover()
-          {
-            contextMenu.addEventListener("mouseout", mouseout, false);
-            contextMenu.removeEventListener("mouseover", mouseover);
-          }
-          function mouseout(event)
-          {
-            if (event.target.parentElement != contextMenu)
-            {
-              var relatedTarget = event.relatedTarget;
-              if (relatedTarget.parentNode == this || relatedTarget == this)
-                 return;
-              listItem.classList.remove("context");
-              contextMenu.removeEventListener("mouseout", mouseout);
-            }
-          }
-
-          contextMenu.addEventListener("mouseover", mouseover, false);
-          break;
-        case "update-now":
+          var listItem = findParentData(element, "access", true);
+          if (listItem != context)
+            listItem.classList.add("show-context-menu");
+          break;
+        case "update-subscription":
           ext.backgroundPage.sendMessage(
           {
             type: "subscriptions.update",
-            url: getParentAccessElement().dataset.access
+            url: findParentData(element, "access", false)
           });
           break;
-        case "website":
+        case "remove-subscription":
           ext.backgroundPage.sendMessage(
           {
-            type: "subscriptions.website",
-            url: getParentAccessElement().dataset.access
+            type: "subscriptions.remove",
+            url: findParentData(element, "access", false)
           });
           break;
-        case "source":
-          window.open(getParentAccessElement().dataset.access);
-          break;
-        case "delete":
-          ext.backgroundPage.sendMessage(
-          {
-            type: "subscriptions.remove",
-            url: getParentAccessElement().dataset.access
-          });
-          break;
       }
     }
   }
 
   function onDOMLoaded()
   {
-    function updateTemplate(template, selector, messageId)

[Thomas Greiner, 2016/01/19 11:27:29]

This functionality belongs into "i18n.js". If elements inside templates are not
retrieved on selecting `[class^="i18n_"]` we need to add that it should also
search for those within `<template>` elements.

[saroyanm, 2016/01/22 09:55:08]

Done.

-    {
-      template.content.querySelector(selector).
-              textContent = ext.i18n.getMessage(messageId);
-    }
-
-    var template = document.querySelector("#recommend-list-table template");
-    updateTemplate(template, ".popular", "options_popular");
-
-    template = document.querySelector("#all-lang-table template");
-    updateTemplate(template, ".button-add span", "options_button_add");
-
-    template = document.querySelector("#blocking-lists-table template");
-    updateTemplate(template, ".update-now", "options_blockingList_update_now");
-    updateTemplate(template, ".website", "options_blockingList_website");
-    updateTemplate(template, ".source", "options_blockingList_source");
-    updateTemplate(template, ".delete", "options_blockingList_delete");
-
     populateLists();
-
     function onFindLanguageKeyUp()
     {
       var searchStyle = E("search-style");
       if (!this.value)
         searchStyle.innerHTML = "";
       else
         searchStyle.innerHTML = "#all-lang-table li:not([data-search*=\"" + this.value.toLowerCase() + "\"]) { display: none; }";
     }
 
     function getKey(e)
@@ skipping 26 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 90 matching lines @@
   function onSubscriptionMessage(action, subscription)
   {
     switch (action)
     {
       case "added":
         updateSubscription(subscription);
         updateShareLink();
 
         var knownSubscription = subscriptionsMap[subscription.url];
         if (knownSubscription)
-          collections.blockingLists.addItems(knownSubscription);
+          collections.filterLists.addItems(knownSubscription);
         else
-          collections.blockingLists.addItems(subscription);
+          collections.filterLists.addItems(subscription);
         break;
       case "disabled":
         updateSubscription(subscription);
         updateShareLink();
         break;
-      case "updated":
+      case "lastDownload":
         updateSubscription(subscription);

[Thomas Greiner, 2016/01/19 11:27:29]

That's not what "updated" is meant to notify you of. In FilterNotifier,
"subscription.updated" is triggered whenever filters in the subscription change.
Modifications to the subscription itself have their own events:
- disabled
- downloadStatus
- homepage
- lastDownload
- title

[saroyanm, 2016/01/22 09:55:10]

Ahh right, thanks for pointing that out. Done.

         break;
       case "homepage":
         // TODO: NYI
         break;
       case "removed":
         var knownSubscription = subscriptionsMap[subscription.url];
         getAcceptableAdsURL(function(acceptableAdsUrl)
         {
           if (subscription.url == acceptableAdsUrl)
           {
             subscription.disabled = true;
             updateSubscription(subscription);
           }
           else
           {
             if (subscription.url in recommendationsMap)
               knownSubscription.disabled = true;
             else
             {
               collections.custom.removeItem(knownSubscription);
               delete subscriptionsMap[subscription.url];
             }
           }
           updateShareLink();
+          collections.filterLists.removeItem(knownSubscription);
         });
-        collections.blockingLists.removeItem(knownSubscription);
         break;
       case "title":
         // TODO: NYI
         break;
     }
   }
 
   function onShareLinkClick(e)
   {
     e.preventDefault();
@@ skipping 68 matching lines @@
     filter: ["addSubscription", "error"]
   });
   ext.backgroundPage.sendMessage(
   {
     type: "filters.listen",
     filter: ["added", "loaded", "removed"]
   });
   ext.backgroundPage.sendMessage(
   {
     type: "subscriptions.listen",
-    filter: ["added", "disabled", "updated", "homepage", "removed", "title"]
+    filter: ["added", "disabled", "homepage", "lastDownload", "removed", "title"]
   });
 
   window.addEventListener("DOMContentLoaded", onDOMLoaded, false);
 })();