Bug 35432

Summary: [Gtk] Make DRT EventSender::keyDown to consider 'hardware_keycode' field when synthesizing an event
Product: WebKit Reporter: Antonio Gomes <tonikitoo>
Component: Tools / TestsAssignee: Antonio Gomes <tonikitoo>
Status: CLOSED FIXED    
Severity: Normal CC: gustavo, hausmann, jdiggs, webkit.review.bot, xan.lopez
Priority: P2 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 18662, 30587    
Attachments:
Description Flags
patch 0.1
tonikitoo: commit-queue-
patch 0.2 none

Antonio Gomes
Reported 2010-02-26 08:16:05 PST
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 ...
Attachments
patch 0.1 (3.20 KB, patch)
2010-02-26 08:32 PST, Antonio Gomes 		tonikitoo: commit-queue-
DetailsFormatted DiffDiff
patch 0.2 (3.31 KB, patch)
2010-02-26 09:26 PST, Antonio Gomes 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Antonio Gomes
Comment 1 2010-02-26 08:32:44 PST
Created attachment 49588 [details] patch 0.1
WebKit Review Bot
Comment 2 2010-02-26 08:37:58 PST
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
Comment 3 2010-02-26 09:26:46 PST
Created attachment 49591 [details] patch 0.2 Fixed the style issue.
Gustavo Noronha (kov)
Comment 4 2010-02-26 10:21:18 PST
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
Comment 5 2010-02-26 13:48:35 PST
r55309
Antonio Gomes
Comment 6 2010-04-11 20:01:00 PDT
Comment on attachment 49591 [details] patch 0.2 Clearing flags on attachment: 49591 Committed r55309: <http://trac.webkit.org/changeset/55309>
Note You need to log in before you can comment on or make changes to this bug.