[Webkit-unassigned] [Bug 86694] Implement DOM_KEY_LOCATION_LEFT and RIGHT of KeyboardEvent's location property

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from Takashi Sakamoto <tasak at google.com>  2012-05-17 23:16:13 PST ---
Thank you for reviewing.

(In reply to comment #2)
> (From update of attachment 142419 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=142419&action=review
> 
> Looks great.
> 
> > Source/WebCore/ChangeLog:25
> > +        (WebCore):
> 
> Unhelpful cruft like this can be removed from ChangeLogs. This is just a bug in the script - it's meant to be helpful, not authoritative.

I see. I removed.

> 
> Thank you for providing helpful comments!
> 
> > Source/WebCore/dom/KeyboardEvent.cpp:57
> > +static inline int getWindowsVirtualCodeWithoutLocation(int keycode)
> 
> In WebCore coding style, "get" prefix means that the result is returned via an out argument. So, just "windowsVirtualCodeWithoutLocation" would be right.

Done.

> > Source/WebCore/dom/KeyboardEvent.cpp:74
> > +static inline unsigned getKeyLocation(const PlatformKeyboardEvent& key)
> 
> Ditto.

Done.

> Also, I don't understand why this returns unsigned, and not KeyboardEvent::KeyLocationCode.

I see. I don't have enough reason.
Done.

> > Tools/DumpRenderTree/mac/EventSendingController.mm:644
> > +    } else if ([character isEqualToString:@"rightMenu"]) {
> 
> Perhaps "leftAlt" and "rightAlt" would be better for these. On Windows, the key that maps to VK_MENU is Alt, and on Mac, it's Option - calling that "Menu" is surprising and confusing, regardless of how the Windows API constant is named.

Done.

> 
> > LayoutTests/fast/events/keydown-leftright-keys.html:51
> > +    testKeyEventWithLocation("rightShift", VK_SHIFT, KEY_LOCATION_RIGHT);
> 
> If you put KEY_LOCATION_RIGHT in quotes, will test output become easier to read?

I agree. Done.

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