Closed
Bug 874669
Opened 11 years ago
Closed 11 years ago
Remove legacy event type constants
Categories
(Core :: DOM: Events, defect)
Core
DOM: Events
Tracking
()
RESOLVED
FIXED
mozilla25
People
(Reporter: emk, Assigned: emk)
Details
Attachments
(1 file, 1 obsolete file)
4.29 KB,
patch
|
Details | Diff | Splinter Review |
These constants were defined only for captureEvents/releaseEvents. They have no useful purpose anymore.
Assignee | ||
Comment 1•11 years ago
|
||
Comment 2•11 years ago
|
||
webkit does have those constants. I couldn't figure out why they were added.
Comment 3•11 years ago
|
||
Do you think you could check why they added those constants (long ago).
Updated•11 years ago
|
Flags: needinfo?(VYV03354)
Assignee | ||
Comment 4•11 years ago
|
||
(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)
Comment 5•11 years ago
|
||
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?
Assignee | ||
Comment 6•11 years ago
|
||
Trident or Presto doesn't support the property. If you prefer to remove all of the constants, please r+ the v1 patch.
Updated•11 years ago
|
Attachment #753816 -
Flags: review?(bugs)
Comment 7•11 years ago
|
||
Comment on attachment 752413 [details] [diff] [review] patch Maybe we should try this, but needs to land very early in a cycle, so perhaps FF25. Also, I think FF24 will be the next ESR, so would be good to not land this in that release.
Attachment #752413 -
Flags: review+
Assignee | ||
Comment 8•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/398f67b7d629
Comment 9•11 years ago
|
||
Sorry, backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/c2ea60e04312 - I suspect that in a few hours it'll look like the mochitest-1 failures like https://tbpl.mozilla.org/php/getParsedLog.php?id=24541486&tree=Mozilla-Inbound 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.
Assignee | ||
Comment 10•11 years ago
|
||
Probably bug 873996? (Technically the uuid bump shouldn't be needed because removing constants will not change the vtable layout.)
Assignee | ||
Comment 11•11 years ago
|
||
Relanded with clobber: https://hg.mozilla.org/integration/mozilla-inbound/rev/1d5db5f16a32
Comment 12•11 years ago
|
||
Backed out for: https://tbpl.mozilla.org/php/getParsedLog.php?id=24565121&tree=Mozilla-Inbound remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/edf2dd195222
Comment 13•11 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=24566168&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=24566198&tree=Mozilla-Inbound etc
Assignee | ||
Comment 14•11 years ago
|
||
Didn't fail on local and try server. Strange... https://tbpl.mozilla.org/?tree=Try&rev=f8fb00e437a3
Assignee | ||
Comment 15•11 years ago
|
||
Landed with touching the CLOBBER file. I hope it works... https://hg.mozilla.org/integration/mozilla-inbound/rev/61be0e4a8df6
Comment 16•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/61be0e4a8df6
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Comment 17•11 years ago
|
||
releaseEvents have been restored as Bug 673919 has been backed out. How about this?
Assignee | ||
Comment 18•11 years ago
|
||
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.
Comment 19•11 years ago
|
||
Got it.
You need to log in
before you can comment on or make changes to this bug.
Description
•