RESOLVED FIXED 93322
Don't re-use the same EventDispatcher instance to dispatch events. 
https://bugs.webkit.org/show_bug.cgi?id=93322
Summary Don't re-use the same EventDispatcher instance to dispatch events.
Hayato Ito
Reported 2012-08-06 19:12:53 PDT
Three are some member functions in EventDispatcher, which assume the instance of EventDispatcher is used for only one event. Example: - EventDispatcher::ensureEventAncestor calculates ancestors of node. But once the calculation is done, it never recalculates. The result of the calculation depends on the given event's type(). So it is potentially dangerous to call EventDispatcher::dispatchEvent(Event) twice with different events against the same EventDispatcher instance. We have to make sure the EventDspatcher instance is used only once per event. Currently dispatching 'dblclick' event violates this rule.
Attachments
Fix dblclick event and add ASSERT (5.74 KB, patch)
2012-08-06 22:38 PDT, Hayato Ito 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Hayato Ito
Comment 1 2012-08-06 22:38:54 PDT
Created attachment 156868 [details] Fix dblclick event and add ASSERT
Dimitri Glazkov (Google)
Comment 2 2012-08-07 07:10:49 PDT
Comment on attachment 156868 [details] Fix dblclick event and add ASSERT ok
WebKit Review Bot
Comment 3 2012-08-07 20:09:44 PDT
Comment on attachment 156868 [details] Fix dblclick event and add ASSERT Clearing flags on attachment: 156868 Committed r124975: <http://trac.webkit.org/changeset/124975>
WebKit Review Bot
Comment 4 2012-08-07 20:09:47 PDT
All reviewed patches have been landed. Closing bug.
Darin Adler
Comment 5 2012-08-07 23:08:39 PDT
Comment on attachment 156868 [details] Fix dblclick event and add ASSERT View in context: https://bugs.webkit.org/attachment.cgi?id=156868&action=review > Source/WebCore/dom/MouseEvent.cpp:-218 > - doubleClickEvent->initMouseEvent(eventNames().dblclickEvent, event()->bubbles(), event()->cancelable(), event()->view(), > - event()->detail(), event()->screenX(), event()->screenY(), event()->clientX(), event()->clientY(), > - event()->ctrlKey(), event()->altKey(), event()->shiftKey(), event()->metaKey(), > - event()->button(), relatedTarget); This is the correct formatting for a multiple line function call in the WebKit coding style. > Source/WebCore/dom/MouseEvent.cpp:219 > + doubleClickEvent->initMouseEvent(eventNames().dblclickEvent, event()->bubbles(), event()->cancelable(), event()->view(), > + event()->detail(), event()->screenX(), event()->screenY(), event()->clientX(), event()->clientY(), > + event()->ctrlKey(), event()->altKey(), event()->shiftKey(), event()->metaKey(), > + event()->button(), relatedTarget); Whereas this is incorrect.
Hayato Ito
Comment 6 2012-08-07 23:33:16 PDT
Oh. Nice catch. Thank you. I think WebKit doesn't like a patch which only fixes style. So I'll fix it if i have a chance later. (In reply to comment #5) > (From update of attachment 156868 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=156868&action=review > > > Source/WebCore/dom/MouseEvent.cpp:-218 > > - doubleClickEvent->initMouseEvent(eventNames().dblclickEvent, event()->bubbles(), event()->cancelable(), event()->view(), > > - event()->detail(), event()->screenX(), event()->screenY(), event()->clientX(), event()->clientY(), > > - event()->ctrlKey(), event()->altKey(), event()->shiftKey(), event()->metaKey(), > > - event()->button(), relatedTarget); > > This is the correct formatting for a multiple line function call in the WebKit coding style. > > > Source/WebCore/dom/MouseEvent.cpp:219 > > + doubleClickEvent->initMouseEvent(eventNames().dblclickEvent, event()->bubbles(), event()->cancelable(), event()->view(), > > + event()->detail(), event()->screenX(), event()->screenY(), event()->clientX(), event()->clientY(), > > + event()->ctrlKey(), event()->altKey(), event()->shiftKey(), event()->metaKey(), > > + event()->button(), relatedTarget); > > Whereas this is incorrect.
Note You need to log in before you can comment on or make changes to this bug.