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);

[Thomas Greiner, 2016/06/16 10:42:22]

Detail: Variable names should start with a lower-case letter.

[saroyanm, 2016/06/16 18:22:59]

Done.

         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)
-        {
-          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 controls = listItem.querySelectorAll(".control");
+        for (var k = 0; k < controls.length; k++)
+          controls[k].setAttribute("aria-labelledby", ItemId);
 
         this._setEmpty(table, null);
         if (table.hasChildNodes())
         {
           table.insertBefore(listItem,
               table.childNodes[this.items.indexOf(item)]);
         }
         else
           table.appendChild(listItem);
         this.updateItem(item);
@@ skipping 46 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;
+      element.setAttribute("label", title);

[Thomas Greiner, 2016/06/16 10:42:22]

What's the "label" attribute supposed to do?

[saroyanm, 2016/06/16 18:22:59]

I was testing with screen reader and apparently in this case it helps to provide
information about the list item, we are using labeldBy for the checkbox, but
it's still not obvious  for the people what is the item is about, so using label
we can provide information about the item to the blind people.

[Thomas Greiner, 2016/06/17 14:07:50]

Ok, in that case please use the standardized "aria-label" attribute instead (see
https://www.w3.org/TR/wai-aria/states_and_properties#aria-label) because I
couldn't find any documentation for "label".

[saroyanm, 2016/06/21 13:29:56]

I had another look and seems like removing this attribute works fine as well,
for some reason now I can hear duplication if I keep this attribute, so I think
it's better to remove it for now. So your suspicions regarding this label was
correct, sorry for that. But I was sure that this was the reason I used the
attribute :/ 

[Thomas Greiner, 2016/06/22 10:26:13]

I could imagine that the duplication might be because when you focus the
control, it reads first what section it's in and then its "aria-labelledby"
attribute which again refers to the section. In that case it should be
sufficient to remove the "aria-labelledby" attribute.

[saroyanm, 2016/06/22 13:46:16]

Genius! Done.

       if (title)
         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
+      emptyText: "options_dialog_language_other_empty"
     }
   ]);
   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 552 matching lines @@
   });
   ext.backgroundPage.sendMessage(
   {
     type: "subscriptions.listen",
     filter: ["added", "disabled", "homepage", "lastDownload", "removed",
         "title", "downloadStatus", "downloading"]
   });
 
   window.addEventListener("DOMContentLoaded", onDOMLoaded, false);
 })();