Bug 45942 - [GTK] fast/forms/listbox-selection.html fails
Summary: [GTK] fast/forms/listbox-selection.html fails
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P3 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2010-09-16 20:05 PDT by Martin Robinson
Modified: 2010-09-20 11:16 PDT (History)
5 users (show)

See Also:


Attachments
Patch for this issue (4.45 KB, patch)
2010-09-16 20:07 PDT, Martin Robinson
dbates: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Robinson 2010-09-16 20:05:05 PDT
It appears the issue is that keyDown in the EventSender does not handle all the appropriate modifier strings.
Comment 1 Martin Robinson 2010-09-16 20:07:55 PDT
Created attachment 67877 [details]
Patch for this issue
Comment 2 Daniel Bates 2010-09-17 14:57:40 PDT
Comment on attachment 67877 [details]
Patch for this issue

View in context: https://bugs.webkit.org/attachment.cgi?id=67877&action=prettypatch

This patch looks sane to me.

r=me

> LayoutTests/platform/gtk/Skipped:-1169
> -fast/forms/listbox-selection.html

I did a brief search for all layout tests that call eventSender.keyDown() with a non-empty list of modifiers that are also in the GTK Skipped list and found fast/events/keydown-1.html. Does this still fail?

> WebKitTools/DumpRenderTree/gtk/EventSender.cpp:463
> +    // Get the modifiers from the second argument.
> +    guint modifiers = argumentCount >= 2 ? gdkModifersFromJSValue(context, arguments[1]) : 0;

I noticed that you did not put a similar comment "// Get the modifiers from the second argument." above the other call sites in mouseUpCallback and mouseDownCallback. We may want to consider putting such a comment at these other call sites.
Comment 3 Martin Robinson 2010-09-20 10:43:05 PDT
(In reply to comment #2)
> I did a brief search for all layout tests that call eventSender.keyDown() with a non-empty list of modifiers that are also in the GTK Skipped list and found fast/events/keydown-1.html. Does this still fail?

This one just needed platform-specific results. I've added them.

> I noticed that you did not put a similar comment "// Get the modifiers from the second argument." above the other call sites in mouseUpCallback and mouseDownCallback. We may want to consider putting such a comment at these other call sites.

I just removed this comment. After further consideration it really answers "What?" instead of "Why?" It's superfluous, I think.
Comment 4 Martin Robinson 2010-09-20 10:43:25 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > I did a brief search for all layout tests that call eventSender.keyDown() with a non-empty list of modifiers that are also in the GTK Skipped list and found fast/events/keydown-1.html. Does this still fail?
> 
> This one just needed platform-specific results. I've added them.
> 
> > I noticed that you did not put a similar comment "// Get the modifiers from the second argument." above the other call sites in mouseUpCallback and mouseDownCallback. We may want to consider putting such a comment at these other call sites.
> 
> I just removed this comment. After further consideration it really answers "What?" instead of "Why?" It's superfluous, I think.

By the way, thanks for the awesome review!
Comment 5 Martin Robinson 2010-09-20 10:44:20 PDT
Committed r67860: <http://trac.webkit.org/changeset/67860>
Comment 6 WebKit Review Bot 2010-09-20 11:16:08 PDT
http://trac.webkit.org/changeset/67860 might have broken GTK Linux 32-bit Release