issue 4264 - centralize action handling

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 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"
   };
 
   function Collection(details)
@@ skipping 431 matching lines @@
       else
         location.href = link;
     });
   }
 
   function switchTab(id)
   {
     location.hash = id;
   }
 
+  function execAction(action, key, 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;
+        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");
+        break;
+      case "close-dialog":
+        closeDialog();
+        break;
+      case "edit-custom-filters":
+        editCustomFilters();
+        break;
+      case "edit-domain-exception":
+        document.querySelector("#whitelisting .controls").classList.add("mode-edit");
+        E("whitelisting-textbox").focus();
+        break;
+      case "import-subscription":
+        var url = E("blockingList-textbox").value;
+        addEnableSubscription(url);
+        closeDialog();
+        break;
+      case "open-context-menu":
+        var listItem = findParentData(element, "access", true);
+        if (listItem != context)
+          listItem.classList.add("show-context-menu");
+        break;
+      case "open-dialog":
+        var dialog = findParentData(element, "dialog", false);
+        openDialog(dialog);
+        break;
+      case "open-doclink":
+        var doclink = findParentData(element, "doclink", false);
+        openDocLink(doclink);
+        break;
+      case "remove-filter":
+        ext.backgroundPage.sendMessage(
+        {
+          type: "filters.remove",
+          text: findParentData(element, "access", false)
+        });
+        break;
+      case "remove-subscription":
+        ext.backgroundPage.sendMessage(
+        {
+          type: "subscriptions.remove",
+          url: findParentData(element, "access", false)
+        });
+        break;
+      case "save-custom-filters":
+        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);
+        }
+        break;
+      case "toggle-disable-subscription":
+        ext.backgroundPage.sendMessage(
+        {
+          type: "subscriptions.toggle",
+          keepInstalled: true,
+          url: findParentData(element, "access", false)
+        });
+        break;
+      case "toggle-pref":
+        ext.backgroundPage.sendMessage(
+        {
+          type: "prefs.toggle",
+          key: findParentData(element, "pref", 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 "update-all-subscriptions":
+        ext.backgroundPage.sendMessage(
+        {
+          type: "subscriptions.update"
+        });
+        break;
+      case "update-subscription":
+        ext.backgroundPage.sendMessage(
+        {
+          type: "subscriptions.update",
+          url: findParentData(element, "access", false)
+        });
+        break;
+    }
+  }
+
   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)
-        return;
+    var actions = findParentData(e.target, "action", false);
+    if (!actions)
+      return;
 
-      if (element.hasAttribute("data-action"))
-        break;
-
-      element = element.parentElement;
-    }
-
-    var element = findParentData(e.target, "action", true);
-    var actions = element.getAttribute("data-action").split(",");
+    actions = actions.split(",");
     for (var i = 0; i < actions.length; i++)
     {
-      switch (actions[i])
-      {
-        case "add-domain-exception":
-          addWhitelistedDomain();
-          break;
-        case "add-predefined-subscription":
-          var dialog = E("dialog-content-predefined");
-          var title = dialog.querySelector("h3").textContent;
-          var 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");
-          break;
-        case "close-dialog":
-          closeDialog();
-          break;
-        case "edit-custom-filters":
-          E("custom-filters").classList.add("mode-edit");
-          editCustomFilters();
-          break;
-        case "edit-domain-exception":
-          document.querySelector("#whitelisting .controls").classList.add("mode-edit");
-          E("whitelisting-textbox").focus();
-          break;
-        case "import-subscription":
-          var url = E("blockingList-textbox").value;
-          addEnableSubscription(url);
-          closeDialog();
-          break;
-        case "open-dialog":
-          var dialog = findParentData(element, "dialog", false);
-          openDialog(dialog);
-          break;
-        case "open-doclink":
-          var doclink = findParentData(element, "doclink", false);
-          openDocLink(doclink);
-          break;
-        case "save-custom-filters":
-          sendMessageHandleErrors(
-          {
-            type: "filters.importRaw",
-            text: E("custom-filters-raw").value,
-            removeExisting: true
-          },
-          function()
-          {
-            E("custom-filters").classList.remove("mode-edit");
-          });
-          break;
-        case "switch-tab":
-          var tabId = findParentData(e.target, "tab", false);
-          switchTab(tabId);
-          break;
-        case "toggle-pref":
-          ext.backgroundPage.sendMessage(
-          {
-            type: "prefs.toggle",
-            key: findParentData(element, "pref", false)
-          });
-          break;
-        case "update-all-subscriptions":
-          ext.backgroundPage.sendMessage(
-          {
-            type: "subscriptions.update"
-          });
-          break;
-        case "open-context-menu":
-          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: 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;
-      }
+      execAction(actions[i], "Enter", 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];
   }
@@ skipping 15 matching lines @@
       return;
 
     var container = findParentData(element, "action", true);
     if (!container || !container.hasAttribute("data-keys"))
       return;
 
     var keys = container.getAttribute("data-keys").split(" ");
     if (keys.indexOf(key) < 0)
       return;
 
-    switch (container.getAttribute("data-action"))
+    var actions = container.getAttribute("data-action").split(",");
+    for (var i = 0; i < actions.length; i++)
     {
-      case "add-domain-exception":
-        addWhitelistedDomain();
-        break;
-      case "open-doclink":
-        var doclink = findParentData(element, "doclink", false);
-        openDocLink(doclink);
-        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);
-        }
-        break;
+      execAction(actions[i], key, element);
     }
   }
 
   function selectTabItem(tabId, container, focus)
   {
     // Show tab content
     document.body.setAttribute("data-tab", tabId);
 
     // Select tab
     var tabList = container.querySelector("[role='tablist']");
@@ skipping 201 matching lines @@
       {
         ext.backgroundPage.sendMessage(
         {
           type: "filters.get",
           subscriptionUrl: subscriptions[i].url
         },
         function(filters)
         {
           for (var i = 0; i < filters.length; i++)
             updateFilter(filters[i]);
+
+          isCustomFiltersLoaded = true;
         });
       }
     });
     loadRecommendations();
     ext.backgroundPage.sendMessage(
     {
       type: "prefs.get",
       key: "subscriptions_exceptionsurl"
     },
     function(url)
@@ skipping 29 matching lines @@
         text: "@@||" + domain.value.toLowerCase() + "^$document"
       });
     }
 
     domain.value = "";
     document.querySelector("#whitelisting .controls").classList.remove("mode-edit");
   }
 
   function editCustomFilters()
   {
+    if (!isCustomFiltersLoaded)

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

Theoretically user will not need this check, but technically it's still possible
that the edit view might be triggered before the filters are loaded.

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

Nevertheless it'd be good to use `console.error()` to indicate that we've
encountered some unexpected behavior.

Note that it's still unclear how we should communicate this to the user.

[saroyanm, 2017/03/03 13:10:20]

Agree, done
I think we should revisit all console.errors we will have as soon we will
implement user notifications in options page.

+      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);
     E("custom-filters-raw").value = filterTexts.join("\n");
   }
 
   function addEnableSubscription(url, title, homepage)
   {
     var messageType = null;
@@ skipping 297 matching lines @@
   ext.backgroundPage.sendMessage(
   {
     type: "subscriptions.listen",
     filter: ["added", "disabled", "homepage", "lastDownload", "removed",
         "title", "downloadStatus", "downloading"]
   });
 
   window.addEventListener("DOMContentLoaded", onDOMLoaded, false);
   window.addEventListener("hashchange", onHashChange, false);
 })();