issue 3741 - Add "remove" option to list items in new options page

new-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 filtersMap = Object.create(null);
   var collections = Object.create(null);
   var acceptableAdsUrl = null;
-  var maxLabelId = 0;
+  var maxItemId = 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)
@@ skipping 57 matching lines @@
     }.bind(this));
 
     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 listItem = document.createElement("li");
+        var itemId = "item-" + (++maxItemId);
         listItem.appendChild(document.importNode(template.content, true));
         listItem.setAttribute("data-access", item.url || item.text);
+        listItem.setAttribute("id", itemId);
+        listItem.setAttribute("role", "section");
 
-        var labelId = "label-" + (++maxLabelId);
         var label = listItem.querySelector(".display");
-        label.setAttribute("id", labelId);
+        label.setAttribute("for", itemId);
         if (item.recommended && label.hasAttribute("data-tooltip"))
         {
           var tooltipId = label.getAttribute("data-tooltip");
           tooltipId = tooltipId.replace("%value%", item.recommended);
           label.setAttribute("data-tooltip", tooltipId);
         }
 
-        var control = listItem.querySelector(".control");
-        if (control)
+        var controls = listItem.querySelectorAll(".control");
+        for (var k = 0; k < controls.length; k++)
         {
-          control.setAttribute("aria-labelledby", labelId);
-          control.addEventListener("click", this.details[j].onClick, false);
-
-          var role = control.getAttribute("role");
-          if (role == "checkbox" && !label.hasAttribute("data-action"))
-          {
-            var controlId = "control-" + maxLabelId;
-            control.setAttribute("id", controlId);
-            label.setAttribute("for", controlId);
-          }
+          var titleValue = getMessage(controls[k].getAttribute("title"));

[Thomas Greiner, 2016/06/22 15:49:46]

Detail: Please keep this in the if-statement as it was before because we don't
need to call `getMessage()` if the control doesn't have a "title" attribute.

[saroyanm, 2016/06/23 15:04:23]

Done.

+          if (controls[k].hasAttribute("title"))
+            controls[k].setAttribute("title", titleValue)
         }
 
         this._setEmpty(table, null);
         if (table.hasChildNodes())
         {
           table.insertBefore(listItem,
               table.childNodes[this.items.indexOf(item)]);
         }
         else
           table.appendChild(listItem);
@@ skipping 47 matching lines @@
     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 = this._getItemTitle(item, i);
       element.querySelector(".display").textContent = title;
-      if (title)
+      element.setAttribute("aria-label", title);

[Thomas Greiner, 2016/06/22 15:49:46]

Detail: Was this change intentional? If so, why is it necessary to update this
attribute? It's not being set when creating the list item.

[saroyanm, 2016/06/23 15:04:23]

Yes this is how we are marking the label of the section, I did remove this in
previous patch because this were causing the duplication, so I brought it back
and works okey when I'm using screen-reader, otherwise it's not being obvious
what the control element applies to.

[Thomas Greiner, 2016/06/24 15:22:10]

But why is it necessary since you already use `<label for="...">` to define the
label for this element?

And why are you setting it in `updateItem()` but not in `addItems()`?

[saroyanm, 2016/06/24 15:54:51]

When I only use <label for="..."> and focusing the control element using screen
reader I can't hear anything regarding the list item itself.
The title can be changed, so we need to update the element during the update
call.

+      if (this.details[i].searchable)
         element.setAttribute("data-search", title.toLowerCase());
       var control = element.querySelector(".control[role='checkbox']");
       if (control)
       {
         control.setAttribute("aria-checked", item.disabled == false);
-        if (item.url == acceptableAdsUrl && this.details[i].onClick ==
-                                            toggleDisableSubscription)
+        if (item.url == acceptableAdsUrl && this == collections.filterLists)
           control.setAttribute("disabled", true);
       }
 
       var dateElement = element.querySelector(".date");
       var timeElement = element.querySelector(".time");
       if (dateElement && timeElement)
       {
         var message = element.querySelector(".message");
         if (item.isDownloading)
         {
@@ skipping 60 matching lines @@
     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 = 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 = findParentData(e.target, "access", false);
-    ext.backgroundPage.sendMessage(
-    {
-      type: "subscriptions.toggle",
-      keepInstalled: true,
-      url: subscriptionUrl
-    });
-  }
-
-  function onAddLanguageSubscriptionClick(e)
-  {
-    e.preventDefault();
-    var url = findParentData(this, "access", false);
-    addEnableSubscription(url);
-  }
-
-  function onRemoveFilterClick()
-  {
-    var filter = findParentData(this, "access", false);
-    ext.backgroundPage.sendMessage(
-    {
-      type: "filters.remove",
-      text: filter
-    });
-  }
-
   collections.popular = new Collection(
   [
     {
-      id: "recommend-list-table",
-      onClick: toggleRemoveSubscription
+      id: "recommend-list-table"
     }
   ]);
   collections.langs = new Collection(
   [
     {
       id: "blocking-languages-table",
-      emptyText: "options_dialog_language_added_empty",
-      onClick: toggleRemoveSubscription
+      emptyText: "options_dialog_language_added_empty"
     },
     {
       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
+      searchable: true
     }
   ]);
   collections.acceptableAds = new Collection(
   [
     {
-      id: "acceptableads-table",
-      onClick: toggleRemoveSubscription
+      id: "acceptableads-table"
     }
   ]);
   collections.custom = new Collection(
   [
     {
-      id: "custom-list-table",
-      onClick: toggleRemoveSubscription
+      id: "custom-list-table"
     }
   ]);
   collections.whitelist = new Collection(
   [
     {
       id: "whitelisting-table",
-      emptyText: "options_whitelisted_empty",
-      onClick: onRemoveFilterClick
+      emptyText: "options_whitelisted_empty"
     }
   ]);
   collections.customFilters = new Collection(
   [
     {
       id: "custom-filters-table",
       emptyText: "options_customFilters_empty"
     }
   ]);
   collections.filterLists = new Collection(
   [
     {
       id: "all-filter-lists-table",
-      onClick: toggleDisableSubscription,
       useOriginalTitle: true
     }
   ]);
 
   function updateLanguageCollections(subscription)
   {
     if (subscription.recommended == "ads")
     {
       if (subscription.disabled)
       {
@@ skipping 226 matching lines @@
             url: findParentData(element, "access", false)
           });
           break;
         case "remove-subscription":
           ext.backgroundPage.sendMessage(
           {
             type: "subscriptions.remove",
             url: findParentData(element, "access", false)
           });
           break;
+        case "toggle-remove-subscription":
+          var subscriptionUrl = findParentData(element, "access", false);
+          if (element.getAttribute("aria-checked") == "true")
+          {
+            ext.backgroundPage.sendMessage({
+              type: "subscriptions.remove",
+              url: subscriptionUrl
+            });
+          }
+          else
+            addEnableSubscription(subscriptionUrl);
+          break;
+        case "toggle-disable-subscription":
+          ext.backgroundPage.sendMessage(
+          {
+            type: "subscriptions.toggle",
+            keepInstalled: true,
+            url: findParentData(element, "access", false)
+          });
+          break;
+        case "add-language-subscription":
+          addEnableSubscription(findParentData(element, "access", false));
+          break;
+        case "remove-filter":
+          ext.backgroundPage.sendMessage(
+          {
+            type: "filters.remove",
+            text: findParentData(element, "access", false)
+          });
+          break;
       }
     }
   }
 
   function onDOMLoaded()
   {
     populateLists();
     function onFindLanguageKeyUp()
     {
       var searchStyle = E("search-style");
@@ skipping 300 matching lines @@
         if (subscription.url in subscriptionsMap)
           subscription = updateSubscription(subscription);
         else
           addSubscription(subscription);
 
         collections.filterLists.addItems(subscription);
         updateLanguageCollections(subscription);
         break;
       case "removed":
         var knownSubscription = subscriptionsMap[subscription.url];
-
         if (subscription.url == acceptableAdsUrl || subscription.recommended)
         {
           subscription.disabled = true;
           onSubscriptionMessage("disabled", subscription);
         }
         else
         {
           collections.custom.removeItem(knownSubscription);
           delete subscriptionsMap[subscription.url];
         }
@@ skipping 231 matching lines @@
   });
   ext.backgroundPage.sendMessage(
   {
     type: "subscriptions.listen",
     filter: ["added", "disabled", "homepage", "lastDownload", "removed",
         "title", "downloadStatus", "downloading"]
   });
 
   window.addEventListener("DOMContentLoaded", onDOMLoaded, false);
 })();