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)
@@ skipping 15 matching lines @@
       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());
-
-        updateBlockingList(listItem, item);

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

This doesn't belong into Collection since it's specific to one table. Therefore
I'd suggest moving it into the observer function.

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

We are calling this method on observer as well, This method is used to update
the date/time and links of the filterList item, but in general it should be part
of updateItem method, I've added that implementation to updateItem method, hope
it's more clear now.

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

This code is only relevant for subscriptions. However, Collection also manages
filters which don't specify source, homepage, etc. Therefore it should be
separate from Collection.

However, I'm fine with how it looks right now to centralize logic.

+
+        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())
+        {
           table.insertBefore(listItem, 
-                            table.childNodes[this.items.indexOf(item)]);

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

Detail: Please use braces around it if you occupy more than a single line.

Note that we usually indent a continuing line with two additional double-spaces.

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

Done.

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

Detail: What I meant with "indent using double-spaces" was to use four spaces
instead of the usual two when starting a new line:

abcdef(ghi
....jklmno)

instead of

abcdef(ghi
      ..jklmno)

Because that way we can write longer lines while still making clear that the
second line is part of the previous statement and not another independent
statement.

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

Got it, sorry for that, I was thinking that we need to align the second line
with the related expression on line above for readability.

+              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 getListElement = this._createElementQuery(item);
     for (var i = 0; i < this.details.length; i++)
     {
       var table = E(this.details[i].id);
       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/25 15:40:33]

The above two variables are no longer used.

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

Done.

       var element = table.firstChild;
       while (element)
       {
-        if ((element.tagName == "LI" && !element.classList.contains("static")) ||
-            element.nodeType == 3)
+        if (element.tagName == "LI" && !element.classList.contains("static"))
           table.removeChild(element);
-        element = element.nextSibling
+        element = element.nextElementSibling;
       }
 
       this._setEmpty(table, this.details[i].emptyText);
     }
   };
 
-  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 timedate = i18n_timeDateStrings(subscription.lastDownload * 1000);
-        var date = new Date(subscription.lastDownload * 1000);
-        dateElement.textContent = date.getFullYear() + "-" +
-                                    date.getMonth()+1 + "-" + date.getDate();
-        timeElement.textContent = date.getHours() + ":" + date.getMinutes();
-      }
-    }
-    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 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 = getParentAccessElement(e.target).
-                          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 toggleDisableSubscription(e)
   {
     e.preventDefault();
-    var subscriptionUrl = e.target.parentNode.getAttribute("data-access");
+    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",
       text: filter
     });
   }
 
   collections.popular = new Collection(
   [
     {
@@ skipping 43 matching lines @@
       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",
+      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];
-              if (recommendation.type == "ads" && 
-                 (collections.langs.hasId(tableId) ||
-                 collections.allLangs.hasId(tableId)))

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

I've been thinking about this again. Wouldn't you agree that all of that should
be handled by the Collection?

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

I assume you are referring to the "Collection.prototype.updateItem" from
https://codereview.adblockplus.org/29332808/
While the referencing review is almost done, I think make sense to rebase
current review after pushing the changes, I'll check to finish with that review
quick.

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

I guess you're right. We can leave it here for now and clean it up afterwards by
moving the relevant parts into Collection.

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

Done, hope now it's more clear.

-              {
-                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 + "']");
-          if (listItem)
-            updateBlockingList(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)
     {
-      for (var param in subscription)

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

Detail: It's a property, not a parameter.

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

Done.

-        if (param != "title")

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

Why did you specifically exclude the "title" property.

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

To use titles specified in Recommendation, so the subscriptions in
subscriptionsMap will always have the initial title, anyway we need to update
the title as soon "title" action is triggered, otherwise we can't update the
title when lastDownload is triggered as in current situation.

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

Right, forgot about that case.

It's actually trickier than that because "subscriptions.title" can be triggered
at any point (e.g. subscription updates in the background and includes a
different title than before).
So as soon as we implement the updating of titles we should somehow make sure
that the titles from recommended subscriptions stay unchanged.

Out of the scope of this review, however, so let's tackle this separately.

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

Acknowledged.

-          knownSubscription[param] = subscription[param];
+      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 49 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 getParentAccessElement(element)
-  {
-    while (!element.getAttribute("data-access"))
-      element = element.parentNode;

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

This may cause an exception if there is no parent element that has that
attribute.

In the upcoming review for #2409 I was planning to introduce the following
function which you could use here as well so feel free to implement it here
already or improve this function based on it.

function findData(element, dataName, returnElement)
{
  while (element)
  {
    if (element.hasAttribute("data-" + dataName))
      return returnElement ? element : element.getAttribute("data-" + dataName);

    element = element.parentElement;
  }
  return null;
}

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

Like it, thanks.

-
-    return element;
+  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)
-      {
-        var context = document.querySelector(".context");
-        if (context)
-          context.classList.remove("context");
         return;
-      }
 
       if (element.hasAttribute("data-action"))
         break;
 
       element = element.parentElement;
     }
 
     var actions = element.getAttribute("data-action").split(",");
     for (var i = 0; i < actions.length; i++)
     {
@@ skipping 47 matching lines @@
           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 = getParentAccessElement(element);
-          var contextMenu = listItem.querySelector(".content");
-          listItem.classList.add("context");
-          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(element).getAttribute("data-access")
+            url: findParentData(element, "access", false)
           });
-          getParentAccessElement(element).classList.remove("context");

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

Why not just add a "close-context-menu" action to the list elements directly
instead of introducing "website" and "source" actions and manually add it to
other ones?

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

Done.

-          break;
-        case "website":
-          getParentAccessElement(element).classList.remove("context");
-          break;
-        case "source":
-          getParentAccessElement(element).classList.remove("context");
-          break;
-        case "delete":

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

Detail: This message name doesn't describe what it does (i.e. what's deleted).
Therefore I'd suggest renaming it to "remove-subscription".

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

Done.

+          break;
+        case "remove-subscription":
           ext.backgroundPage.sendMessage(
           {
             type: "subscriptions.remove",
-            url: getParentAccessElement(element).getAttribute("data-access")
+            url: findParentData(element, "access", false)
           });
-          getParentAccessElement(element).classList.remove("context");
           break;
       }
     }
   }
 
   function onDOMLoaded()
   {
     populateLists();
     function onFindLanguageKeyUp()
     {
@@ skipping 34 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 "lastDownload":
         updateSubscription(subscription);
         break;
       case "homepage":
         // TODO: NYI
@@ skipping 11 matching lines @@
           {
             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);

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

Detail: Semantically, updating the UI should happen after the storage was
modified so I'd suggest moving this code one line up.

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

Done.

         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", "homepage", "lastDownload", "removed", "title", "updated"]

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

Detail: You removed the handling for "updated" so I guess you can remove it from
here.

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

Done.

+    filter: ["added", "disabled", "homepage", "lastDownload", "removed", "title"]
   });
 
   window.addEventListener("DOMContentLoaded", onDOMLoaded, false);
 })();