If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Default event bubbling/cancelable flags should be set in the nsEvent constructor

RESOLVED FIXED in Firefox 49

Status

()

Core
DOM: Events
RESOLVED FIXED
10 years ago
a year ago

People

(Reporter: smaug, Assigned: stone)

Tracking

unspecified
mozilla49
Points:
---

Firefox Tracking Flags

(firefox49 fixed)

Details

(Whiteboard: [tw-dom] btpp-active)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

10 years ago
 
Is this still valid, Olli?
Flags: needinfo?(bugs)
(Reporter)

Comment 2

2 years ago
Trying to recall what this was about.
Probably about having a switch-case which then checks event type and based on that
sets the right flags. That way dispatching different events in different places would get consistent handling. So yes, I think still valid.
Flags: needinfo?(bugs)
Whiteboard: [tw-dom]
(Assignee)

Comment 3

2 years ago
(In reply to Olli Pettay [:smaug] from comment #2)
> Trying to recall what this was about.
> Probably about having a switch-case which then checks event type and based
> on that
> sets the right flags. That way dispatching different events in different
> places would get consistent handling. So yes, I think still valid.

We're planning to have an enum for all event types and use switch-case to set the right flags, which centralize the logic to set all types of events in the event base class. Is my understanding correct?
Assignee: nobody → sshih
Flags: needinfo?(bugs)
(Reporter)

Comment 4

2 years ago
Yeah, something like that. And this is needed for those event types only which are dispatched using Widget*Event classes on stack. Higher level DOM events need explicit initialization anyhow.
Flags: needinfo?(bugs)

Updated

2 years ago
Whiteboard: [tw-dom] → [tw-dom] btpp-active
(Assignee)

Comment 5

2 years ago
Created attachment 8741675 [details] [diff] [review]
Bug 386743: Set default event bubbling/cancelable flags in the WidgetEvent constructor.
(Assignee)

Comment 6

2 years ago
Comment on attachment 8741675 [details] [diff] [review]
Bug 386743: Set default event bubbling/cancelable flags in the WidgetEvent constructor.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b390bdb6747f8e5f8acf65f162cb68ad837c0355
Attachment #8741675 - Flags: review?(bugs)
(Reporter)

Comment 7

a year ago
Sorry about delay, have had high review load. I'll try to get to this tomorrow.
(Reporter)

Comment 8

a year ago
Comment on attachment 8741675 [details] [diff] [review]
Bug 386743: Set default event bubbling/cancelable flags in the WidgetEvent constructor.

So I was hoping we could set all the default cancalable/bubbles flags in one place, since currently we do set them in many places where we (widget) events.
But this is the right start for simplifying the setup and making it easier to maintain.

>+  void UpdateFlags() {
Nit, { goes to its own line

And could we call the method something else, since this is only about cancelable and bubbles...
perhaps call it SetDefaultCancelableAndBubbles().

>+    switch (mClass) {
>+      case eEditorInputEventClass:
>+        mFlags.mCancelable = false;
>+        mFlags.mBubbles = mFlags.mIsTrusted;
>+        break;
(not related to this bug, but it is odd that mBubbles depends on mIsTrusted.)
Attachment #8741675 - Flags: review?(bugs) → review+
(Assignee)

Comment 9

a year ago
Created attachment 8745187 [details] [diff] [review]
Bug 386743: Set default event bubbling/cancelable flags in the WidgetEvent constructor. r=smaug

Follow reviewer's feedbacks to refine codes and change summary.
Attachment #8741675 - Attachment is obsolete: true
Attachment #8745187 - Flags: review+
(Assignee)

Comment 10

a year ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b390bdb6747f8e5f8acf65f162cb68ad837c0355
(Assignee)

Updated

a year ago
Keywords: checkin-needed

Comment 11

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/dfd6e945e298
Keywords: checkin-needed

Comment 12

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/dfd6e945e298
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.