Make touch event handling more consistent with Android keyboard.

chrome/browser/resources/keyboard/common.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.
 
 /**
  * @fileoverview A simple virtual keyboard implementation.
  */
 
 var KEY_MODE = 'key';
 var SHIFT_MODE = 'shift';
@@ skipping 74 matching lines @@
 
 /**
  * Send the given key to chrome, via the experimental extension API.
  * @param {string} key The key to send.
  * @param {string=} opt_type The type of event to send (keydown or keyup). If
  *     omitted send both keydown and keyup events.
  * @return {void}
  */
 function sendKey(key, type) {
   var keyEvent = {'keyIdentifier': key};
-  if (!type || type == 'keydown') {
+  if (!type || type == 'keydown' || type == 'keyup') {

[bryeung, 2011/08/12 18:07:44]

This if and the type parameter are now useless: type can only take on these
three values.

[mazda, 2011/08/16 09:45:50]

Done.

     // A keypress event is automatically generated for printable characters
     // immediately following the keydown event.
-    keyEvent.type = 'keydown';
     if (chrome.experimental) {
+      keyEvent.type = 'keydown';
+      chrome.experimental.input.sendKeyboardEvent(keyEvent);
+      keyEvent.type = 'keyup';
       chrome.experimental.input.sendKeyboardEvent(keyEvent);
     }
     // Exit shift mode after pressing any key but space.
     if (currentMode == SHIFT_MODE && key != 'Spacebar') {
       transitionMode(SHIFT_MODE);
     }
     // Enter shift mode after typing a period for a new sentence.
     if (currentMode != SHIFT_MODE && key == '.') {
       transitionMode(SHIFT_MODE);
     }
   }
-  if (!type || type == 'keyup') {
-    keyEvent.type = 'keyup';
-    if (chrome.experimental) {
-      chrome.experimental.input.sendKeyboardEvent(keyEvent);
-    }
-  }
 }
 
 /**
  * Add a child div element that represents the content of the given element.
  * A child div element that represents a text content is added if
  * opt_textContent is given. Otherwise a child element that represents an image
  * content is added. If the given element already has a child, the child element
  * is modified.
  * @param {Element} element The DOM Element to which the content is added.
  * @param {string} opt_textContent The text to be inserted.
@@ skipping 21 matching lines @@
 /**
  * Set up the event handlers necessary to respond to mouse and touch events on
  * the virtual keyboard.
  * @param {BaseKey} key The BaseKey object corresponding to this key.
  * @param {Element} element The top-level DOM Element to set event handlers on.
  * @param {function()} keyDownHandler The event handler called when the key is
  *     pressed. This will be called repeatedly when holding a repeating key.
  * @param {function()=} opt_keyUpHandler The event handler called when the key
  *     is released. This is only called once per actual key press.
  */
 function setupKeyEventHandlers(key, element, keyDownHandler, opt_keyUpHandler) {

[bryeung, 2011/08/12 18:07:44]

This should no longer accept two handlers, as the keyup and keydown are now
always sent together.

Since SvgKey is the only one that needs to potentially act on keydown, you could
make a helper function to setupKeyEventHandlers that SvgKey calls into directly.

e.g.

function setupKeyEventHandlersHelper(key, element, down_handler, up_handler) {
  // contents of current setupKeyEventHandlers
}
function setupKeyEventHandlers(key, element, handler) {
  setupKeyEventHandlersHelper(key, element, null, handler);
}

This will make the common case much easier to understand (you won't be passing a
mysterious "null" all the time), and doesn't duplicate any code for SvgKey.

[mazda, 2011/08/16 09:45:50]

Actually SymbolKey, ShfitKey and HideKeybaordKey also act on keydown event, but
I agree with the idea of removing mysterious "null".
How about making setup functions for keydown and keyup respectively.
i.e.

function setupKeyDownEventHandler(key, element, handler) {
  setupKeyEventHandlersHelper(key, element, handler, null);
}

function setupKeyUpEventHandler(key, element, handler) {
  setupKeyEventHandlersHelper(key, element, null, handler);
}

I modified the code to use these functions.

   /**
    * Handle a key down event on the virtual key.
    * @param {UIEvent} evt The UI event which triggered the key down.
    */
   var downHandler = function(evt) {
     // Don't process a key down if the key is already down.
     if (key.pressed) {
       return;
     }
     key.pressed = true;
@@ skipping 35 matching lines @@
     if (repeatKey.key == key) {
       repeatKey.cancel();
     }
 
     if (opt_keyUpHandler) {
       opt_keyUpHandler();
     }
     evt.preventDefault();
   };
 
+  var outHandler = function(evt) {
+    // Reset key press state if the point goes out of the element.
+    key.pressed = false;
+  }
+
   // Setup mouse event handlers.
   element.addEventListener('mousedown', downHandler);
   element.addEventListener('mouseup', upHandler);
-  element.addEventListener('mouseout', upHandler);
+  element.addEventListener('mouseout', outHandler);
 
   // Setup touch handlers.
   element.addEventListener('touchstart', downHandler);
   element.addEventListener('touchend', upHandler);
+  // TODO(mazda): Add a handler for touchleave once Webkit supports it.
+  // element.addEventListener('touchleave', outHandler);
 }
 
 /**
  * Create closure for the sendKey function.
  * @param {string} key The key paramater to sendKey.
  * @param {string=} type The type parameter to sendKey.
  * @return {function()} A function which calls sendKey(key, type).
  */
 function sendKeyFunction(key, type) {
   return function() {
@@ skipping 123 matching lines @@
   /** @inheritDoc */
   makeDOM: function(mode) {
     if (!this.modes_[mode]) {
       return null;
     }
 
     this.modeElements_[mode] = document.createElement('div');
     this.modeElements_[mode].className = 'key';
     addContent(this.modeElements_[mode], this.modes_[mode].display);
 
-    setupKeyEventHandlers(this, this.modeElements_[mode],
-        sendKeyFunction(this.modes_[mode].keyIdentifier, 'keydown'),
+    setupKeyEventHandlers(this, this.modeElements_[mode], null,
         sendKeyFunction(this.modes_[mode].keyIdentifier, 'keyup'));
 
     return this.modeElements_[mode];
   }
 };
 
 /**
  * A key which displays an SVG image.
  * @param {string} className The class that provides the image.
  * @param {string} keyId The key identifier for the key.
@@ skipping 12 matching lines @@
 SvgKey.prototype = {
   __proto__: BaseKey.prototype,
 
   /** @inheritDoc */
   makeDOM: function(mode) {
     this.modeElements_[mode] = document.createElement('div');
     this.modeElements_[mode].className = 'key';
     this.modeElements_[mode].classList.add(this.className_);
     addContent(this.modeElements_[mode]);
 
-    setupKeyEventHandlers(this, this.modeElements_[mode],
-        sendKeyFunction(this.keyId_, 'keydown'),
-        sendKeyFunction(this.keyId_, 'keyup'));
+    if (this.repeat_) {
+      // send the key event on key down if key repeat is enabled
+      setupKeyEventHandlers(this, this.modeElements_[mode],
+          sendKeyFunction(this.keyId_, 'keydown'));

[bryeung, 2011/08/12 18:07:44]

Are you sure this behaviour matches Android?

Now, you'll be sending keydown and keyup messages for every repeat character.  I
thought that it only sent one keyup.

[mazda, 2011/08/16 09:45:50]

Yes, I checked that the delete key on Android keyboard sends both keydown and
keyup events per one character deletion while the key is touched. That's
different from the behavior of physical keyboards.
The virtual keyboard on iPad also behaves the same way.

+    } else {
+      setupKeyEventHandlers(this, this.modeElements_[mode], null,
+          sendKeyFunction(this.keyId_, 'keyup'));
+    }
 
     return this.modeElements_[mode];
   }
 };
 
 /**
  * A Key that remains the same through all modes.
  * @param {string} content The display text for the key.
  * @param {string} keyId The key identifier for the key.
  * @constructor
@@ skipping 10 matching lines @@
 SpecialKey.prototype = {
   __proto__: BaseKey.prototype,
 
   /** @inheritDoc */
   makeDOM: function(mode) {
     this.modeElements_[mode] = document.createElement('div');
     this.modeElements_[mode].className = 'key';
     this.modeElements_[mode].classList.add(this.className_);
     addContent(this.modeElements_[mode], this.content_);
 
-    setupKeyEventHandlers(this, this.modeElements_[mode],
-        sendKeyFunction(this.keyId_, 'keydown'),
+    setupKeyEventHandlers(this, this.modeElements_[mode], null,
         sendKeyFunction(this.keyId_, 'keyup'));
 
     return this.modeElements_[mode];
   }
 };
 
 /**
  * A shift key.
  * @constructor
  * @extends {BaseKey}
@@ skipping 87 matching lines @@
 
 DotComKey.prototype = {
   __proto__: BaseKey.prototype,
 
   /** @inheritDoc */
   makeDOM: function(mode) {
     this.modeElements_[mode] = document.createElement('div');
     this.modeElements_[mode].className = 'key com';
     addContent(this.modeElements_[mode], '.com');
 
-    setupKeyEventHandlers(this, this.modeElements_[mode],
+    setupKeyEventHandlers(this, this.modeElements_[mode], null,
         function() {
           sendKey('.');
           sendKey('c');
           sendKey('o');
           sendKey('m');
         });
 
     return this.modeElements_[mode];
   }
 };
@@ skipping 84 matching lines @@
    * @param {string} mode The mode to show.
    * @return {void}
    */
   showMode: function(mode) {
     for (var i = 0; i < MODES.length; ++i) {
       this.modeElements_[MODES[i]].style.display = 'none';
     }
     this.modeElements_[mode].style.display = '-webkit-box';
   },
 };