Bug 45942 – [GTK] fast/forms/listbox-selection.html fails

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Martin Robinson]

It appears the issue is that keyDown in the EventSender does not handle all the appropriate modifier strings.

[Martin Robinson]

Created attachment 67877 [details]
Patch for this issue

[Daniel Bates]

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.

[Martin Robinson]

(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.

[Martin Robinson]

(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!

[Martin Robinson]

Committed r67860: <http://trac.webkit.org/changeset/67860>

[WebKit Review Bot]

http://trac.webkit.org/changeset/67860 might have broken GTK Linux 32-bit Release