Summary: | [GTK] DRT doesn't support scheduleAsynchronousKeyDown. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | ChangSeok Oh <kevin.cs.oh> | ||||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | gustavo, mrobinson, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
ChangSeok Oh
2012-02-13 03:06:46 PST
Created attachment 127978 [details]
Patch
Comment on attachment 127978 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=127978&action=review r-, although it's just for a few nitpicks and a possible useless include, I think you'll prefer to upload a new patch anyway? =) > Source/WebKit/gtk/WebCoreSupport/DumpRenderTreeSupportGtk.cpp:53 > +#include "PlatformKeyboardEvent.h" Why do we need this extra include? > Tools/DumpRenderTree/gtk/EventSender.cpp:761 > + sendKeyDown(static_cast<gpointer>(pressEvent)); I think I would prefer to have a proper function with the appropriate type for the argument that is called from here and from the idle callback. Comment on attachment 127978 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=127978&action=review > Tools/DumpRenderTree/gtk/EventSender.cpp:834 > + g_idle_add(sendKeyDown, static_cast<gpointer>(pressEvent)); Perhaps g_timeout_add after a 0 delay would be better here? Comment on attachment 127978 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=127978&action=review Thanks for the review :) >> Source/WebKit/gtk/WebCoreSupport/DumpRenderTreeSupportGtk.cpp:53 >> +#include "PlatformKeyboardEvent.h" > > Why do we need this extra include? Oops. Sorry :p >> Tools/DumpRenderTree/gtk/EventSender.cpp:761 >> + sendKeyDown(static_cast<gpointer>(pressEvent)); > > I think I would prefer to have a proper function with the appropriate type for the argument that is called from here and from the idle callback. Do you mean it looks better to wrap sendKeyDown and g_idle_add(sendKeyDown... in a function with arguments? For example, bool sendKeyDown(bool asynchronous)? Comment on attachment 127978 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=127978&action=review >> Tools/DumpRenderTree/gtk/EventSender.cpp:834 >> + g_idle_add(sendKeyDown, static_cast<gpointer>(pressEvent)); > > Perhaps g_timeout_add after a 0 delay would be better here? I followed the way of scheduleAsynchronousClickCallback. It has used g_idle_add. Do we need to modify this, too? :) Created attachment 128132 [details]
Patch
(In reply to comment #4) > >> Tools/DumpRenderTree/gtk/EventSender.cpp:761 > >> + sendKeyDown(static_cast<gpointer>(pressEvent)); > > > > I think I would prefer to have a proper function with the appropriate type for the argument that is called from here and from the idle callback. > > Do you mean it looks better to wrap sendKeyDown and g_idle_add(sendKeyDown... in a function with arguments? > For example, bool sendKeyDown(bool asynchronous)? No, I mean sendKeyDown should take GdkEvent* pressEvent as the only argument. You would call it directly here, and you would have a handler for the idle that would also call it. Created attachment 128229 [details]
Patch
Comment on attachment 128229 [details]
Patch
Yep! Thanks
Comment on attachment 128229 [details] Patch Clearing flags on attachment: 128229 Committed r108628: <http://trac.webkit.org/changeset/108628> All reviewed patches have been landed. Closing bug. |