Bug 35432 – [Gtk] Make DRT EventSender::keyDown to consider 'hardware_keycode' field when synthesizing an event

• Component: Tools / Tests
• Status: CLOSED FIXED

[Antonio Gomes]

When synthesizing an event in Gtk's DRT EventSender keydownCallback implementation, an invalid 'hardware_keycode' value
can cause it to be badly processed by Gtk+.

Scenarios:

1) (most of current layout tests fall in this category)
* Focused node an editor (e.g. <input type=text> , <textarea>, etc).
* EventSender::keyDown("arrowDown") is called from JS.
* webkit_web_view_key_press is called and the event is processed by WebCore.

bool webkit_web_view_key_press_event (...)
(...)
  if (frame->eventHandler()->keyEvent(keyboardEvent))
    return TRUE;
(...)


2) On the other hand, in spatial navigation tests in bug 18662 ...

* Focused node is a link (i.e. <a href="xxx">)
* EventSender::keyDown("arrowDown") is called from JS.
* webkit_web_view_key_press does not proccess the event and fallback to GTK+

bool webkit_web_view_key_press_event(...)
(...)
  /* Chain up to our parent class for binding activation */
  return GTK_WIDGET_CLASS(webkit_web_view_parent_class)->key_press_event(widget, event);


|gtk_bindings_activate_event| fails at some point because hardware_keycode is not provided.


patch coming ...

[Antonio Gomes]

Created attachment 49588 [details]
patch 0.1

[WebKit Review Bot]

Attachment 49588 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKitTools/DumpRenderTree/gtk/EventSender.cpp:565:  Declaration has space between type name and * in GdkKeymapKey *keys  [whitespace/declaration] [3]
WebKitTools/DumpRenderTree/gtk/EventSender.cpp:567:  Extra space before ( in function call  [whitespace/parens] [4]
WebKitTools/DumpRenderTree/gtk/EventSender.cpp:569:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 3 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Antonio Gomes]

Created attachment 49591 [details]
patch 0.2

Fixed the style issue.

[Gustavo Noronha (kov)]

Comment on attachment 49591 [details]
patch 0.2

> diff --git a/WebKitTools/DumpRenderTree/gtk/EventSender.cpp b/WebKitTools/DumpRenderTree/gtk/EventSender.cpp
> index 458e0ba..88330d2 100644
> --- a/WebKitTools/DumpRenderTree/gtk/EventSender.cpp
> +++ b/WebKitTools/DumpRenderTree/gtk/EventSender.cpp
> @@ -559,6 +559,16 @@ static JSValueRef keyDownCallback(JSContextRef context, JSObjectRef function, JS
>  
>      gboolean return_val;
>      event.key.type = GDK_KEY_PRESS;
> +
> +    // When synthesizing an event, an invalid hardware_keycode value
> +    // can cause it to be badly processed by Gtk+.
> +    GdkKeymapKey* keys;
> +    gint n_keys;
> +    if (gdk_keymap_get_entries_for_keyval(gdk_keymap_get_default(), gdkKeySym, &keys, &n_keys)) {
> +        event.key.hardware_keycode = keys[0].keycode;
> +        g_free(keys);
> +    }
> +

I suggest moving this code to before the gboolean return_val declaration, just so it is close to the rest of the general initializations (which are done for both the key press, and key release events).

This looks good to me, otherwise. Please check if we could enable a layout test with this, though. Would be good to be sure this is working before you land the spatial navigation patch. Thanks!

[Antonio Gomes]

r55309

[Antonio Gomes]

Comment on attachment 49591 [details]
patch 0.2

Clearing flags on attachment: 49591

Committed r55309: <http://trac.webkit.org/changeset/55309>