Bug 111112 – [chromium] Keydown event for 'shift+alt' returns win keycode instead of 'alt'

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[chandra shekar vallala]

Steps to reproduce the problem:
1. Create an onkeydown event handler for an html input element
2. Press shift
3. While holding shift, press alt

What is the expected behavior?
The onkeydown event for the alt keypress should contain the following:

altKey: true
which: 18

What went wrong?
The onkeydown event for the alt keypress contains the following:

altKey: false
which: 91

Ref: http://code.google.com/p/chromium/issues/detail?id=177100

[chandra shekar vallala]

Created attachment 190807 [details]
patch

[Tony Chang]

Comment on attachment 190807 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=190807&action=review

> Source/WebCore/ChangeLog:12
> +        Added Manual Test : ManualTests/shift-alt-key-event.html
> +        Try press Shift then alt key. The test passes if the shiftKey, altKey values
> +        of JSKeyEvent are true and keycode/which is 18.

Did you try writing a layout test using eventSender?  It would be nice to know why that doesn't work.

Also, did you test this in Firefox on Linux?  What does it send?  If we are matching Firefox, please include that in the ChangeLog.

[chandra shekar vallala]

(In reply to comment #2)
> (From update of attachment 190807 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=190807&action=review
> 
> > Source/WebCore/ChangeLog:12
> > +        Added Manual Test : ManualTests/shift-alt-key-event.html
> > +        Try press Shift then alt key. The test passes if the shiftKey, altKey values
> > +        of JSKeyEvent are true and keycode/which is 18.
> 
> Did you try writing a layout test using eventSender?  It would be nice to know why that doesn't work.
> 
Yes, I tried writing a layout test using eventSender. It didn't work since EventeSender::keyDown is unable to simulate a WebKeyboardEvent with keycode = GDK_Meta, modifier=Shift|Alt

> Also, did you test this in Firefox on Linux?  What does it send?  If we are matching Firefox, please include that in the ChangeLog.
Yes, I tried in Firefox, Opera and its working fine. Firefox do not map the keycode to Windows keycode directly, They find the right keycode for a GdkEventKey if the key event has a modifier.

Chrome key code conversion does the same in above case. 
ui/base/keycodes/keyboard_code_conversion_x.cc:791

[chandra shekar vallala]

Created attachment 191592 [details]
patch

Added Firefox behaviour in change log

[Tony Chang]

Comment on attachment 191592 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=191592&action=review

Looks like you also need to rebase your patch to ToT.

> ManualTests/shift-alt-key-event.html:8
> +function myFunction(event)

Please use a better name than myFunction.  How about keyDown?

> ManualTests/shift-alt-key-event.html:14
> +    if(event.shiftKey == true && event.altKey == true && event.which == 18) {

Nit: Put a space after if.

> ManualTests/shift-alt-key-event.html:16
> +    }else {

Nit: Space between } and else.

> ManualTests/shift-alt-key-event.html:19
> +    output.innerHTML=text;

Nit: Spaces around =.

> ManualTests/shift-alt-key-event.html:25
> +<p>Test for <a href="http://bugs.webkit.org/show_bug.cgi?id=?????">bug ?????</a>:

Please fill in the bug number or just remove this text.

[chandra shekar vallala]

Created attachment 191610 [details]
updated patch

Done.

[chandra shekar vallala]

Created attachment 192288 [details]
rebased the patch

Hi Tony,

Please look at the rebased patch.

[WebKit Review Bot]

Comment on attachment 192288 [details]
rebased the patch

Clearing flags on attachment: 192288

Committed r145276: <http://trac.webkit.org/changeset/145276>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.