Last Comment Bug 672054 - Remove nsIDOMNSUIEvent, nsIDOMNSMouseEvent
: Remove nsIDOMNSUIEvent, nsIDOMNSMouseEvent
Status: RESOLVED FIXED
: addon-compat, dev-doc-needed
Product: Core
Classification: Components
Component: DOM: Events (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla9
Assigned To: :Ms2ger (⌚ UTC+1/+2)
:
:
Mentors:
Depends on: 710239
Blocks: 716822
  Show dependency treegraph
 
Reported: 2011-07-16 09:42 PDT by :Ms2ger (⌚ UTC+1/+2)
Modified: 2012-01-10 04:11 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Part a: Remove nsIDOMNSUIEvent (49.41 KB, patch)
2011-08-02 02:56 PDT, :Ms2ger (⌚ UTC+1/+2)
bugs: review+
Ms2ger: checkin+
Details | Diff | Splinter Review
Part b: Remove nsIDOMNSMouseEvent (36.07 KB, patch)
2011-08-02 02:57 PDT, :Ms2ger (⌚ UTC+1/+2)
bugs: review+
Ms2ger: checkin+
Details | Diff | Splinter Review
Part c: Remove nsIDOMNSEvent (102.35 KB, patch)
2011-08-05 04:28 PDT, :Ms2ger (⌚ UTC+1/+2)
bugs: review+
Details | Diff | Splinter Review

Description :Ms2ger (⌚ UTC+1/+2) 2011-07-16 09:42:40 PDT
I've got patches that just need to be finished.
Comment 1 :Ms2ger (⌚ UTC+1/+2) 2011-08-02 02:56:37 PDT
Created attachment 550036 [details] [diff] [review]
Part a: Remove nsIDOMNSUIEvent
Comment 2 :Ms2ger (⌚ UTC+1/+2) 2011-08-02 02:57:28 PDT
Created attachment 550037 [details] [diff] [review]
Part b: Remove nsIDOMNSMouseEvent
Comment 3 Olli Pettay [:smaug] (reviewing overload) 2011-08-05 04:27:12 PDT
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
Comment 4 :Ms2ger (⌚ UTC+1/+2) 2011-08-05 04:28:33 PDT
Created attachment 550999 [details] [diff] [review]
Part c: Remove nsIDOMNSEvent
Comment 5 Olli Pettay [:smaug] (reviewing overload) 2011-08-05 04:29:42 PDT
...and then nsDOMUIEvent::QueryInterface could warn about using
nsIDOMNSUIEvent.
Same with other NS*Event interfaces.
Comment 6 Olli Pettay [:smaug] (reviewing overload) 2011-08-05 04:35:44 PDT
Comment on attachment 550037 [details] [diff] [review]
Part b: Remove nsIDOMNSMouseEvent

update the iid of nsIDOMMouseEvent, nsIDOMDragEvent and nsIDOMMouseScrollEvent
Comment 7 Olli Pettay [:smaug] (reviewing overload) 2011-08-05 05:03:13 PDT
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.
Comment 8 :Ms2ger (⌚ UTC+1/+2) 2011-08-20 03:33:42 PDT
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?
Comment 9 :Mook 2011-08-25 21:38:44 PDT
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.
Comment 10 :Ms2ger (⌚ UTC+1/+2) 2011-08-26 12:32:58 PDT
Comment on attachment 550036 [details] [diff] [review]
Part a: Remove nsIDOMNSUIEvent

http://hg.mozilla.org/mozilla-central/rev/ce4f04d1c8e7
Comment 11 :Ms2ger (⌚ UTC+1/+2) 2011-08-26 12:33:30 PDT
Comment on attachment 550037 [details] [diff] [review]
Part b: Remove nsIDOMNSMouseEvent

http://hg.mozilla.org/mozilla-central/rev/cc1e08803869
Comment 12 Jorge Villalobos [:jorgev] 2012-01-09 13:53:19 PST
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.
Comment 13 :Ms2ger (⌚ UTC+1/+2) 2012-01-10 00:31:11 PST
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.

Note You need to log in before you can comment on or make changes to this bug.