[Webkit-unassigned] [Bug 49289] [chromium] menu key doesn't work when capslock or numslock is on

[bugzilla-daemon at webkit.org]

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


Tony Chang <tony at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #73443|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #5 from Tony Chang <tony at chromium.org>  2010-11-10 09:56:53 PST ---
(From update of attachment 73443)
View in context: https://bugs.webkit.org/attachment.cgi?id=73443&action=review

>>> WebKit/chromium/public/WebInputEvent.h:133
>>> +    static const int InputModifiers = ShiftKey | ControlKey | AltKey | MetaKey;
>> 
>> Nit: I don't like this name much.  Aren't capslock and numlock also input modifiers?  Should there just be a static method that returns event.modifiers & (ShiftKey | ControlKey | AltKey | MetaKey) like the other methods?
> 
> I can't think of a better name (really the best name is "modifier", but the enum took that already). I got this name from the getWebInputModifiers() function in WebInputEventConversion.cpp
> 
> I think it's more versatile as a bitmask but if you would rather it be an applied bitmask I can do that.

Ok, if you want to keep it as a bitmask, can you change the if below to have named bools to make it easier to read?  E.g.:
bool isUnmodifiedMenuKey = !(event.modifiers & WebInputEvent::InputModifiers) && (event.windowsKeyCode == VKEY_APPS);
bool isShiftF10 = (event.modifiers == WebInputEvent::ShiftKey) && (event.windowsKeyCode == VKEY_F10)
if ((isUnmodifiedMenuKey || isShiftF10) && event.type == contextMenuTriggeringEventType) {

Please fix the style complaint too.

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