Closed Bug 386743 Opened 13 years ago Closed 4 years ago

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

Categories

(Core :: DOM: Events, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: smaug, Assigned: stone)

References

Details

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

Attachments

(1 file, 1 obsolete file)

 
Is this still valid, Olli?
Flags: needinfo?(bugs)
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]
(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)
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)
Whiteboard: [tw-dom] → [tw-dom] btpp-active
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)
Sorry about delay, have had high review load. I'll try to get to this tomorrow.
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+
Follow reviewer's feedbacks to refine codes and change summary.
Attachment #8741675 - Attachment is obsolete: true
Attachment #8745187 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/dfd6e945e298
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.