Issue 1434 - Remove some non-standard JS usage from buildtools.

lib/keySelector.js

 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 Cu.import("resource://gre/modules/Services.jsm");
 
 let validModifiers =
 {
   ACCEL: null,
   CTRL: "control",
@@ skipping 75 matching lines @@
       if (keyChar && keyChar.length == 1)
         keyData.char = keyChar.toUpperCase();
 
       let keyCode = key.getAttribute("keycode");
       if (keyCode && "DOM_" + keyCode.toUpperCase() in Ci.nsIDOMKeyEvent)
         keyData.code = Ci.nsIDOMKeyEvent["DOM_" + keyCode.toUpperCase()];
 
       if (!keyData.char && !keyData.code)
         continue;
 
-      let modifiers = key.getAttribute("modifiers");
-      modifiers = modifiers ? modifiers.toUpperCase().match(/\w+/g) : null;

[Wladimir Palant, 2014/11/17 20:05:17]

If you are doing it this way, why not use [] instead of null? Then you don't
need the if block below any more.

+      let keyModifiers = key.getAttribute("modifiers");
+      let modifiers = keyModifiers ? keyModifiers.toUpperCase().match(/\w+/g) : null;

[Wladimir Palant, 2014/12/09 21:24:49]

Comment from previous round not addressed or replied to:

On 2014/11/17 20:05:17, Wladimir Palant wrote:
If you are doing it this way, why not use [] instead of null? Then you don't
need the if block below any more.

       if (modifiers)
       {
         for (let modifier of modifiers)
         {
           if (modifier in validModifiers)
             keyData[validModifiers[modifier]] = true;
         }
       }
 
       let canonical = [keyData.shift, keyData.meta, keyData.alt, keyData.control, keyData.char || keyData.code].join(" ");
       this._existingShortcuts[canonical] = true;
     }
   },
 
   /**
    * Selects a keyboard shortcut variant that isn't already taken,
    * parses it into an object.
    */
   selectKey: function(/**String*/ variants) /**Object*/
   {
-    variants = variants.split(/\s*,\s*/);
-    if (!variants)

[Wladimir Palant, 2014/11/17 20:05:17]

This condition can never be true - String.split() always returns an array with
at least one entry.

-      return null;
-
-    for (let variant of variants)
+    for (let variant of variants.split(/\s*,\s*/))
     {
       if (!variant)
         continue;
 
       let keyData =
       {
         shift: false,
         meta: false,
         alt: false,
         control: false,
         char: null,
         code: null,
         codeName: null
       };
 
-      variant = variant.toUpperCase().split(/\s+/);
-      if (!variant)

[Wladimir Palant, 2014/11/17 20:05:17]

Same here, this if block is pointless.

-        continue;
-
-      for (let part of variant)
+      for (let part of variant.toUpperCase().split(/\s+/))
       {
         if (part in validModifiers)
           keyData[validModifiers[part]] = true;
         else if (part.length == 1)
           keyData.char = part;
         else if ("DOM_VK_" + part in Ci.nsIDOMKeyEvent)
         {
           keyData.code = Ci.nsIDOMKeyEvent["DOM_VK_" + part];
           keyData.codeName = "VK_" + part;
         }
@@ skipping 67 matching lines @@
     return false;
   if (key.meta != event.metaKey || key.control != event.ctrlKey)
     return false;
 
   if (key.char && event.charCode && String.fromCharCode(event.charCode).toUpperCase() == key.char)
     return true;
   if (key.code && event.keyCode && event.keyCode == key.code)
     return true;
   return false;
 };