Bug 64150 – dispatchEvent should raise an exception if the event object is already being dispatched

• Component: UI Events
• Status: RESOLVED FIXED

[Dominic Cooney]

This came up in the context of bug 63934.

DOM Events says calling dispatchEvent with an event that is being dispatched should raise DISPATCH_REQUEST_ERR.
<http://www.w3.org/TR/2011/WD-DOM-Level-3-Events-20110531/#events-EventTarget-dispatchEven

For context, the status quo in WebKit is that calling dispatchEvent and passing an event that is in the process of being dispatched resets the event’s target. Subsequent event listeners of the original event dispatch see an unrelated event.target, despite event.target being a readonly attribute that DOM Events says "must be the EventTarget object on which dispatchEvent is called." Firefox 5.0 makes dispatchEvent of an event that is being dispatched a no-op.

[Dominic Cooney]

Created attachment 100086 [details]
WIP patch

[Dominic Cooney]

Created attachment 100090 [details]
Patch

[WebKit Review Bot]

Comment on attachment 100090 [details]
Patch

Attachment 100090 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9000461

New failing tests:
http/tests/misc/slow-loading-mask.html
http/tests/navigation/javascriptlink-frames.html
fast/blockflow/japanese-rl-selection.html
svg/transforms/text-with-mask-with-svg-transform.svg
fast/backgrounds/background-leakage.html
fast/box-shadow/scaled-box-shadow.html
fast/backgrounds/repeat/negative-offset-repeat.html
svg/W3C-SVG-1.1/struct-use-01-t.svg
transforms/2d/hindi-rotated.html
http/tests/websocket/tests/hixie76/websocket-event-target.html
svg/repaint/filter-repaint.svg
fast/blockflow/japanese-lr-selection.html

[WebKit Review Bot]

Created attachment 100094 [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Dominic Cooney]

http/tests/websocket/tests/hixie76/websocket-event-target.html failures look relevant; I will investigate.

[Dominic Cooney]

Comment on attachment 100090 [details]
Patch

Clearing review: ? to investigate WebSocket test failures.

[Dominic Cooney]

Created attachment 100475 [details]
Patch

[Dimitri Glazkov (Google)]

Comment on attachment 100475 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=100475&action=review

> Source/WebCore/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=64150

It would be really good if you included here a link to the part of the spec you're implementing.

[Darin Adler]

Comment on attachment 100475 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=100475&action=review

> Source/WebCore/dom/Event.h:174
> +        bool isBeingDispatched() const { return eventPhase(); }

Even though the implementation is different, this seems redundant with the dispatched() function. Do we really need both?

[Dominic Cooney]

(In reply to comment #9)
> (From update of attachment 100475 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=100475&action=review
> 
> > Source/WebCore/dom/Event.h:174
> > +        bool isBeingDispatched() const { return eventPhase(); }
> 
> Even though the implementation is different, this seems redundant with the dispatched() function. Do we really need both?

The meanings *should* be different, so I think we need both:

dispatched()—has event dispatch started for this event at any point in the past, ie can you no longer call init*Event.

isBeingDispatched()—is event dispatch processing this event right now.

So the state transitions are different:

!dispatched() → dispatched() [terminal state]
!isBeingDispatched() → isBeingDispatched() → !isBeingDispatched() → …

However since bug 63934 is about the event target being reset to 0, which dispatched() depends on, I expect dispatched() is broken. If it is OK I will work on that in the context of bug 63934.

[Dominic Cooney]

Adding Darin in reply to comment 9:

<https://bugs.webkit.org/show_bug.cgi?id=64150#c10>

[Dominic Cooney]

Committed r90972: <http://trac.webkit.org/changeset/90972>