1313943 - Legacy event initializers should all do nothing during dispatch

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Test: http://w3c-test.org/dom/events/Event-init-while-dispatching.html

DOM's initEvent says that it must do nothing during dispatch.  UI Events says they behave the same as initEvent, so the events defined there also shouldn't.  Currently we only do this for initEvent itself, not all the other legacy event initialization.  Edge passes that test, and Chrome passes most of it (not initCustomEvent).  It probably makes sense for all the legacy initializers to be no-ops during dispatch.

Comment 1

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Attachment: Bug 1313943 - Legacy event initializers should all do nothing during dispatch;

DOM's initEvent says that it must do nothing during dispatch.  UI Events
says they behave the same as initEvent, so the legacy initializers
defined there also shouldn't do anything during dispatch.  Previously we
only did this for initEvent itself, but other legacy initializers would
function normally during dispatch.

Edge passes the wpt test that's updated in this commit.  Chrome passes
all but initCustomEvent.  It would be nice to expand the test to have
coverage for all our legacy initializers, but the likelihood of
real-world compat issues from this behavior is slim enough that I didn't
think it was worth the effort.

Review commit: https://reviewboard.mozilla.org/r/89478/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89478/

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8805854 [details]
Bug 1313943 - Legacy event initializers should all do nothing during dispatch;

https://reviewboard.mozilla.org/r/89478/#review89016

I tried to track down all the specs for these.  Overall seems reasonable to me and unlikely to break in the wild.  Thanks!

::: dom/events/ClipboardEvent.cpp:56
(Diff revision 1)
>  void
>  ClipboardEvent::InitClipboardEvent(const nsAString& aType, bool aCanBubble,
>                                     bool aCancelable,
>                                     DataTransfer* aClipboardData)
>  {
> +  NS_ENSURE_TRUE_VOID(!mEvent->mFlags.mIsBeingDispatched);

It seems this is not exposed to script.

::: dom/events/CommandEvent.cpp:54
(Diff revision 1)
>  CommandEvent::InitCommandEvent(const nsAString& aTypeArg,
>                                 bool aCanBubbleArg,
>                                 bool aCancelableArg,
>                                 const nsAString& aCommand)
>  {
> +  NS_ENSURE_TRUE(!mEvent->mFlags.mIsBeingDispatched, NS_OK);

I can't find the spec for CommandEvent.

::: dom/events/DeviceMotionEvent.cpp:54
(Diff revision 1)
>                       const DeviceAccelerationInit& aAccelIncludingGravity,
>                       const DeviceRotationRateInit& aRotationRate,
>                       Nullable<double> aInterval,
>                       Nullable<uint64_t> aTimeStamp)
>  {
> +  NS_ENSURE_TRUE_VOID(!mEvent->mFlags.mIsBeingDispatched);

This is a mozilla extension, so seems ok.

::: dom/events/DragEvent.cpp:58
(Diff revision 1)
>                           bool aMetaKey,
>                           uint16_t aButton,
>                           EventTarget* aRelatedTarget,
>                           DataTransfer* aDataTransfer)
>  {
> +  NS_ENSURE_TRUE_VOID(!mEvent->mFlags.mIsBeingDispatched);

I don't see initDragEvent in the spec, so this is a mozilla extension as well?

::: dom/events/FocusEvent.cpp:52
(Diff revision 1)
>                             bool aCancelable,
>                             nsGlobalWindow* aView,
>                             int32_t aDetail,
>                             EventTarget* aRelatedTarget)
>  {
> +  NS_ENSURE_TRUE_VOID(!mEvent->mFlags.mIsBeingDispatched);

This is not exposed to script.  I think you should be able to add here `MOZ_ASSERT(!mEvent->mFlags.mIsBeingDispatched)`.

::: dom/events/KeyboardEvent.cpp:340
(Diff revision 1)
>                              bool aShiftKey,
>                              bool aMetaKey,
>                              uint32_t aKeyCode,
>                              uint32_t aCharCode)
>  {
> +  NS_ENSURE_TRUE(!mEvent->mFlags.mIsBeingDispatched, NS_OK);

The spec calls this `initKeyboardEvent()` while we call it `initKeyEvent()`.  Seems ok to change this.

::: dom/events/MouseScrollEvent.cpp:59
(Diff revision 1)
>                                         bool aMetaKey,
>                                         uint16_t aButton,
>                                         EventTarget* aRelatedTarget,
>                                         int32_t aAxis)
>  {
> +  NS_ENSURE_TRUE_VOID(!mEvent->mFlags.mIsBeingDispatched);

Seems a mozilla extension or removed from the spec.  Should be ok.

::: dom/events/SimpleGestureEvent.cpp:127
(Diff revision 1)
>                                             uint32_t aAllowedDirectionsArg,
>                                             uint32_t aDirectionArg,
>                                             double aDeltaArg,
>                                             uint32_t aClickCountArg)
>  {
> +  NS_ENSURE_TRUE_VOID(!mEvent->mFlags.mIsBeingDispatched);

Seems a mozilla extension.

::: dom/events/StorageEvent.cpp:90
(Diff revision 1)
>                                 const nsAString& aOldValue,
>                                 const nsAString& aNewValue,
>                                 const nsAString& aURL,
>                                 DOMStorage* aStorageArea)
>  {
> +  NS_ENSURE_TRUE_VOID(!mEvent->mFlags.mIsBeingDispatched);

Mozilla extension.

::: dom/events/TouchEvent.cpp:97
(Diff revision 1)
>                             bool aMetaKey,
>                             TouchList* aTouches,
>                             TouchList* aTargetTouches,
>                             TouchList* aChangedTouches)
>  {
> +  NS_ENSURE_TRUE_VOID(!mEvent->mFlags.mIsBeingDispatched);

Seems loosely referenced in the spec as a non-compatible thing some browsers implement.  Should be fine to change.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/90bbef07e8be
Legacy event initializers should all do nothing during dispatch; r=bkelly

Comment 4

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/90bbef07e8be