issue 1526 - Implement new options page design for Chrome/Opera/Safari

options.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 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
@@ skipping 20 matching lines @@
         this.sort(function(a, b)
         {
           var aValue = (a.title || a.url || a.text).toLowerCase();
           var bValue = (b.title || b.url || a.text).toLowerCase();
           if (aValue < bValue)
             return -1;
           if (aValue > bValue)
             return 1;
           return 0;
         });
-        for (var i=0; i < arguments.length; i++)
-        {
-          for (var j=0; j < this.table.length; j++)
+        for (var i = 0; i < arguments.length; i++)
+        {
+          for (var j = 0; j < this.table.length; j++)
           {
             var object = arguments[i];
             var text = object.title || object.text;
             var access = object.url || object.text;
             var index = this.indexOf(object);
             var table = E(this.table[j].id);
             var template = this.table[j].template;
             var listener = this.table[j].listener;
             
-            var list = document.createElement("li");
-            list.innerHTML = template;
-            list.dataset.access = access;
-            list.getElementsByClassName("display")[0].textContent = text;
+            var listItem = document.createElement("li");
+            listItem.innerHTML = template;
+            listItem.dataset.access = access;
+            listItem.querySelector(".display").textContent = text;
             if (object.isAdsType)
-              list.dataset.value = text.toLowerCase();
-
-            var control = list.getElementsByClassName("control")[0];
+              listItem.dataset.search = text.toLowerCase();
+
+            var control = listItem.querySelector(".control");
             if (control)
             {
               control.addEventListener("click", listener, false);
               control.checked = !object.disabled;
             }
             if (table.hasChildNodes)
-              table.insertBefore(list, table.childNodes[index]);
+              table.insertBefore(listItem, table.childNodes[index]);
             else
-              table.appendChild(list);
+              table.appendChild(listItem);
           }
         }
         return length;
       },
       remove: function(obj)
       {
         var index = this.indexOf(obj);
         this.splice(index, 1);
         var access = obj.url || obj.text;
         for (var i=0; i < this.table.length; i++)
         {
           var table = E(this.table[i].id);
           var element = table.querySelector("[data-access='"+access+"']");
           element.parentElement.removeChild(element);
         }
       }
     },
-    subscriptions: {
-      getArray: function(subscription)
-      {
-        var collArray = null;
-        if (subscription.isPopular)
-          collArray = collections.popular;
-        else if (subscription.isAdsType && subscription.disabled)
-          collArray = collections.allLangs;
-        else if (subscription.isAdsType && !subscription.disabled)
-          collArray = collections.langs;
-        else if (subscription.url == acceptableAdsUrl)
-          collArray = collections.acceptableAds;
-        else
-          collArray = collections.custom;
-        return collArray;
-      },
+    subscriptions:
+    {
       update: function(subscription)
       {
         var optionSubscription = this[subscription.url];
         if (optionSubscription)
           optionSubscription.disabled = subscription.disabled;
         else
         {
-          var collArray = this.getArray(subscription);
           optionObj.observe(subscription, ["disabled"]);
+          var collArray = null;
+          if (subscription.isPopular)
+            collArray = collections.popular;
+          else if (subscription.isAdsType && subscription.disabled)
+            collArray = collections.allLangs;
+          else if (subscription.isAdsType && !subscription.disabled)
+            collArray = collections.langs;
+          else if (subscription.url == acceptableAdsUrl)
+            collArray = collections.acceptableAds;
+          else
+            collArray = collections.custom;
+
           collArray.push = collections.push;
           collArray.push(subscription);
           this[subscription.url] = subscription;
         }
       },
       remove: function(subscription)
       {
         var optionSubscription = this[subscription.url];
           
         if (optionSubscription.isAdsType)
         {
           collections.langs.remove(optionSubscription);
           collections.allLangs.push(optionSubscription);
         }
         else if(optionSubscription.isPopular)
         {
           optionSubscription.disabled = true;
         }
         else
         {
           collections.custom.remove(optionSubscription);
         }
       },
       chboxListener: function(e)
       {
         e.preventDefault();
-        var target = e.target;
-        var isChecked = target.checked;
+        var isChecked = e.target.checked;
         var url = e.target.parentNode.dataset.access;
         if (!isChecked)
-          SendMessage.removeSubscription(url);
+          sendMessage.removeSubscription(url);
         else
-          SendMessage.addSubscription(url);
+          sendMessage.addSubscription(url);
       },
-      acceptableAdsListener: function(e)
+      toggleAcceptableAds: function(e)
       {
         e.preventDefault();
         var target = e.target;
         var isChecked = target.checked;
         var url = e.target.parentNode.dataset.access;
         
         if (isChecked)
-          SendMessage.addSubscription(url);
+          sendMessage.addSubscription(url);
         else
-          SendMessage.removeSubscription(url);
+          sendMessage.removeSubscription(url);
       },
       addBtnListener: function(e)
       {
         e.preventDefault();
         var url = this.parentNode.dataset.access;
-        SendMessage.addSubscription(url);
+        sendMessage.addSubscription(url);
       }
     },
     filters:
     {
       update: function(filter)
       {
         var optionFilter = this[filter.text];
         var match = filter.text.match(/^@@\|\|([^\/:]+)\^\$document$/);
         if (match && !optionFilter)
         {
           filter.title = match[1];
           collections.whitelist.push(filter);
           this[filter.text] = filter;
         }
         else
         {
           // TODO: add `filters[i].text` to list of custom filters
         }
       },
       remove: function(filter)
       {
         collections.whitelist.remove(filter);
       },
       deleteBtnClick: function()
       {
         var filter = this.parentNode.dataset.access;
-        SendMessage.removeFilter(filter);
+        sendMessage.removeFilter(filter);
       }
     },
     observe: function(obj, props)
     {
       props.forEach(function(property)
       {
-        obj["__"+property] = obj[property];
+        obj["$"+property] = obj[property];
         Object.defineProperty(obj, property,
         {
           get: function()
           {
-            return this["__"+property];
+            return this["$"+property];
           },
           set: function(value)
           {
-            this["__"+property] = value;
+            this["$"+property] = value;
             var access = obj.url || obj.text;
             var elements = document.querySelectorAll("[data-access='" + access + "']");
             for (var i=0; i < elements.length; i++)
             {
-              var elem = elements[i];
-              var control = elem.getElementsByClassName("control")[0];
-              if (control.tagName == "INPUT")
+              var element = elements[i];
+              var control = element.getElementsByClassName("control")[0];
+              if (control.localName == "input")
                 control.checked = !obj.disabled;
               if (obj.isAdsType)
               {
                 collections.allLangs.remove(obj);
                 collections.langs.push(obj);
               }
             }
           }
         });
       });
@@ skipping 32 matching lines @@
     {
       id: "all-lang-table", 
       template: "<button class='button-add control'><span>+add</span></button><span class='display'></span>",
       listener: optionObj.subscriptions.addBtnListener
     }
   ]);
   collections.acceptableAds = createCollArray([
     {
       id: "acceptableads-table", 
       template: "<input type='checkbox' class='control' /><span class='display'></span>",
-      listener: optionObj.subscriptions.acceptableAdsListener
+      listener: optionObj.subscriptions.toggleAcceptableAds
     }
   ]);
   collections.custom = createCollArray([
     {
       id: "custom-list-table", 
       template: "<input type='checkbox' class='control' /><span class='display'></span>",
       listener: optionObj.subscriptions.chboxListener
     }
   ]);
   collections.whitelist = createCollArray([
     {
       id: "whitelisting-table", 
       template: "<button class='delete control'></button><span class='display'></span>",
       listener: optionObj.filters.deleteBtnClick
     }
   ]);
   
-  var Recommendations =
-  {
-    load: function()
-    {
-      var request = new XMLHttpRequest();
-      request.open("GET", "subscriptions.xml", false);
-      request.onload = function()
-      {
-        var list = document.getElementById("subscriptionSelector");
-        var docElem = request.responseXML.documentElement;
-        var elements = docElem.getElementsByTagName("subscription");
-        for (var i = 0; i < elements.length; i++)
-        {
-          var element = elements[i];
-          var subscription = Object.create(null);
-          subscription.title = element.getAttribute("title");
-          subscription.url = element.getAttribute("url");
-          subscription.disabled = true;
-          var prefix = element.getAttribute("prefixes");
-          if (prefix)
-          {
-            subscription.isAdsType = true;
-            var prefix = element.getAttribute("prefixes").replace(/,/g, '_');
-            subscription.title = ext.i18n.getMessage("options_language_" + prefix);
-          }
-          else
-            subscription.title = element.getAttribute("specialization");
-          
-          if (element.getAttribute("popular"))
-            subscription.isPopular = true;
-          optionObj.subscriptions.update(subscription);
-        }
-      }.bind(this);
-      request.send();
-    }
-  };
+  function loadRecommendations()
+  {
+    var request = new XMLHttpRequest();
+    request.open("GET", "subscriptions.xml", false);
+    request.onload = function()
+    {
+      var list = document.getElementById("subscriptionSelector");
+      var docElem = request.responseXML.documentElement;
+      var elements = docElem.getElementsByTagName("subscription");
+      for (var i = 0; i < elements.length; i++)
+      {
+        var element = elements[i];
+        var subscription = Object.create(null);
+        subscription.title = element.getAttribute("title");
+        subscription.url = element.getAttribute("url");
+        subscription.disabled = true;
+        var prefix = element.getAttribute("prefixes");
+        if (prefix)
+        {
+          subscription.isAdsType = true;
+          var prefix = element.getAttribute("prefixes").replace(/,/g, "_");
+          subscription.title = ext.i18n.getMessage("options_language_" + prefix);
+        }
+        else
+          subscription.title = element.getAttribute("specialization");
+        
+        if (element.getAttribute("popular"))
+          subscription.isPopular = true;
+        optionObj.subscriptions.update(subscription);
+      }
+    };
+    request.send(null);
+  }
   
   function onDOMLoaded()
   {
     updateShareLink();
     populateLists();
     
     var tabList = document.querySelectorAll("#main-navigation-tabs li");
-    for (var i = 0; i < tabList.length; ++i)
-    {
-      tabList[i].addEventListener("click", function(ev)
-      {
-        document.body.dataset.tab = this.id.substr(4);
+    for (var i = 0; i < tabList.length; i++)
+    {
+      tabList[i].addEventListener("click", function(e)
+      {
+        document.body.dataset.tab = e.currentTarget.dataset.show;
       }, false);
     }
     
     var searchLanguage = function()
     {
-      var searchStyle = document.getElementById('search_style');

[Thomas Greiner, 2015/02/18 17:19:27]

You're using `document.getElementById()` here but `E()` a few lines below.
Again, watch out for inconsistencies.

-      var searchVal = this.value;
-      if (!searchVal)
+      var searchStyle = E('search_style');

[Thomas Greiner, 2015/02/20 14:02:07]

Review comments should not only be addressed for one specific part of the code
but if you know that an issue occurs in other places as well, please also fix
them there. Otherwise we'd end up with many duplicate comments about the same
issue.

In this case you have addressed the comment about replacing `"` instead of `'`
from
http://codereview.adblockplus.org/6088024630755328/diff2/5742506566221824:621...
as expected but didn't address the same issue elsewhere in the code, like in
this line.

[saroyanm, 2015/02/26 12:18:33]

Done.

+      if (!this.value)
         searchStyle.innerHTML = "";
       else
-        searchStyle.innerHTML = "#all-lang-table li:not([data-value*=\"" + this.value.toLowerCase() + "\"]) { display: none; }";
+        searchStyle.innerHTML = "#all-lang-table li:not([data-search*=\"" + this.value.toLowerCase() + "\"]) { display: none; }";
     };
     
     // Update version number in navigation sidebar
-    ext.backgroundPage.sendMessage({
+    ext.backgroundPage.sendMessage(

[Thomas Greiner, 2015/02/20 14:02:07]

It's still inconsistent. As I noted in a previous comment of mine, the "{" is in
its own line whereas the "}" is not. Since we don't have one style we always go
with, feel free to choose one of the following:

fn({foo: "bar"}, function(foo)
{
  return null;
});

or

fn({
 foo: "bar"
}, function(foo)
{
  return null;
});

or (less common in our code base)

fn(
  {
    foo: "bar"
  },
  function(foo)
  {
    return null;
  }
);

[saroyanm, 2015/02/26 12:18:33]

Used the one from your suggestion:
https://hg.adblockplus.org/adblockpluschrome/file/72785e0948d0/block.js#l42 

+    {
       method: "app.get",
       what: "addonVersion"
-      }, function(addonVersion)
-      {
-        E("abp-version").textContent = addonVersion;
+    }, function(addonVersion)
+    {
+      E("abp-version").textContent = addonVersion;
     });
     
     var whitelistDomainBtnClick = function()
     {
-      var domain = E("whitelisting-textbox").value;
-      if (domain)
-        SendMessage.addWhitelistedDomain(domain);
-      E("whitelisting-textbox").value = "";
+      var domain = E("whitelisting-textbox");
+      if (domain.value)
+        SendMessage.addWhitelistedDomain(domain.value);
+      domain.value = "";
     };
     
     var placeholderValue  = ext.i18n.getMessage("options_modal_language_find");
     E("find-language").setAttribute("placeholder", placeholderValue);
     setLinks("block-element-explanation", "#");
     
     E("add-blocking-list").addEventListener("click", function()
     {
       openModal("customlist");
     }, false);
@@ skipping 49 matching lines @@
         {
           type: "filters.get",
           subscriptionUrl: subscriptions[i].url
         }, function(filters)
         {
           for (var i = 0; i < filters.length; i++)
             optionObj.filters.update(filters[i]);
         });
       }
     });
-    Recommendations.load();
-    SendMessage.getAcceptableAdsURL(function(url)
+    loadRecommendations();
+    sendMessage.getAcceptableAdsURL(function(url)
     {
       acceptableAdsUrl = url;
       var subscription = Object.create(null);
       subscription.url = acceptableAdsUrl;
       subscription.disabled = true;
       subscription.title = ext.i18n.getMessage("options_acceptableAds_description");
       optionObj.subscriptions.update(subscription);
       
       // Load user subscriptions
       ext.backgroundPage.sendMessage(
       {
         type: "subscriptions.get",
         downloadable: true
       }, function(subscriptions)
       {
         for (var i = 0; i < subscriptions.length; i++)
-          MessageListeners.onSubscriptionMessage("added", subscriptions[i]);
+          messageListeners.onSubscriptionMessage("added", subscriptions[i]);
       });
     });
   }
   
   function editOwnRulsBtnClick()
   {
     
   }
   
-  var SendMessage =
+  var sendMessage =
   {
     getAcceptableAdsURL: function(callback)
     {
       ext.backgroundPage.sendMessage(
       {
         type: "prefs.get",
         key: "subscriptions_exceptionsurl"
       }, function(value)
       {
-        SendMessage.getAcceptableAdsURL = function(callback)
+        sendMessage.getAcceptableAdsURL = function(callback)
         {
           callback(value);
         }
-        SendMessage.getAcceptableAdsURL(callback);
+        sendMessage.getAcceptableAdsURL(callback);
       });
     },
     addSubscription: function(url, title, homepage)
     {
       var message = {
         type: "subscriptions.add",
         url: url
       };
       if (title)
         message.title = title;
@@ skipping 21 matching lines @@
     removeFilter: function(filter)
     {
       ext.backgroundPage.sendMessage(
       {
         type: "filters.remove",
         text: filter
       });
     }
   };
 
-  var MessageListeners =
+  var messageListeners =
   {
     onFilterMessage: function(action, filter)
     {
       switch (action)
       {
         case "added":
           optionObj.filters.update(filter);
           break;
         case "loaded":
           populateLists();
@@ skipping 21 matching lines @@
             subscription.disabled = true;
             optionObj.subscriptions.update(subscription);
           }
           else
             optionObj.subscriptions.remove(subscription);
           break;
         case "title":
           // TODO: NYI
           break;
       }
-    },
-    showAddSubscriptionDialog: function(subscription)
-    {
-      E("blockingList-textbox").value = subscription.url;
-      openModal("customlist");
     }
   };
+
+  function showAddSubscriptionDialog(subscription)
+  {
+    E("blockingList-textbox").value = subscription.url;
+    openModal("customlist");
+  }
   
   function updateShareLink()
   {
     ext.backgroundPage.sendMessage(
     {
       type: "filters.blocked",
       url: "https://platform.twitter.com/widgets/",
       requestType: "SCRIPT",
       docDomain: "adblockplus.org",
       thirdParty: true
-    }, function(blocked)
+    },
+    function(blocked)
     {
       // TODO: modify "share" link accordingly
     });
   }
   
   function getDocLink(link, callback)
   {
     ext.backgroundPage.sendMessage(
     {
       type: "app.get",
@@ skipping 31 matching lines @@
   {
     return document.getElementById(id);
   }
   
   ext.onMessage.addListener(function(message)
   {
     switch (message.type)
     {
       case "app.listen":
         if (message.action == "addSubscription")
-          MessageListeners.showAddSubscriptionDialog(message.args[0]);
+          messageListeners.showAddSubscriptionDialog(message.args[0]);
         break;
       case "filters.listen":
-        MessageListeners.onFilterMessage(message.action, message.args[0]);
+        messageListeners.onFilterMessage(message.action, message.args[0]);
         break;
       case "subscriptions.listen":
-        MessageListeners.onSubscriptionMessage(message.action, message.args[0]);
+        messageListeners.onSubscriptionMessage(message.action, message.args[0]);
         break;
     }
   });
   
   ext.backgroundPage.sendMessage(
   {
     type: "app.listen",
     filter: ["addSubscription"]
   });
   ext.backgroundPage.sendMessage(
   {
     type: "filters.listen",
     filter: ["added", "loaded", "removed"]
   });
   ext.backgroundPage.sendMessage(
   {
     type: "subscriptions.listen",
     filter: ["added", "disabled", "homepage", "removed", "title"]
   });
   
   window.addEventListener("DOMContentLoaded", onDOMLoaded, false);
 })();