Closed
Bug 672054
Opened 13 years ago
Closed 12 years ago
Remove nsIDOMNSUIEvent, nsIDOMNSMouseEvent
Categories
(Core :: DOM: Events, defect)
Core
DOM: Events
Tracking
()
RESOLVED
FIXED
mozilla9
People
(Reporter: Ms2ger, Assigned: Ms2ger)
References
Details
(Keywords: addon-compat, dev-doc-needed)
Attachments
(3 files)
49.41 KB,
patch
|
smaug
:
review+
Ms2ger
:
checkin+
|
Details | Diff | Splinter Review |
36.07 KB,
patch
|
smaug
:
review+
Ms2ger
:
checkin+
|
Details | Diff | Splinter Review |
102.35 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
I've got patches that just need to be finished.
Assignee | ||
Comment 1•13 years ago
|
||
Attachment #550036 -
Flags: review?(Olli.Pettay)
Assignee | ||
Comment 2•13 years ago
|
||
Attachment #550037 -
Flags: review?(Olli.Pettay)
Comment 3•13 years ago
|
||
Comment on attachment 550036 [details] [diff] [review] Part a: Remove nsIDOMNSUIEvent >+ virtual nsresult Which(PRUint32* aWhich) { { should be in the next line >- nsCOMPtr<nsIDOMNSUIEvent> nsUIEvent(do_QueryInterface(aEvent)); >- if (nsUIEvent) >- nsUIEvent->GetPreventDefault(&preventDefault); >+ nsCOMPtr<nsIDOMNSEvent> domNSEvent = do_QueryInterface(aEvent); >+ if (domNSEvent) >+ domNSEvent->GetPreventDefault(&preventDefault); if (expr) { stmt; } Could you ask someone to check if addons use nsIDOMNSUIEvent often. If they do, could you add a dummy nsIDOMNSUIEvent: nsIDOMUIEvent interface to nsIDOMUIEvent.idl
Attachment #550036 -
Flags: review?(Olli.Pettay) → review+
Assignee | ||
Comment 4•13 years ago
|
||
Attachment #550999 -
Flags: review?(Olli.Pettay)
Comment 5•13 years ago
|
||
...and then nsDOMUIEvent::QueryInterface could warn about using nsIDOMNSUIEvent. Same with other NS*Event interfaces.
Comment 6•13 years ago
|
||
Comment on attachment 550037 [details] [diff] [review] Part b: Remove nsIDOMNSMouseEvent update the iid of nsIDOMMouseEvent, nsIDOMDragEvent and nsIDOMMouseScrollEvent
Attachment #550037 -
Flags: review?(Olli.Pettay) → review+
Comment 7•13 years ago
|
||
Comment on attachment 550999 [details] [diff] [review] Part c: Remove nsIDOMNSEvent I guess you should update iid of all the event interfaces, with that r+ for the patch. But please ask someone to check how often nsIDOMNSEvent is used in addons. We may need to keep the interface just for the consts.
Attachment #550999 -
Flags: review?(Olli.Pettay) → review+
Assignee | ||
Comment 8•13 years ago
|
||
I asked Jesse to check for add-ons, and nsIDOMNSMouseEvent and nsIDOMNSUIEvent aren't used in code, and for nsIDOMNSEvent he said that just 7 files used it, and "most of the nsIDOMNSEvent hits are just using the constants, but there are some QI calls as well". I'm happy to keep nsIDOMNSEvent for the constants, but I'd rather not have to keep supporting it in all events' QI implementations. What do you think?
I believe you missed two tiny spots for nsIDOMNSUIEvent: https://mxr.mozilla.org/mozilla-central/search?string=nsuievent&find=\.xml&tree=mozilla-central They're just there for constants, so it's not that bad. I think they end up being exposed to unprivileged content (http://software.hixie.ch/utilities/js/live-dom-viewer/?%3Cscript%3Edocument.write%28NSUIEvent%29%3B%3C%2Fscript%3E) but that's probably not worth thinking about.
Assignee | ||
Comment 10•13 years ago
|
||
Comment on attachment 550036 [details] [diff] [review] Part a: Remove nsIDOMNSUIEvent http://hg.mozilla.org/mozilla-central/rev/ce4f04d1c8e7
Attachment #550036 -
Flags: checkin+
Assignee | ||
Comment 11•13 years ago
|
||
Comment on attachment 550037 [details] [diff] [review] Part b: Remove nsIDOMNSMouseEvent http://hg.mozilla.org/mozilla-central/rev/cc1e08803869
Attachment #550037 -
Flags: checkin+
Updated•13 years ago
|
Keywords: addon-compat
Comment 12•12 years ago
|
||
Are the check-ins in comment #10 and comment #11 already in Firefox? If so, which version(s)? I'm getting reports from an add-on developer that says nsIDOMNSUIEvent is no longer there, and this bug is not resolved and doesn't have a milestone. If this is being fixed in chunks during several releases, I would appreciate it that you create dependent bugs that are easier to track.
Assignee | ||
Comment 13•12 years ago
|
||
You're right, of course. I'd planned to get nsIDOMNSEvent done faster, but there always seems to be more urgent things to do. nsIDOMNS{UI,Mouse}Event were removed in 9, and I filed bug 716822 for nsIDOMNSEvent.
Keywords: dev-doc-needed
Summary: Remove nsIDOMNSUIEvent, nsIDOMNSMouseEvent, nsIDOMNSEvent → Remove nsIDOMNSUIEvent, nsIDOMNSMouseEvent
Target Milestone: --- → mozilla9
Assignee | ||
Updated•12 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•