Summary: | [GTK] fast/forms/listbox-selection.html fails | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Martin Robinson <mrobinson> | ||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | abarth, dbates, eric, webkit.review.bot, xan.lopez | ||||
Priority: | P3 | Keywords: | Gtk | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | PC | ||||||
OS: | Linux | ||||||
Attachments: |
|
Description
Martin Robinson
2010-09-16 20:05:05 PDT
Created attachment 67877 [details]
Patch for this issue
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. (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. (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! Committed r67860: <http://trac.webkit.org/changeset/67860> http://trac.webkit.org/changeset/67860 might have broken GTK Linux 32-bit Release |