issue 4264 - centralize action handling

new-options.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
- * Copyright (C) 2006-2016 Eyeo GmbH
+ * Copyright (C) 2006-2017 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/>.
  */
 
+/* globals checkShareResource, getDocLink, i18nFormatDateTime, openSharePopup,
+           E */
+
 "use strict";
 
-(function()
 {
-  var subscriptionsMap = Object.create(null);
-  var filtersMap = Object.create(null);
-  var collections = Object.create(null);
-  var acceptableAdsUrl = null;
-  var isCustomFiltersLoaded = false;
-  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"
-  };
+  let subscriptionsMap = Object.create(null);
+  let filtersMap = Object.create(null);
+  let collections = Object.create(null);
+  let acceptableAdsUrl = null;
+  let isCustomFiltersLoaded = false;
+  let {getMessage} = ext.i18n;
+  let filterErrors = new Map([
+    ["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");
+    let placeholder = table.querySelector(".empty-placeholder");
     if (text && !placeholder)
     {
       placeholder = document.createElement("li");
       placeholder.className = "empty-placeholder";
       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, "\\'");
+    let access = (item.url || item.text).replace(/'/g, "\\'");
     return function(container)
     {
       return container.querySelector("[data-access='" + access + "']");
     };
   };
 
   Collection.prototype._getItemTitle = function(item, i)
   {
     if (item.url == acceptableAdsUrl)
       return getMessage("options_acceptableAds_description");
     if (this.details[i].useOriginalTitle && item.originalTitle)
       return item.originalTitle;
     return item.title || item.url || item.text;
   };
 
   Collection.prototype.addItem = function(item)
   {
     if (this.items.indexOf(item) >= 0)
       return;
 
     this.items.push(item);
-    this.items.sort(function(a, b)
+    this.items.sort((a, b) =>
     {
       // Make sure that Acceptable Ads is always last, since it cannot be
       // disabled, but only be removed. That way it's grouped together with
       // the "Own filter list" which cannot be disabled either at the bottom
       // of the filter lists in the Advanced tab.
       if (a.url == acceptableAdsUrl)
         return 1;
       if (b.url == acceptableAdsUrl)
         return -1;
 
-      var aTitle = this._getItemTitle(a, 0).toLowerCase();
-      var bTitle = this._getItemTitle(b, 0).toLowerCase();
+      let aTitle = this._getItemTitle(a, 0).toLowerCase();
+      let bTitle = this._getItemTitle(b, 0).toLowerCase();
       return aTitle.localeCompare(bTitle);
-    }.bind(this));
-
-    for (var j = 0; j < this.details.length; j++)
-    {
-      var table = E(this.details[j].id);
-      var template = table.querySelector("template");
-      var listItem = document.createElement("li");
+    });
+
+    for (let j = 0; j < this.details.length; j++)
+    {
+      let table = E(this.details[j].id);
+      let template = table.querySelector("template");
+      let listItem = document.createElement("li");
       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("role", "section");
 
-      var label = listItem.querySelector(".display");
+      let label = listItem.querySelector(".display");
       if (item.recommended && label.hasAttribute("data-tooltip"))
       {
-        var tooltipId = label.getAttribute("data-tooltip");
+        let 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++)
-      {
-        if (controls[k].hasAttribute("title"))
-        {
-          var titleValue = getMessage(controls[k].getAttribute("title"));
-          controls[k].setAttribute("title", titleValue)
+      for (let control of listItem.querySelectorAll(".control"))
+      {
+        if (control.hasAttribute("title"))
+        {
+          let titleValue = getMessage(control.getAttribute("title"));
+          control.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);
     }
     return length;
   };
 
   Collection.prototype.removeItem = function(item)
   {
-    var index = this.items.indexOf(item);
+    let 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);
+    let getListElement = this._createElementQuery(item);
+    for (let detail of this.details)
+    {
+      let table = E(detail.id);
+      let element = getListElement(table);
 
       // Element gets removed so make sure to handle focus appropriately
-      var control = element.querySelector(".control");
+      let 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;
+          let focusableElement = element.parentElement;
           while (focusableElement)
           {
-            if (focusableElement.classList.contains("tab-content")
-                || focusableElement.classList.contains("dialog-content"))
+            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);
+        this._setEmpty(table, detail.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 + "']");
+    let access = (item.url || item.text).replace(/'/g, "\\'");
+    for (let i = 0; i < this.details.length; i++)
+    {
+      let table = E(this.details[i].id);
+      let element = table.querySelector("[data-access='" + access + "']");
       if (!element)
         continue;
 
-      var title = this._getItemTitle(item, i);
+      let title = this._getItemTitle(item, i);
       element.querySelector(".display").textContent = title;
       element.setAttribute("aria-label", title);
       if (this.details[i].searchable)
         element.setAttribute("data-search", title.toLowerCase());
-      var control = element.querySelector(".control[role='checkbox']");
+      let 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");
-      var timeElement = element.querySelector(".time");
+      let dateElement = element.querySelector(".date");
+      let timeElement = element.querySelector(".time");
       if (dateElement && timeElement)
       {
-        var message = element.querySelector(".message");
+        let message = element.querySelector(".message");
         if (item.isDownloading)
         {
-          var text = getMessage("options_filterList_lastDownload_inProgress");
+          let text = getMessage("options_filterList_lastDownload_inProgress");
           message.textContent = text;
           element.classList.add("show-message");
         }
         else if (item.downloadStatus != "synchronize_ok")
         {
-          var error = filterErrors[item.downloadStatus];
+          let error = filterErrors.get(item.downloadStatus);
           if (error)
             message.textContent = getMessage(error);
           else
             message.textContent = item.downloadStatus;
           element.classList.add("show-message");
         }
         else if (item.lastDownload > 0)
         {
-          var dateTime = i18n_formatDateTime(item.lastDownload * 1000);
+          let dateTime = i18nFormatDateTime(item.lastDownload * 1000);
           dateElement.textContent = dateTime[0];
           timeElement.textContent = dateTime[1];
           element.classList.remove("show-message");
         }
       }
 
-      var websiteElement = element.querySelector(".context-menu .website");
+      let websiteElement = element.querySelector(".context-menu .website");
       if (websiteElement)
       {
         if (item.homepage)
           websiteElement.setAttribute("href", item.homepage);
         else
           websiteElement.setAttribute("aria-hidden", true);
       }
 
-      var sourceElement = element.querySelector(".context-menu .source");
+      let sourceElement = element.querySelector(".context-menu .source");
       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 element = table.firstChild;
+    for (let detail of this.details)
+    {
+      let table = E(detail.id);
+      let element = table.firstChild;
       while (element)
       {
         if (element.tagName == "LI" && !element.classList.contains("static"))
           table.removeChild(element);
         element = element.nextElementSibling;
       }
 
-      this._setEmpty(table, this.details[i].emptyText);
+      this._setEmpty(table, detail.emptyText);
     }
   };
 
   function focusNextElement(container, currentElement)
   {
-    var focusables = container.querySelectorAll("a, button, input, .control");
+    let focusables = container.querySelectorAll("a, button, input, .control");
     focusables = Array.prototype.slice.call(focusables);
-    var index = focusables.indexOf(currentElement);
+    let index = focusables.indexOf(currentElement);
     index += (index == focusables.length - 1) ? -1 : 1;
 
-    var nextElement = focusables[index];
+    let nextElement = focusables[index];
     if (!nextElement)
       return false;
 
     nextElement.focus();
     return true;
   }
 
-  collections.popular = new Collection(
-  [
+  collections.popular = new Collection([
     {
       id: "recommend-list-table"
     }
   ]);
-  collections.langs = new Collection(
-  [
+  collections.langs = new Collection([
     {
       id: "blocking-languages-table",
       emptyText: "options_dialog_language_added_empty"
     },
     {
       id: "blocking-languages-dialog-table",
       emptyText: "options_dialog_language_added_empty"
     }
   ]);
-  collections.allLangs = new Collection(
-  [
+  collections.allLangs = new Collection([
     {
       id: "all-lang-table",
       emptyText: "options_dialog_language_other_empty",
       searchable: true
     }
   ]);
-  collections.acceptableAds = new Collection(
-  [
+  collections.acceptableAds = new Collection([
     {
       id: "acceptableads-table"
     }
   ]);
-  collections.custom = new Collection(
-  [
+  collections.custom = new Collection([
     {
       id: "custom-list-table"
     }
   ]);
-  collections.whitelist = new Collection(
-  [
+  collections.whitelist = new Collection([
     {
       id: "whitelisting-table",
       emptyText: "options_whitelisted_empty"
     }
   ]);
-  collections.customFilters = new Collection(
-  [
+  collections.customFilters = new Collection([
     {
       id: "custom-filters-table",
       emptyText: "options_customFilters_empty"
     }
   ]);
-  collections.filterLists = new Collection(
-  [
+  collections.filterLists = new Collection([
     {
       id: "all-filter-lists-table",
       useOriginalTitle: true
     }
   ]);
 
   function toggleShowLanguage(subscription)
   {
     if (subscription.recommended == "ads")
     {
       if (subscription.disabled)
       {
         collections.allLangs.addItem(subscription);
         collections.langs.removeItem(subscription);
       }
       else
       {
         collections.allLangs.removeItem(subscription);
         collections.langs.addItem(subscription);
       }
     }
   }
 
   function addSubscription(subscription)
   {
-    var collection;
+    let collection;
     if (subscription.recommended)
     {
       if (subscription.recommended != "ads")
         collection = collections.popular;
       else if (subscription.disabled == false)
         collection = collections.langs;
       else
         collection = collections.allLangs;
     }
     else if (subscription.url == acceptableAdsUrl)
       collection = collections.acceptableAds;
     else
       collection = collections.custom;
 
     collection.addItem(subscription);
     subscriptionsMap[subscription.url] = subscription;
     toggleShowLanguage(subscription);
     updateTooltips();
   }
 
   function updateSubscription(subscription)
   {
-    for (var name in collections)
+    for (let name in collections)
       collections[name].updateItem(subscription);
 
     toggleShowLanguage(subscription);
   }
 
   function updateFilter(filter)
   {
-    var match = filter.text.match(/^@@\|\|([^\/:]+)\^\$document$/);
+    let match = filter.text.match(/^@@\|\|([^/:]+)\^\$document$/);
     if (match && !filtersMap[filter.text])
     {
       filter.title = match[1];
       collections.whitelist.addItem(filter);
     }
     else
       collections.customFilters.addItem(filter);
 
     filtersMap[filter.text] = filter;
   }
 
   function loadRecommendations()
   {
     fetch("subscriptions.xml")
-      .then(function(response)
+      .then((response) =>
       {
         return response.text();
       })
-      .then(function(text)
-      {
-        var list = document.getElementById("subscriptionSelector");
-        var doc = new DOMParser().parseFromString(text, "application/xml");
-        var elements = doc.documentElement.getElementsByTagName("subscription");
-        for (var i = 0; i < elements.length; i++)
-        {
-          var element = elements[i];
-          var type = element.getAttribute("type");
-          var subscription = {
+      .then((text) =>
+      {
+        let doc = new DOMParser().parseFromString(text, "application/xml");
+        let elements = doc.documentElement.getElementsByTagName("subscription");
+        for (let element of elements)
+        {
+          let type = element.getAttribute("type");
+          let subscription = {
             disabled: true,
             downloadStatus: null,
             homepage: null,
             originalTitle: element.getAttribute("title"),
             recommended: type,
             url: element.getAttribute("url")
           };
 
-          var prefix = element.getAttribute("prefixes");
+          let prefix = element.getAttribute("prefixes");
           if (prefix)
           {
             prefix = prefix.replace(/\W/g, "_");
             subscription.title = getMessage("options_language_" + prefix);
           }
           else
           {
             type = type.replace(/\W/g, "_");
-            subscription.title = getMessage("common_feature_" + type + "_title");
+            subscription.title = getMessage("common_feature_" +
+                                            type + "_title");
           }
 
           addSubscription(subscription);
         }
       });
   }
 
   function findParentData(element, dataName, returnElement)
   {
     while (element)
     {
       if (element.hasAttribute("data-" + dataName))
-        return returnElement ? element : element.getAttribute("data-" + dataName);
+      {
+        if (returnElement)
+          return element;
+        return element.getAttribute("data-" + dataName);
+      }
 
       element = element.parentElement;
     }
     return null;
   }
 
   function sendMessageHandleErrors(message, onSuccess)
   {
-    ext.backgroundPage.sendMessage(message, function(errors)
+    ext.backgroundPage.sendMessage(message, (errors) =>
     {
       if (errors.length > 0)
         alert(errors.join("\n"));
       else if (onSuccess)
         onSuccess();
     });
   }
 
   function openDocLink(id)
   {
-    getDocLink(id, function(link)
+    getDocLink(id, (link) =>
     {
       if (id == "share-general")
         openSharePopup(link);
       else
         location.href = link;
     });
   }
 
   function switchTab(id)
   {
     location.hash = id;
   }
 
-  function execAction(action, key, element)
+  function execAction(action, element)
   {
     switch (action)
     {
       case "add-domain-exception":
         addWhitelistedDomain();
         break;
       case "add-language-subscription":
         addEnableSubscription(findParentData(element, "access", false));
         break;
-      case "add-predefined-subscription":
-        var dialog = E("dialog-content-predefined");
-        var title = dialog.querySelector("h3").textContent;
-        var url = dialog.querySelector(".url").textContent;
+      case "add-predefined-subscription": {
+        let dialog = E("dialog-content-predefined");
+        let title = dialog.querySelector("h3").textContent;
+        let url = dialog.querySelector(".url").textContent;
         addEnableSubscription(url, title);
         closeDialog();
         break;
+      }
       case "cancel-custom-filters":
         E("custom-filters").classList.remove("mode-edit");
         break;
       case "cancel-domain-exception":
         E("whitelisting-textbox").value = "";
-        document.querySelector("#whitelisting .controls").classList.remove("mode-edit");
+        document.querySelector("#whitelisting .controls").classList
+          .remove("mode-edit");
         break;
       case "close-dialog":
         closeDialog();
         break;
       case "edit-custom-filters":
         editCustomFilters();
         break;
       case "edit-domain-exception":
-        document.querySelector("#whitelisting .controls").classList.add("mode-edit");
+        document.querySelector("#whitelisting .controls").classList
+          .add("mode-edit");
         E("whitelisting-textbox").focus();
         break;
-      case "import-subscription":
-        var url = E("blockingList-textbox").value;
+      case "import-subscription": {
+        let url = E("blockingList-textbox").value;
         addEnableSubscription(url);
         closeDialog();
         break;
-      case "open-context-menu":
-        var listItem = findParentData(element, "access", true);
-        if (listItem != context)
+      }
+      case "open-context-menu": {
+        let listItem = findParentData(element, "access", true);
+        if (listItem && !listItem.classList.contains("show-context-menu"))
           listItem.classList.add("show-context-menu");
         break;
-      case "open-dialog":
-        var dialog = findParentData(element, "dialog", false);
+      }
+      case "open-dialog": {
+        let dialog = findParentData(element, "dialog", false);
         openDialog(dialog);
         break;
-      case "open-doclink":
-        var doclink = findParentData(element, "doclink", false);
+      }
+      case "open-doclink": {
+        let doclink = findParentData(element, "doclink", false);
         openDocLink(doclink);
         break;
+      }
       case "remove-filter":
-        ext.backgroundPage.sendMessage(
-        {
+        ext.backgroundPage.sendMessage({
           type: "filters.remove",
           text: findParentData(element, "access", false)
         });
         break;
       case "remove-subscription":
-        ext.backgroundPage.sendMessage(
-        {
+        ext.backgroundPage.sendMessage({
           type: "subscriptions.remove",
           url: findParentData(element, "access", false)
         });
         break;
       case "save-custom-filters":
-        sendMessageHandleErrors(
-        {
+        sendMessageHandleErrors({
           type: "filters.importRaw",
           text: E("custom-filters-raw").value,
           removeExisting: true
         },
-        function()
+        () =>
         {
           E("custom-filters").classList.remove("mode-edit");
         });
         break;
       case "switch-tab":
-        if (key == "Enter")
-        {
-          var tabId = findParentData(element, "tab", false);
-          switchTab(tabId);
-        }
-        else if (element.hasAttribute("aria-selected"))
-        {
-          if (key == "ArrowLeft" || key == "ArrowUp")
-          {
-            element = element.previousElementSibling
-                || container.lastElementChild;
-          }
-          else if (key == "ArrowRight" || key == "ArrowDown")
-          {
-            element = element.nextElementSibling
-                || container.firstElementChild;
-          }
-
-          var tabId = findParentData(element, "tab", false);
-          switchTab(tabId);
-        }
+        let tabId = findParentData(element, "tab", false);
+        switchTab(tabId);
         break;
       case "toggle-disable-subscription":
-        ext.backgroundPage.sendMessage(
-        {
+        ext.backgroundPage.sendMessage({
           type: "subscriptions.toggle",
           keepInstalled: true,
           url: findParentData(element, "access", false)
         });
         break;
       case "toggle-pref":
-        ext.backgroundPage.sendMessage(
-        {
+        ext.backgroundPage.sendMessage({
           type: "prefs.toggle",
           key: findParentData(element, "pref", false)
         });
         break;
       case "toggle-remove-subscription":
-        var subscriptionUrl = findParentData(element, "access", false);
+        let subscriptionUrl = findParentData(element, "access", false);
         if (element.getAttribute("aria-checked") == "true")
         {
           ext.backgroundPage.sendMessage({
             type: "subscriptions.remove",
             url: subscriptionUrl
           });
         }
         else
           addEnableSubscription(subscriptionUrl);
         break;
       case "update-all-subscriptions":
-        ext.backgroundPage.sendMessage(
-        {
+        ext.backgroundPage.sendMessage({
           type: "subscriptions.update"
         });
         break;
       case "update-subscription":
-        ext.backgroundPage.sendMessage(
-        {
+        ext.backgroundPage.sendMessage({
           type: "subscriptions.update",
           url: findParentData(element, "access", false)
         });
         break;
     }
   }
 
   function onClick(e)
   {
-    var context = document.querySelector(".show-context-menu");
+    let context = document.querySelector(".show-context-menu");
     if (context)
       context.classList.remove("show-context-menu");
 
-    var actions = findParentData(e.target, "action", false);
+    let actions = findParentData(e.target, "action", false);
     if (!actions)
       return;
 
     actions = actions.split(",");
-    for (var i = 0; i < actions.length; i++)
-    {
-      execAction(actions[i], "Enter", e.target);
+    for (let action of actions)
+    {
+      execAction(action, e.target);
     }
   }
 
   function getKey(e)
   {
     // e.keyCode has been deprecated so we attempt to use e.key
     if ("key" in e)
       return e.key;
     return getKey.keys[e.keyCode];
   }
   getKey.keys = {
     9: "Tab",
     13: "Enter",
     27: "Escape",
     37: "ArrowLeft",
     38: "ArrowUp",
     39: "ArrowRight",
     40: "ArrowDown"
   };
 
   function onKeyUp(e)
   {
-    var key = getKey(e);
-    var element = document.activeElement;
+    let key = getKey(e);
+    let element = document.activeElement;
     if (!key || !element)
       return;
 
-    var container = findParentData(element, "action", true);
+    let container = findParentData(element, "action", true);
     if (!container || !container.hasAttribute("data-keys"))
       return;
 
-    var keys = container.getAttribute("data-keys").split(" ");
+    let keys = container.getAttribute("data-keys").split(" ");
     if (keys.indexOf(key) < 0)
       return;
 
-    var actions = container.getAttribute("data-action").split(",");
-    for (var i = 0; i < actions.length; i++)
-    {
-      execAction(actions[i], key, element);
+    if (element.getAttribute("role") == "tab")
+    {
+      if (key == "ArrowLeft" || key == "ArrowUp")
+        element = element.previousElementSibling || container.lastElementChild;
+      else if (key == "ArrowRight" || key == "ArrowDown")
+        element = element.nextElementSibling || container.firstElementChild;
+    }
+
+    let actions = container.getAttribute("data-action").split(",");
+    for (let action of actions)
+    {
+      execAction(action, element);
     }
   }
 
   function selectTabItem(tabId, container, focus)
   {
-    if (tabId == "advanced-customFilters" && !isCustomFiltersLoaded)
-      switchTab("advanced-allFilterLists");

[Thomas Greiner, 2017/01/27 16:04:33]

This appears to be a loop that only resolves when `isCustomFiltersLoaded` is
`true`:

onHashChange -> selectTabItem -> switchTab -> (next item in event loop) ->
onHashChange -> ...

Therefore this could turn into an infinite loop in case of issues with loading
custom filters so it'd be great if we could stick to the current approach of
updating the UI by reacting to events.

But apart from that, is there a reason why this check even exists? Just because
there might not be any custom filters to look at, doesn't mean that the tab
doesn't provide any value.

[saroyanm, 2017/01/27 16:13:44]

It's not a loop, switch from one tab to another if the one not ready to be load.
It shouldn't turn into infinite loop if there is error with page.
We might reconsider the way we are showing the "Edit" tab while we currently
using the implementation in current options page, which is tricky and doesn't
include update of textarea when new filter is added. I think it's safer this way
to switch tab if the filters are not loaded and we can fix the general issue
with the texarea not being populated dynamical in separate issue.

[Thomas Greiner, 2017/01/27 16:39:27]

You're right. I must've misread that as "advanced-allFilterlists" when I wrote
the first part of the comment.
It might be safer but it's a terrible user experience because:

a) Clicking on the tab doesn't switch to it but you stay on the
"advanced-allFilterlists" tab.

b) Deep-linking to the "advanced-customFilters" tab will show the
"advanced-allFilterlists" tab. This will cause confusion used by instructions
that may refer to UI elements within that tab.

Therefore I'd suggest to stick to the implementation that's in the current
options page and not update the text area, only the list of filters.

[saroyanm, 2017/01/27 16:56:41]

I'll strongly disagree sticking to the current option page implementation while
using deeplinking to the edit mode, because if we will allow user to switch to
the edit mode while the filters are not loaded completely, click on save button
might lead to the loose of the existing custom filter lists.
So I can see two possibilities:
1) Do not allow deeplinking to the edit mode.
2) Only switch to edit mode if the custom filter lists are populated.

 

[Thomas Greiner, 2017/01/30 11:57:53]

"advanced-customFilters" is not a deep-link to the edit mode. I assume that's
where the confusion comes from. The text area should not be updated whereas the
list of custom filters should, right?

In that case maybe it'd be best to prevent the user from accessing edit mode
until custom filters have finished loading.

[saroyanm, 2017/02/01 12:28:10]

Probably, I was refering to: /new-options.html#advanced-customFilters 
Maybe I'm wrong
Does that mean that you are fine with current approach ?

[Thomas Greiner, 2017/02/02 12:24:50]

The "advanced-customFilters" can be accessed either in "view" or "edit" mode
(see `editCustomFilters()`). While you should still be able to view the custom
filters while they're loading, you shouldn't be able to edit them.

Your current approach, however, does not allow the user to even view them.

[saroyanm, 2017/02/02 12:34:01]

Clear, agree we should definately show the custom filters view tab instead of
full filterlists tab, will update with the next patch

-
     // Show tab content
     document.body.setAttribute("data-tab", tabId);
 
     // Select tab
-    var tabList = container.querySelector("[role='tablist']");
+    let tabList = container.querySelector("[role='tablist']");
     if (!tabList)
       return null;
 
-    var previousTab = tabList.querySelector("[aria-selected]");
+    let previousTab = tabList.querySelector("[aria-selected]");
     previousTab.removeAttribute("aria-selected");
     previousTab.setAttribute("tabindex", -1);
 
-    var tab = tabList.querySelector("li[data-tab='" + tabId + "']");
+    let tab = tabList.querySelector("li[data-tab='" + tabId + "']");
     tab.setAttribute("aria-selected", true);
     tab.setAttribute("tabindex", 0);
 
-    var tabContentId = tab.getAttribute("aria-controls");
-    var tabContent = document.getElementById(tabContentId);
+    let tabContentId = tab.getAttribute("aria-controls");
+    let tabContent = document.getElementById(tabContentId);
 
     // Select sub tabs
     if (tab.hasAttribute("data-subtab"))
       selectTabItem(tab.getAttribute("data-subtab"), tabContent, false);
 
     if (tab && focus)
       tab.focus();
 
-    if (tabId == "advanced-customFilters")

[Thomas Greiner, 2017/01/27 16:04:33]

What about making this more generic to declare the behavior in the HTML: It's a
dependency that needs to be resolved before the tab should be shown.

For instance, I could imagine "data-dependency='edit-custom-filters'" and handle
it like this:

if (tab.hasAttribute("data-dependency"))
  execAction(tab.getAttribute("data-dependency");

[saroyanm, 2017/01/27 16:13:44]

I'll investigate that and provide in new patch, I agree that dependency should
make more sense here.

[saroyanm, 2017/02/08 12:14:00]

This is not relevant anymore while we are showing view of custom filters in new
patch and also we can use data-action="switch-tab,edit-custom-filters" currently
in HTML.

[Thomas Greiner, 2017/03/02 15:10:13]

You're right. Using "data-action" is more appropriate in this case but you
removed it in your first patch.

-      editCustomFilters();
-
     return tabContent;
   }
 
   function onHashChange()
   {
-    var hash = location.hash.substr(1);
+    let hash = location.hash.substr(1);
     if (!hash)
       return;
 
     // Select tab and parent tabs
-    var tabIds = hash.split("-");
-    var tabContent = document.body;
-    for (var i = 0; i < tabIds.length; i++)
-    {
-      var tabId = tabIds.slice(0, i + 1).join("-");
+    let tabIds = hash.split("-");
+    let tabContent = document.body;
+    for (let i = 0; i < tabIds.length; i++)
+    {
+      let tabId = tabIds.slice(0, i + 1).join("-");
       tabContent = selectTabItem(tabId, tabContent, true);
       if (!tabContent)
         break;
     }
   }
 
   function onDOMLoaded()
   {
     populateLists();
     function onFindLanguageKeyUp()
     {
-      var searchStyle = E("search-style");
+      let searchStyle = E("search-style");
       if (!this.value)
         searchStyle.innerHTML = "";
       else
-        searchStyle.innerHTML = "#all-lang-table li:not([data-search*=\"" + this.value.toLowerCase() + "\"]) { display: none; }";
+      {
+        searchStyle.innerHTML = "#all-lang-table li:not([data-search*=\"" +
+          this.value.toLowerCase() + "\"]) { display: none; }";
+      }
     }
 
     // Initialize navigation sidebar
-    ext.backgroundPage.sendMessage(
-    {
+    ext.backgroundPage.sendMessage({
       type: "app.get",
       what: "addonVersion"
     },
-    function(addonVersion)
+    (addonVersion) =>
     {
       E("abp-version").textContent = addonVersion;
     });
-    getDocLink("releases", function(link)
+    getDocLink("releases", (link) =>
     {
       E("link-version").setAttribute("href", link);
     });
 
     updateShareLink();
     updateTooltips();
 
     // Initialize interactive UI elements
     document.body.addEventListener("click", onClick, false);
     document.body.addEventListener("keyup", onKeyUp, false);
-    var placeholderValue  = getMessage("options_dialog_language_find");
+    let 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)
+    E("whitelisting-textbox").addEventListener("keypress", (e) =>
     {
       if (getKey(e) == "Enter")
         addWhitelistedDomain();
     }, false);
 
     // Advanced tab
-    var tweaks = document.querySelectorAll("#tweaks li[data-pref]");
-    tweaks = Array.prototype.map.call(tweaks, function(checkbox)
+    let tweaks = document.querySelectorAll("#tweaks li[data-pref]");
+    tweaks = Array.prototype.map.call(tweaks, (checkbox) =>
     {
       return checkbox.getAttribute("data-pref");
     });
-    tweaks.forEach(function(key)
-    {
-      getPref(key, function(value)
+    for (let key of tweaks)
+    {
+      getPref(key, (value) =>
       {
         onPrefMessage(key, value, true);
       });
-    });
-    ext.backgroundPage.sendMessage(
-    {
+    }
+    ext.backgroundPage.sendMessage({
       type: "app.get",
       what: "features"
     },
-    function(features)
+    (features) =>
     {
       hidePref("show_devtools_panel", !features.devToolsPanel);
     });
 
-    var filterTextbox = document.querySelector("#custom-filters-add input");
+    let filterTextbox = document.querySelector("#custom-filters-add input");
     placeholderValue = getMessage("options_customFilters_textbox_placeholder");
     filterTextbox.setAttribute("placeholder", placeholderValue);
     function addCustomFilters()
     {
-      var filterText = filterTextbox.value;
-      sendMessageHandleErrors(
-      {
+      let filterText = filterTextbox.value;
+      sendMessageHandleErrors({
         type: "filters.add",
         text: filterText
       },
-      function()
+      () =>
       {
         filterTextbox.value = "";
       });
     }
-    E("custom-filters-add").addEventListener("submit", function(e)
+    E("custom-filters-add").addEventListener("submit", (e) =>
     {
       e.preventDefault();
       addCustomFilters();
     }, false);
-    var customFilterEditButtons = document.querySelectorAll("#custom-filters-edit-wrapper button");
+
+    // Help tab
+    getDocLink("faq", (link) =>
+    {
+      E("link-faq").setAttribute("href", link);
+    });
+    getDocLink("social_twitter", (link) =>
+    {
+      E("link-twitter").setAttribute("href", link);
+    });
+    getDocLink("social_facebook", (link) =>
+    {
+      E("link-facebook").setAttribute("href", link);
+    });
+    getDocLink("social_gplus", (link) =>
+    {
+      E("link-gplus").setAttribute("href", link);
+    });
+    getDocLink("social_renren", (link) =>
+    {
+      E("link-renren").setAttribute("href", link);
+    });
+    getDocLink("social_weibo", (link) =>
+    {
+      E("link-weibo").setAttribute("href", link);
+    });
+
+    // Set forum link
+    ext.backgroundPage.sendMessage({
+      type: "app.get",
+      what: "platform"
+    },
+    (platform) =>
+    {
+      ext.backgroundPage.sendMessage({
+        type: "app.get",
+        what: "application"
+      },
+      (application) =>
+      {
+        if (platform == "chromium" && application != "opera")
+          application = "chrome";
+
+        getDocLink(application + "_support", (link) =>
+        {
+          E("link-forum").setAttribute("href", link);
+        });
+      });
+    });
 
     E("dialog").addEventListener("keydown", function(e)
     {
       switch (getKey(e))
       {
         case "Escape":
           closeDialog();
           break;
         case "Tab":
           if (e.shiftKey)
           {
             if (e.target.classList.contains("focus-first"))
             {
               e.preventDefault();
               this.querySelector(".focus-last").focus();
             }
           }
           else if (e.target.classList.contains("focus-last"))
           {
             e.preventDefault();
             this.querySelector(".focus-first").focus();
           }
           break;
       }
     }, false);
 
     onHashChange();
   }
 
-  var focusedBeforeDialog = null;
+  let focusedBeforeDialog = null;
   function openDialog(name)
   {
-    var dialog = E("dialog");
+    let dialog = E("dialog");
     dialog.setAttribute("aria-hidden", false);
     dialog.setAttribute("aria-labelledby", "dialog-title-" + name);
     document.body.setAttribute("data-dialog", name);
 
-    var defaultFocus = document.querySelector("#dialog-content-" + name
-      + " .default-focus");
+    let defaultFocus = document.querySelector(
+      "#dialog-content-" + name + " .default-focus"
+    );
     if (!defaultFocus)
       defaultFocus = dialog.querySelector(".focus-first");
     focusedBeforeDialog = document.activeElement;
     defaultFocus.focus();
   }
 
   function closeDialog()
   {
-    var dialog = E("dialog");
+    let dialog = E("dialog");
     dialog.setAttribute("aria-hidden", true);
     dialog.removeAttribute("aria-labelledby");
     document.body.removeAttribute("data-dialog");
     focusedBeforeDialog.focus();
   }
 
   function populateLists()
   {
     subscriptionsMap = Object.create(null);
     filtersMap = Object.create(null);
 
     // Empty collections and lists
-    for (var property in collections)
+    for (let property in collections)
       collections[property].clearAll();
 
-    ext.backgroundPage.sendMessage(
-    {
+    ext.backgroundPage.sendMessage({
       type: "subscriptions.get",
       special: true
     },
-    function(subscriptions)
+    (subscriptions) =>
     {
       // Load filters
-      for (var i = 0; i < subscriptions.length; i++)
-      {
-        ext.backgroundPage.sendMessage(
-        {
+      for (let subscription of subscriptions)
+      {
+        ext.backgroundPage.sendMessage({
           type: "filters.get",
-          subscriptionUrl: subscriptions[i].url
+          subscriptionUrl: subscription.url
         },
-        function(filters)
-        {
-          for (var i = 0; i < filters.length; i++)
-            updateFilter(filters[i]);
+        (filters) =>
+        {
+          for (let filter of filters)
+            updateFilter(filter);
 
           isCustomFiltersLoaded = true;
         });
       }
     });
     loadRecommendations();
-    ext.backgroundPage.sendMessage(
-    {
+    ext.backgroundPage.sendMessage({
       type: "prefs.get",
       key: "subscriptions_exceptionsurl"
     },
-    function(url)
+    (url) =>
     {
       acceptableAdsUrl = url;
       addSubscription({
         url: acceptableAdsUrl,
         disabled: true
       });
 
       // Load user subscriptions
-      ext.backgroundPage.sendMessage(
-      {
+      ext.backgroundPage.sendMessage({
         type: "subscriptions.get",
         downloadable: true
       },
-      function(subscriptions)
-      {
-        for (var i = 0; i < subscriptions.length; i++)
-          onSubscriptionMessage("added", subscriptions[i]);
+      (subscriptions) =>
+      {
+        for (let subscription of subscriptions)
+          onSubscriptionMessage("added", subscription);
       });
     });
   }
 
   function addWhitelistedDomain()
   {
-    var domain = E("whitelisting-textbox");
+    let domain = E("whitelisting-textbox");
     if (domain.value)
     {
-      sendMessageHandleErrors(
-      {
+      sendMessageHandleErrors({
         type: "filters.add",
         text: "@@||" + domain.value.toLowerCase() + "^$document"
       });
     }
 
     domain.value = "";
-    document.querySelector("#whitelisting .controls").classList.remove("mode-edit");
+    document.querySelector("#whitelisting .controls")
+      .classList.remove("mode-edit");
   }
 
   function editCustomFilters()
   {
+    if (!isCustomFiltersLoaded)
+    {
+      console.error("Custom filters are not loaded");
+      return;
+    }
+
     E("custom-filters").classList.add("mode-edit");
-    var customFilterItems = collections.customFilters.items;
-    var filterTexts = [];
-    for (var i = 0; i < customFilterItems.length; i++)
-      filterTexts.push(customFilterItems[i].text);
+    let filterTexts = [];
+    for (let customFilterItem of collections.customFilters.items)
+      filterTexts.push(customFilterItem.text);
     E("custom-filters-raw").value = filterTexts.join("\n");
   }
 
   function addEnableSubscription(url, title, homepage)
   {
-    var messageType = null;
-    var knownSubscription = subscriptionsMap[url];
+    let messageType = null;
+    let knownSubscription = subscriptionsMap[url];
     if (knownSubscription && knownSubscription.disabled == true)
       messageType = "subscriptions.toggle";
     else
       messageType = "subscriptions.add";
 
-    var message = {
+    let message = {
       type: messageType,
-      url: url
+      url
     };
     if (title)
       message.title = title;
     if (homepage)
       message.homepage = homepage;
 
     ext.backgroundPage.sendMessage(message);
   }
 
   function onFilterMessage(action, filter)
   {
     switch (action)
     {
       case "added":
         updateFilter(filter);
         updateShareLink();
         break;
       case "loaded":
         populateLists();
         break;
       case "removed":
-        var knownFilter = filtersMap[filter.text];
+        let knownFilter = filtersMap[filter.text];
         collections.whitelist.removeItem(knownFilter);
         collections.customFilters.removeItem(knownFilter);
         delete filtersMap[filter.text];
         updateShareLink();
         break;
     }
   }
 
   function onSubscriptionMessage(action, subscription)
   {
     if (subscription.url in subscriptionsMap)
     {
-      var knownSubscription = subscriptionsMap[subscription.url];
-      for (var property in subscription)
+      let knownSubscription = subscriptionsMap[subscription.url];
+      for (let property in subscription)
       {
         if (property == "title" && knownSubscription.recommended)
           knownSubscription.originalTitle = subscription.title;
         else
           knownSubscription[property] = subscription[property];
       }
       subscription = knownSubscription;
     }
     switch (action)
     {
@@ skipping 28 matching lines @@
         }
         collections.filterLists.removeItem(subscription);
         break;
     }
 
     updateShareLink();
   }
 
   function hidePref(key, value)
   {
-    var element = document.querySelector("[data-pref='" + key + "']");
+    let element = document.querySelector("[data-pref='" + key + "']");
     if (element)
       element.setAttribute("aria-hidden", value);
   }
 
   function getPref(key, callback)
   {
-    var checkPref = getPref.checks[key] || getPref.checkNone;
-    checkPref(function(isActive)
+    let checkPref = getPref.checks[key] || getPref.checkNone;
+    checkPref((isActive) =>
     {
       if (!isActive)
       {
         hidePref(key, !isActive);
         return;
       }
 
-      ext.backgroundPage.sendMessage(
-      {
+      ext.backgroundPage.sendMessage({
         type: "prefs.get",
-        key: key
+        key
       }, callback);
     });
   }
 
   getPref.checkNone = function(callback)
   {
     callback(true);
   };
 
   getPref.checks =
   {
-    notifications_ignoredcategories: function(callback)
+    notifications_ignoredcategories(callback)
     {
       getPref("notifications_showui", callback);
     }
   };
 
   function onPrefMessage(key, value, initial)
   {
     switch (key)
     {
       case "notifications_ignoredcategories":
         value = value.indexOf("*") == -1;
         break;
 
       case "notifications_showui":
         hidePref("notifications_ignoredcategories", !value);
         break;
     }
 
-    var checkbox = document.querySelector("[data-pref='" + key + "'] button[role='checkbox']");
+    let checkbox = document.querySelector(
+      "[data-pref='" + key + "'] button[role='checkbox']"
+    );
     if (checkbox)
       checkbox.setAttribute("aria-checked", value);
   }
 
   function updateShareLink()
   {
-    var shareResources = [
+    let shareResources = [
       "https://facebook.com/plugins/like.php?",
       "https://platform.twitter.com/widgets/",
       "https://apis.google.com/se/0/_/+1/fastbutton?"
     ];
-    var isAnyBlocked = false;
-    var checksRemaining = shareResources.length;
+    let isAnyBlocked = false;
+    let checksRemaining = shareResources.length;
 
     function onResult(isBlocked)
     {
       isAnyBlocked |= isBlocked;
       if (!--checksRemaining)
       {
         // Hide the share tab if a script on the share page would be blocked
         E("tab-share").hidden = isAnyBlocked;
       }
     }
 
-    for (var i = 0; i < shareResources.length; i++)
-      checkShareResource(shareResources[i], onResult);
+    for (let sharedResource of shareResources)
+      checkShareResource(sharedResource, onResult);
   }
 
   function getMessages(id)
   {
-    var messages = [];
-    for (var i = 1; true; i++)
-    {
-      var message = ext.i18n.getMessage(id + "_" + i);
+    let messages = [];
+    for (let i = 1; true; i++)
+    {
+      let message = ext.i18n.getMessage(id + "_" + i);
       if (!message)
         break;
 
       messages.push(message);
     }
     return messages;
   }
 
   function updateTooltips()
   {
-    var anchors = document.querySelectorAll(":not(.tooltip) > [data-tooltip]");
-    for (var i = 0; i < anchors.length; i++)
-    {
-      var anchor = anchors[i];
-      var id = anchor.getAttribute("data-tooltip");
-
-      var wrapper = document.createElement("div");
+    let anchors = document.querySelectorAll(":not(.tooltip) > [data-tooltip]");
+    for (let anchor of anchors)
+    {
+      let id = anchor.getAttribute("data-tooltip");
+
+      let wrapper = document.createElement("div");
       wrapper.className = "tooltip";
       anchor.parentNode.replaceChild(wrapper, anchor);
       wrapper.appendChild(anchor);
 
-      var topTexts = getMessages(id);
-      var bottomTexts = getMessages(id + "_notes");
+      let topTexts = getMessages(id);
+      let bottomTexts = getMessages(id + "_notes");
 
       // We have to use native tooltips to avoid issues when attaching a tooltip
       // to an element in a scrollable list or otherwise it might get cut off
       if (anchor.hasAttribute("data-tooltip-native"))
       {
-        var title = topTexts.concat(bottomTexts).join("\n\n");
+        let title = topTexts.concat(bottomTexts).join("\n\n");
         anchor.setAttribute("title", title);
         continue;
       }
 
-      var tooltip = document.createElement("div");
+      let tooltip = document.createElement("div");
       tooltip.setAttribute("role", "tooltip");
 
-      var flip = anchor.getAttribute("data-tooltip-flip");
+      let flip = anchor.getAttribute("data-tooltip-flip");
       if (flip)
         tooltip.className = "flip-" + flip;
 
-      var imageSource = anchor.getAttribute("data-tooltip-image");
+      let imageSource = anchor.getAttribute("data-tooltip-image");
       if (imageSource)
       {
-        var image = document.createElement("img");
+        let image = document.createElement("img");
         image.src = imageSource;
         image.alt = "";
         tooltip.appendChild(image);
       }
 
-      for (var j = 0; j < topTexts.length; j++)
-      {
-        var paragraph = document.createElement("p");
-        paragraph.innerHTML = topTexts[j];
+      for (let topText of topTexts)
+      {
+        let paragraph = document.createElement("p");
+        paragraph.innerHTML = topText;
         tooltip.appendChild(paragraph);
       }
       if (bottomTexts.length > 0)
       {
-        var notes = document.createElement("div");
+        let notes = document.createElement("div");
         notes.className = "notes";
-        for (var j = 0; j < bottomTexts.length; j++)
-        {
-          var paragraph = document.createElement("p");
-          paragraph.innerHTML = bottomTexts[j];
+        for (let bottomText of bottomTexts)
+        {
+          let paragraph = document.createElement("p");
+          paragraph.innerHTML = bottomText;
           notes.appendChild(paragraph);
         }
         tooltip.appendChild(notes);
       }
 
       wrapper.appendChild(tooltip);
     }
   }
 
-  ext.onMessage.addListener(function(message)
+  ext.onMessage.addListener((message) =>
   {
     switch (message.type)
     {
       case "app.respond":
         switch (message.action)
         {
           case "addSubscription":
-            var subscription = message.args[0];
-            var dialog = E("dialog-content-predefined");
+            let subscription = message.args[0];
+            let dialog = E("dialog-content-predefined");
             dialog.querySelector("h3").textContent = subscription.title || "";
             dialog.querySelector(".url").textContent = subscription.url;
             openDialog("predefined");
             break;
           case "focusSection":
             document.body.setAttribute("data-tab", message.args[0]);
             break;
         }
         break;
       case "filters.respond":
         onFilterMessage(message.action, message.args[0]);
         break;
       case "prefs.respond":
         onPrefMessage(message.action, message.args[0], false);
         break;
       case "subscriptions.respond":
         onSubscriptionMessage(message.action, message.args[0]);
         break;
     }
   });
 
-  ext.backgroundPage.sendMessage(
-  {
+  ext.backgroundPage.sendMessage({
     type: "app.listen",
     filter: ["addSubscription", "focusSection"]
   });
-  ext.backgroundPage.sendMessage(
-  {
+  ext.backgroundPage.sendMessage({
     type: "filters.listen",
     filter: ["added", "loaded", "removed"]
   });
-  ext.backgroundPage.sendMessage(
-  {
+  ext.backgroundPage.sendMessage({
     type: "prefs.listen",
     filter: ["notifications_ignoredcategories", "notifications_showui",
-        "show_devtools_panel", "shouldShowBlockElementMenu"]
+             "show_devtools_panel", "shouldShowBlockElementMenu"]
   });
-  ext.backgroundPage.sendMessage(
-  {
+  ext.backgroundPage.sendMessage({
     type: "subscriptions.listen",
     filter: ["added", "disabled", "homepage", "lastDownload", "removed",
-        "title", "downloadStatus", "downloading"]
+             "title", "downloadStatus", "downloading"]
   });
 
   window.addEventListener("DOMContentLoaded", onDOMLoaded, false);
   window.addEventListener("hashchange", onHashChange, false);
-})();
+}