[Webkit-unassigned] [Bug 89742] [Win] key event's location does not work on Windows platform.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=89742





--- Comment #6 from Takashi Sakamoto <tasak at google.com>  2012-09-03 02:24:01 PST ---
(From update of attachment 149232)
View in context: https://bugs.webkit.org/attachment.cgi?id=149232&action=review

Thank you for reviewing.

>> Source/WebKit/chromium/ChangeLog:10
>> +        keys. So modified keybordEvent to see a scancode and an extended key
> 
> You mean in wParam? It's inaccurate to say those window messages don't supply key code that distinguishes left and right keys because they DO in lParam.

Yes, I mean wParam. I know lParam has extended key bit. However, it is not A VIRTUAL KEYCODE. We have to re-create a new virtual keycode by using the given scancode, extended key bit, and virtual keycode. So I wrote "So modified keybordEvent to see a scancode and an extended key bit in lParam to distinguish."

>> Source/WebKit/chromium/ChangeLog:14
>> +        (WebKit::windowsKeycodeWithLocation):
> 
> This code seems port neutral. Why does it have to be in chromium directory? What do other ports do?

I think, this code is very windows-specific. As far as I know, Linux+GTK uses XKeyEvent and it contains a key symbol which distinguishes between left- and right-hand keys (e.g. keysym 0xffe1, Shift_L or keysym 0xffe2, Shift_R).
(I read chromium/src/content/browser/renderer_host/native_web_keyboard_event_gtk.cc)

>> Source/WebKit/chromium/src/win/WebInputEventFactory.cpp:99
>> +static int windowsKeycodeWithLocation(WPARAM wparam, LPARAM lParam)
> 
> Why is "param" in lParam capitalized?
> It appears that the convention is not to capitalize it.

Done.

>> Source/WebKit/chromium/src/win/WebInputEventFactory.cpp:118
>> +
> 
> Where is the closing } ??

I'm sorry. I have just prepared windows development environment. (So I was planning to see WebKit bot's result.) Now I can build and do tests in Windows. I tested my new patch.

>> Source/WebKit/chromium/src/win/WebInputEventFactory.cpp:123
>> +    int virtualKeycode = ::MapVirtualKey(scancode, MAPVK_VSC_TO_VK_EX);
> 
> I have a lot of difficulty understanding this comment, and how it relates to the code below.

I rewrote the comment and moved the comment before "if ((lParam >> 16) & KF_EXTENDED)". I mean, for example, the code always returns VK_RCONTROL in the case where the given lparam's extended key bit is set and the wparam is VK_CONTROL. However, it might be better to use MapVirtualKey to recreate a new virtual keycode. I think, the conversion should be done by Windows API.

>> Source/WebKit/chromium/src/win/WebInputEventFactory.cpp:124
>> +    return virtualKeyCode ? virtualKeyCode : keycode;
> 
> For which virtual key code is this code used?

Done.

>> Tools/DumpRenderTree/win/EventSender.cpp:486
>> +        keyData += 0x1d << 16; // Left control's scancode
> 
> Why are we not calling MapVirtualKey with MAPVK_VK_TO_VSC?

Done.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.