Bug 89637 - [Chromium] keyDown event in JavaScript: event.shiftKey, event.ctrlKey and event.altKey are always false
Summary: [Chromium] keyDown event in JavaScript: event.shiftKey, event.ctrlKey and eve...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Linux
: P2 Normal
Assignee: Yusuke Sato
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-20 23:50 PDT by Yusuke Sato
Modified: 2012-06-27 19:06 PDT (History)
3 users (show)

See Also:


Attachments
Patch (2.97 KB, patch)
2012-06-22 02:56 PDT, Yusuke Sato
no flags Details | Formatted Diff | Diff
Patch (3.14 KB, patch)
2012-06-27 04:49 PDT, Yusuke Sato
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Sato 2012-06-20 23:50:17 PDT
Chromium for Linux does not set .shiftKey, .ctrlKey, and .altKey correctly. For example, when Control key is pressed, the ctrlKey attribute of the event is false, and when it's released the attribute becomes true. Safari, Firefox, and Chrome for Windows/Mac/ChromeOS do the opposite, true on keyDown and false on keyUp, which should be correct.

http://code.google.com/p/chromium/issues/detail?id=127142
Comment 1 Yusuke Sato 2012-06-22 02:56:00 PDT
Created attachment 148988 [details]
Patch
Comment 2 Tony Chang 2012-06-22 11:02:52 PDT
Comment on attachment 148988 [details]
Patch

Can we write a layout test for this or does eventSender mock the events lower than this?
Comment 3 Tony Chang 2012-06-22 11:05:26 PDT
Comment on attachment 148988 [details]
Patch

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

> Source/WebKit/chromium/ChangeLog:8
> +        This is a Gtk port of http://crrev.com/142209.
> +        
> +        Reviewed by NOBODY (OOPS!).

The "Reviewed by..." line goes below the bug link. See the other entries in the ChangeLog.

> Source/WebKit/chromium/src/gtk/WebInputEventFactory.cpp:277
> +// Normalizes event->state to make it Windows/Mac compatible. Since the way
> +// of setting modifier mask on X is very different than Windows/Mac as shown
> +// in http://crbug.com/127142#c8, the normalization is necessary.

I would move this comment into the ChangeLog.
Comment 4 Yusuke Sato 2012-06-27 04:44:28 PDT
Checked DumpRenderTree/chromium/EventSender.cpp. Since the sender mocks an event by directly setting event->modifiers, I think it's hard to write a layout test for my patch.
Comment 5 Yusuke Sato 2012-06-27 04:49:34 PDT
Created attachment 149730 [details]
Patch
Comment 6 WebKit Review Bot 2012-06-27 19:06:30 PDT
Comment on attachment 149730 [details]
Patch

Clearing flags on attachment: 149730

Committed r121397: <http://trac.webkit.org/changeset/121397>
Comment 7 WebKit Review Bot 2012-06-27 19:06:41 PDT
All reviewed patches have been landed.  Closing bug.