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 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("aria-label", this._getItemTitle(item, j));
         listItem.setAttribute("data-access", item.url || item.text);
-        listItem.setAttribute("id", ItemId);
         listItem.setAttribute("role", "section");
 
         var label = listItem.querySelector(".display");
-        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 controls = listItem.querySelectorAll(".control");
         for (var k = 0; k < controls.length; k++)
-          controls[k].setAttribute("aria-labelledby", ItemId);
+        {
+          if (controls[k].hasAttribute("title"))
+          {
+            var titleValue = getMessage(controls[k].getAttribute("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);
         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("aria-label", title);
+      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 == collections.filterLists)
           control.setAttribute("disabled", true);
       }
 
       var dateElement = element.querySelector(".date");
@@ skipping 87 matching lines @@
     },
     {
       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"
+      emptyText: "options_dialog_language_other_empty",
+      searchable: true
     }
   ]);
   collections.acceptableAds = new Collection(
   [
     {
       id: "acceptableads-table"
     }
   ]);
   collections.custom = new Collection(
   [
@@ skipping 608 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);
 })();