Closed Bug 874669 Opened 8 years ago Closed 8 years ago

Remove legacy event type constants


(Core :: DOM: Events, defect)

Not set





(Reporter: emk, Assigned: emk)



(1 file, 1 obsolete file)

These constants were defined only for captureEvents/releaseEvents. They have no useful purpose anymore.
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → VYV03354
Attachment #752413 - Flags: review?(bugs)
webkit does have those constants. I couldn't figure out why they were added.
Do you think you could check why they added those constants (long ago).
Flags: needinfo?(VYV03354)
Attached patch patch v2Splinter Review
(In reply to Olli Pettay [:smaug] from comment #2)
> webkit does have those constants. I couldn't figure out why they were added.

Oh, I forgot WebKit wouldn't stick some properties until an actual instance is created, and I didn't realize that WebKit supported some of our constants.
Fortunately (?), their constants are unsigned short. So they don't support constants whose value is larger than 65535.

(In reply to Olli Pettay [:smaug] from comment #3)
> Do you think you could check why they added those constants (long ago).

Dunno, it backs to the KHTML-era. For now, I'm leaving constants smaller than 65536 to be safe.
Attachment #752413 - Attachment is obsolete: true
Attachment #752413 - Flags: review?(bugs)
Attachment #753816 - Flags: review?(bugs)
Flags: needinfo?(VYV03354)
It is not clear to me what the patch helps. Would be good to get rid of all the constants, but that is
a bit risky.

Does Trident or Presto have the constants?
Trident or Presto doesn't support the property.
If you prefer to remove all of the constants, please r+ the v1 patch.
Comment on attachment 752413 [details] [diff] [review]

Maybe we should try this, but needs to land very early in a cycle, so perhaps
Also, I think FF24 will be the next ESR, so would be good to not land this in
that release.
Attachment #752413 - Flags: review+
Sorry, backed out in - I suspect that in a few hours it'll look like the mochitest-1 failures like were on non-clobbered builds and the passes were on clobbers, and you can repush it with /CLOBBER modified (or find and fix the reason it needs a clobber), but right now we need a less-broken tree.
Probably bug 873996? (Technically the uuid bump shouldn't be needed because removing constants will not change the vtable layout.)
Didn't fail on local and try server. Strange...
Landed with touching the CLOBBER file. I hope it works...
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
releaseEvents have been restored as Bug 673919 has been backed out. How about this?
If the releaseEvents property is absent, |window.releaseEvent();| will throw. This is the reason the removal broke the compatibility despite that the method is no-op.
However, |window.releaseEvent(Event.MOUSEDOWN);| will not throw even if it is undefined. Also, IE 11 didn't add event type constants.
So I believe legacy event type constants are not needed for Web compat.
Got it.
You need to log in before you can comment on or make changes to this bug.