First-run page (revisited)

chrome/content/ui/firstRun.js

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2013 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 48 matching lines @@
     shade = E("shade");
     shade.addEventListener("mouseover", scrollPage, false);
     shade.addEventListener("mouseout", stopScroll, false);
 
     // Set up typo feature
     if (require("typoBootstrap"))
     {
       var featureTypo = E("feature-typo");
       featureTypo.removeAttribute("hidden");
       
-      var toggleTypo = E("toggle-typo");
       updateToggleButton("typo", Prefs.correctTypos);
 
       var listener = function(name)
       {
         if (name == "correctTypos")
           updateToggleButton("typo", Prefs.correctTypos);
       }
       Prefs.addListener(listener);
       window.addEventListener("unload", function(event)
       {
         Prefs.removeListener(listener);
       }, false);
 
-      toggleTypo.addEventListener("click", function(event)
-      {
-        toggleTypoCorrectionEnabled();
-      }, false);

[Wladimir Palant, 2013/05/28 13:39:01]

If you want a function then please don't wrap it in a pointless callback:

  E("toggle-typo").addEventListener("click", toggleTypoCorrectionEnabled,
false);

Note that you can remove the toggleTypo variable, IMHO this is the only place it
is used.

+      E("toggle-typo").addEventListener("click", toggleTypoCorrectionEnabled, false);
     }
 
     // Set up feature buttons linked to subscriptions
     featureSubscriptions.forEach(setToggleSubscriptionButton);
     var filterListener = function(action)
     {
-      for (var i = 0; i < featureSubscriptions.length; i++)
-      {
-        if (/^(filter|subscription)\.(added|removed|disabled)$/.test(action))

[Wladimir Palant, 2013/05/28 13:39:01]

This regexp should be: /^subscription\.(added|removed|disabled)$/ (we don't care
about filter changes). Also, I don't think you want to run that loop for each
action - please check the action first, go through the loop then only if we have
a match.

+      if (/^subscription\.(added|removed|disabled)$/.test(action))
+      {
+        for (var i = 0; i < featureSubscriptions.length; i++)
         {
           var featureSubscription = featureSubscriptions[i];
           updateToggleButton(featureSubscription.feature, isSubscriptionEnabled(featureSubscription));
         }
       }
     }
     FilterNotifier.addListener(filterListener);
     window.addEventListener("unload", function(event)
     {
       FilterNotifier.removeListener(filterListener);
@@ skipping 168 matching lines @@
 
   function updateToggleButton(feature, isEnabled)
   {
     var button = E("toggle-" + feature);
     button.className = isEnabled ? "disable" : "enable";
     button.textContent = i18n.getMessage(isEnabled ? "firstRun_action_disable" : "firstRun_action_enable");
   }
 
   document.addEventListener("DOMContentLoaded", onDOMLoaded, false);
 })();