Closed Bug 1313943 Opened 3 years ago Closed 3 years ago

Legacy event initializers should all do nothing during dispatch

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: ayg, Assigned: ayg)

Details

Attachments

(1 file)

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 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.
Attachment #8805854 - Flags: review?(bkelly) → review+
Pushed by ayg@aryeh.name:
https://hg.mozilla.org/integration/mozilla-inbound/rev/90bbef07e8be
Legacy event initializers should all do nothing during dispatch; r=bkelly
https://hg.mozilla.org/mozilla-central/rev/90bbef07e8be
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.