Fix the problem that releasing a key after pressing multiple modifiers corrupts keyboard overlay

chrome/browser/resources/keyboard_overlay.js

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 var BASE_KEYBOARD = {
   top: 0,
   left: 0,
   width: 1237,
   height: 514
 };
@@ skipping 102 matching lines @@
   return result;
 }
 
 /**
  * Returns a list of modifiers from the key event.
  */
 function getModifiers(e) {
   if (!e) {
     return [];
   }
+  var isKeyDown = (e.type == 'keydown');
+  var modifierToKeyCode = {'SHIFT': 16, 'CTRL': 17, 'ALT': 18};
   var modifiers = [];

[mazda, 2011/04/18 11:00:20]

Please move the declaration right before line 137.

-  if (e.keyCode == 16 || e.shiftKey) {
-    modifiers.push('SHIFT');
+  var modifier;

[mazda, 2011/04/18 11:00:20]

How about moving modifier to for-loop?

for (var modifier in modifierToKeyCode) {
..

for (var modifier in pressed) {
...

+  var pressed = {'SHIFT': e.shiftKey, 'CTRL': e.ctrlKey, 'ALT': e.altKey};
+  // if e.keyCode is one of Shift, Ctrl and Alt, pressed should
+  // be changed because the key currently pressed
+  // does not affect the values of e.shiftKey, e.ctrlKey and e.altKey
+  for (modifier in modifierToKeyCode) {
+    if (e.keyCode == modifierToKeyCode[modifier]) {
+      pressed[modifier] = isKeyDown;
+    }
   }
-  if (e.keyCode == 17 || e.ctrlKey) {
-    modifiers.push('CTRL');
-  }
-  if (e.keyCode == 18 || e.altKey) {
-    modifiers.push('ALT');
+  // make the result array
+  for (modifier in pressed) {
+    if (pressed[modifier]) {
+      modifiers.push(modifier);
+    }
   }
   return modifiers.sort();
 }
 
 /**
  * Returns an ID of the key.
  */
 function keyId(identifier, i) {
   return identifier + '-key-' + i;
 }
@@ skipping 116 matching lines @@
     if (pos && pos.length > 0) {
       chars.push(hex2char(pos));
     }
   }
   return chars.join(' ');
 }
 
 /**
  * Updates the whole keyboard.
  */
-function update(e) {
-  var modifiers = getModifiers(e);
+function update(modifiers) {
   var instructions = document.getElementById('instructions');
   if (modifiers.length == 0) {
     instructions.style.visibility = 'visible';
   } else {
     instructions.style.visibility = 'hidden';
   }
 
   var keyboardGlyphData = getKeyboardGlyphData();
   var shortcutData = getShortcutData();
   var layout = getLayouts()[keyboardGlyphData.layoutName];
@@ skipping 38 matching lines @@
         keyText.style.textAlign = format;
       }
     }
   }
 }
 
 /**
  * A callback furnction for the onkeydown event.
  */
 function keydown(e) {
+  var modifiers = getModifiers(e);
   if (!getKeyboardOverlayId()) {
     return;
   }
-  update(e);
+  update(modifiers);
 }
 
 /**
  * A callback furnction for the onkeyup event.
  */
 function keyup(e) {

[mazda, 2011/04/18 11:00:20]

How about merging this function into keydown?

+  var modifiers = getModifiers(e);
   if (!getKeyboardOverlayId()) {
     return;
   }
-  update();
+  update(modifiers);
 }
 
 /**
  * Initializes the layout of the keys.
  */
 function initLayout() {
   var layout = getLayouts()[getKeyboardGlyphData().layoutName];
   var keyboard = document.body;
   var minX = window.innerWidth;
   var maxX = 0;
@@ skipping 89 matching lines @@
     keyboardOverlayId = overlayId;
   }
   while(document.body.firstChild) {
     document.body.removeChild(document.body.firstChild);
   }
   initLayout();
   update();
 }
 
 document.addEventListener('DOMContentLoaded', init);