[Webkit-unassigned] [Bug 83486] [EFL][DRT] EventSender needs implementation of leapForward function

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon May 7 13:03:44 PDT 2012


https://bugs.webkit.org/show_bug.cgi?id=83486





--- Comment #12 from Raphael Kubo da Costa (rakuco) <rakuco at webkit.org>  2012-05-07 13:03:42 PST ---
(From update of attachment 140572)
View in context: https://bugs.webkit.org/attachment.cgi?id=140572&action=review

> Tools/DumpRenderTree/efl/EventSender.cpp:66
> +struct MouseEventInfo;

Nit: You could move the DetalyedEvent declaration after MouseEventInfo and avoid this forward declaration here.

> Tools/DumpRenderTree/efl/EventSender.cpp:78
> +static WTF::Vector<DelayedEvent> gEventQueue;

Constructing global static objects this way is dangerous -- in general, we use the DEFINE_STATIC_LOCAL macro for that.

> Tools/DumpRenderTree/efl/EventSender.cpp:148
> +static void feedOrQueueMouseEvent(MouseEventInfo*, bool shouldFeedQueuedEvents = true);

I think this can be made more clear, even if it requires a few more lines of code. A boolean parameter in a function with this name is not very expressive, and someone looking at a place which calls this function for the first time will probably get confused. I suggest using an enum instead of a boolean and removing the default value from it, so you end up calling something like
  feedOrQueueMouseEvent(myMouseEvent, FeedQueuedEvents);
or
  feedOrQueueMouseEvent(myMouseEvent, DoNotFeedQueuedEvents);

> Tools/DumpRenderTree/efl/EventSender.cpp:334
> +    // We need to invert scolling values since in EFL negative z value means that

Nit: scolling -> scrolling

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list