Make touch event handling more consistent with Android keyboard.

Make touch event handling more consistent with Android keyboard.

The notable change is that now characters are input in the timing of touchend
instead of touchstart.
This is necessary for showing popup keyboard by long-press without inputting
the chracacter of the pressed key.
This CL also changes the key to send a pair of keydown and keyup on touchend,
which is consistent with Android keyboard behavior.


BUG=none
TEST=Manually on the device


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=97109

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

http://codereview.chromium.org/7606030/diff/1/chrome/browser/resources/keyboa...
File chrome/browser/resources/keyboard/common.js (right):

http://codereview.chromium.org/7606030/diff/1/chrome/browser/resources/keyboa...
chrome/browser/resources/keyboard/common.js:95: if (!type || type == 'keydown'
|| type == 'keyup') {
This if and the type parameter are now useless: type can only take on these
three values.

http://codereview.chromium.org/7606030/diff/1/chrome/browser/resources/keyboa...
chrome/browser/resources/keyboard/common.js:154: function
setupKeyEventHandlers(key, element, keyDownHandler, opt_keyUpHandler) {
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.

http://codereview.chromium.org/7606030/diff/1/chrome/browser/resources/keyboa...
chrome/browser/resources/keyboard/common.js:404: sendKeyFunction(this.keyId_,
'keydown'));
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]

Thank you for the review.
Please take another look.

http://codereview.chromium.org/7606030/diff/1/chrome/browser/resources/keyboa...
File chrome/browser/resources/keyboard/common.js (right):

http://codereview.chromium.org/7606030/diff/1/chrome/browser/resources/keyboa...
chrome/browser/resources/keyboard/common.js:95: if (!type || type == 'keydown'
|| type == 'keyup') {
On 2011/08/12 18:07:44, bryeung wrote:
> This if and the type parameter are now useless: type can only take on these
> three values.

Done.

http://codereview.chromium.org/7606030/diff/1/chrome/browser/resources/keyboa...
chrome/browser/resources/keyboard/common.js:154: function
setupKeyEventHandlers(key, element, keyDownHandler, opt_keyUpHandler) {
On 2011/08/12 18:07:44, bryeung wrote:
> 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.

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.

http://codereview.chromium.org/7606030/diff/1/chrome/browser/resources/keyboa...
chrome/browser/resources/keyboard/common.js:404: sendKeyFunction(this.keyId_,
'keydown'));
On 2011/08/12 18:07:44, bryeung wrote:
> 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.

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.

[bryeung, 2011-08-16 18:18:02]

LGTM

[commit-bot: I haz the power, 2011-08-17 07:19:00]

Change committed as 97109