Enable user assign media keys in chrome://extensions page.

chrome/browser/resources/extensions/extension_command_list.js

Index: chrome/browser/resources/extensions/extension_command_list.js
diff --git a/chrome/browser/resources/extensions/extension_command_list.js b/chrome/browser/resources/extensions/extension_command_list.js
index d730c9d841ec054f19677f88cb94b7f68d70397b..0cec00516fe88ecfff57ca74bac451112ad2301c 100644
--- a/chrome/browser/resources/extensions/extension_command_list.js
+++ b/chrome/browser/resources/extensions/extension_command_list.js
@@ -26,6 +26,10 @@ cr.define('options', function() {
   /** @const */ var keyRight = 39;
   /** @const */ var keyTab = 9;
   /** @const */ var keyUp = 38;
+  /** @const */ var keyMediaNextTrack = 176;
+  /** @const */ var keyMediaPrevTrack = 177;
+  /** @const */ var keyMediaStop = 178;
+  /** @const */ var keyMediaPlayPause = 179;

[Finnur, 2013/09/04 11:28:12]

The list is alphabetical so this should not be at the bottom.

[zhchbin, 2013/09/04 12:50:03]

Done.

 
   /**
    * Returns whether the passed in |keyCode| is a valid extension command
@@ -48,6 +52,10 @@ cr.define('options', function() {
            keyCode == keyRight ||
            keyCode == keyTab ||
            keyCode == keyUp ||
+           keyCode == keyMediaNextTrack ||
+           keyCode == keyMediaPrevTrack ||
+           keyCode == keyMediaStop ||
+           keyCode == keyMediaPlayPause ||

[Finnur, 2013/09/04 11:28:12]

Same here.

[zhchbin, 2013/09/04 12:50:03]

Done.

            (keyCode >= 'A'.charCodeAt(0) && keyCode <= 'Z'.charCodeAt(0)) ||
            (keyCode >= '0'.charCodeAt(0) && keyCode <= '9'.charCodeAt(0));
   }
@@ -102,6 +110,14 @@ cr.define('options', function() {
             output += 'Tab'; break;
           case keyUp:
             output += 'Up'; break;
+          case keyMediaNextTrack:
+            output += 'MediaNextTrack'; break;
+          case keyMediaPrevTrack:
+            output += 'MediaPrevTrack'; break;
+          case keyMediaStop:
+            output += 'MediaStop'; break;
+          case keyMediaPlayPause:
+            output += 'MediaPlayPause'; break;

[Finnur, 2013/09/04 11:28:12]

Same here.

[zhchbin, 2013/09/04 12:50:03]

Done.

         }
       }
     }
@@ -109,6 +125,20 @@ cr.define('options', function() {
     return output;
   }
 
+  /**
+   * Returns whether the passed in |keyCode| is a keycode of media key. This is
+   * restricted to 'MediaNextTrack', 'MediaPrevTrack', 'MediaStop',
+   * 'MediaPlayPause' at the moment.
+   * @param {int} keyCode The keycode to consider.
+   * @return {boolean} Returns whether the keycode is media key.
+   */
+  function isMediaKey(keyCode) {

[Finnur, 2013/09/04 11:28:12]

I'd like to make this function a bit more general, so that if we add more keys
it is easy to just add to this matrix. It also helps make the code below a bit
more general and readable. So, I would do this:

function modifiers(keyCode) {
  switch (keycode) {
   case keyMediaNextTrack:
   case keyMediaPrevTrack:
   case keyMediaStop:
   case keyMediaPlayPause:
     return areNotAllowed;
   default:
     return areRequired;
  }
}

areAllowed and areRequired can be enums (consts) at the top.

Remember to fix the comment for this function too.

[zhchbin, 2013/09/04 12:50:03]

Done with using a enum:

  /**
   * Enum for whether we require modifiers of a keycode.
   * @enum {number}
   */
  var Modifiers = {
    ARE_NOT_REQUIRED: 0,
    ARE_REQUIRED: 1
  };

+    return keyCode == keyMediaNextTrack ||
+           keyCode == keyMediaPrevTrack ||
+           keyCode == keyMediaStop ||
+           keyCode == keyMediaPlayPause;
+  }
+
   ExtensionCommandList.prototype = {
     __proto__: HTMLDivElement.prototype,
 
@@ -323,8 +353,18 @@ cr.define('options', function() {
       event.preventDefault();
       event.stopPropagation();
 
-      if (!event.ctrlKey && !event.altKey && (!cr.isMac || !event.metaKey))
-        return;  // Ctrl or Alt is a must (or Cmd on Mac).
+      if (!event.ctrlKey && !event.altKey && (!cr.isMac || !event.metaKey) &&
+          !isMediaKey(event.keyCode)) {
+        // Ctrl or Alt is a must (or Cmd on Mac), except for Media Keys.
+        return;
+      }
+
+      // Modifiers can not be used in combination with the Media Keys.
+      if (isMediaKey(event.keyCode)) {
+        if (event.ctrlKey || event.altKey || (cr.isMac && event.metaKey) ||
+            event.shiftKey)
+          return;
+      }

[Finnur, 2013/09/04 11:28:12]

I think this is a bit more readable:

function hasModifier(event, countShiftAsModifier) {
  return event.ctrlKey || event.altKey || (cr.isMac && event.metaKey) ||
         (countShiftAsModifier && event.shiftKey);
}

if (modifiers(event.keyCode) == areRequired && !hasModifier(event, false)) {
  // Ctrl or Alt (or Cmd on Mac) is a must for most shortcuts.
  return;
}

if (modifiers(event.keyCode) == areNotAllowed && hasModifier(event, true))
  return;

You can even (for extra readability) create an enum for "ignoreShift" and "all"
instead of using a bool for countShiftAsModifier.

[zhchbin, 2013/09/04 12:50:03]

Done with still using a boolean for countShiftAsModifier.

 
       var shortcutNode = this.capturingElement_;
       var keystroke = keystrokeToString(event);