[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 #13 from Takashi Sakamoto <tasak at google.com>  2012-09-05 04:32:31 PST ---
(In reply to comment #11)
> (In reply to comment #6)
> > (From update of attachment 149232 [details] [details])
> > 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)
> 
> Sure, but can it be shared with the Apple Windows port?


I see. I moved the code to WebCore/platform/win/KeyEventWin.cpp.

Best regards,
Takashi Sakamoto

> 
> > 
> > >> 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.