Created attachment 419376 [details] Test case Check the attached example and type things like Ctrl or Alt. The event.ctrlKey or event.altKey will be false. This works fine on Mac and other browsers.
Created attachment 419387 [details] Patch
Comment on attachment 419387 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=419387&action=review > Source/WebKit/Shared/gtk/WebEventFactory.cpp:70 > + if (keyboardEvent) { When ”keyboardEvent” is true, the event type will always be “GDK_KEY_PRESS” or “GDK_KEY_RELEASE”; so instead of adding this ”keyboardEvent” flag to the function signature, I think we can directly check for “GDK_KEY_PRESS” instead: if (gdk_event_get_event_type(const_cast<GdkEvent*>(event)) == GDK_KEY_PRESS) { guint keyval; gdk_event_get_keyval(event, &keyval); switch (keyval) { // ... } } return modifiers; …or is there something else I am missing here? > Source/WebKit/Shared/gtk/WebEventFactory.cpp:340 > + Please remove this stray empty line before landing.
Created attachment 419402 [details] Patch
Comment on attachment 419387 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=419387&action=review >> Source/WebKit/Shared/gtk/WebEventFactory.cpp:70 >> + if (keyboardEvent) { > > When ”keyboardEvent” is true, the event type will always be “GDK_KEY_PRESS” or > “GDK_KEY_RELEASE”; so instead of adding this ”keyboardEvent” flag to the function > signature, I think we can directly check for “GDK_KEY_PRESS” instead: > > if (gdk_event_get_event_type(const_cast<GdkEvent*>(event)) == GDK_KEY_PRESS) { > guint keyval; > gdk_event_get_keyval(event, &keyval); > switch (keyval) { > // ... > } > } > > return modifiers; > > …or is there something else I am missing here? True thing, the first version of this method was not getting the event. I'll upload a new version doing that next week. Thanks for the review.
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Created attachment 419560 [details] Patch
Created attachment 419561 [details] Patch
Applied the suggested changes and added a test. PTAL, thanks!
Comment on attachment 419561 [details] Patch Very nice that the patch comes now with a layout test, thanks for that!
Committed r272489: <https://commits.webkit.org/r272489> All reviewed patches have been landed. Closing bug and clearing flags on attachment 419561 [details].