the menu key is ignored in certain situations, depending on modifier state. patch forthcoming.
Created attachment 73443 [details] try1
Attachment 73443 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'WebKit/chromium/ChangeLog', u'WebKit/chromium/public/WebInputEvent.h', u'WebKit/chromium/src/WebViewImpl.cpp']" exit_code: 1 WebKit/chromium/src/WebViewImpl.cpp:580: One space before end of line comments [whitespace/comments] [5] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 73443 [details] try1 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?
Comment on attachment 73443 [details] try1 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.
Comment on attachment 73443 [details] try1 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.
Created attachment 73521 [details] try2 style fixed; readability suggestion implemented.
Attachment 73521 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'WebKit/chromium/ChangeLog', u'WebKit/chromium/public/WebInputEvent.h', u'WebKit/chromium/src/WebViewImpl.cpp']" exit_code: 1 WebKit/chromium/src/WebViewImpl.cpp:574: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 73529 [details] try3 style again
Comment on attachment 73529 [details] try3 Clearing flags on attachment: 73529 Committed r71774: <http://trac.webkit.org/changeset/71774>
All reviewed patches have been landed. Closing bug.