Last Comment Bug 766694 - Add a test to check existence of each nsIDOM* interface in window scope
: Add a test to check existence of each nsIDOM* interface in window scope
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: ---
Assigned To: Olli Pettay [:smaug] (way behind * queues, especially ni? queue)
:
:
Mentors:
Depends on: 873939
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-20 13:38 PDT by Olli Pettay [:smaug] (way behind * queues, especially ni? queue)
Modified: 2013-05-19 22:51 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Something like this (13.81 KB, patch)
2012-06-21 13:53 PDT, Olli Pettay [:smaug] (way behind * queues, especially ni? queue)
bzbarsky: review+
Details | Diff | Splinter Review

Description Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2012-06-20 13:38:41 PDT
It is way too easy to pollute global scope by adding just nsIDOM* interface.
If we had a test to check all the known nsIDOM* interfaces, whoever is adding
a new one would need to think whether it is actually needed, or if nsI* interface would
be ok.

nsIInterfaceInfoManager::EnumerateInterfacesWhoseNamesStartWith should work for the test.
Comment 1 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2012-06-21 13:53:10 PDT
Created attachment 635446 [details] [diff] [review]
Something like this

We have all sorts of crap there in the list atm.
But we shouldn't add more.

https://tbpl.mozilla.org/?tree=Try&rev=78614934ecc3
Probably doesn't pass on try, since I expect there can be some
platform specific stuff too.
Comment 2 Mounir Lamouri (:mounir) 2012-06-21 14:12:40 PDT
Out of curiosity, how did you generate that list?

Also, could you do the opposite test? Checking that all interfaces in the list really exist so if we remove Moz prefixed interfaces, we clean the list instead of keeping it full of dead interfaces.
Comment 3 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2012-06-21 14:18:15 PDT
(In reply to Mounir Lamouri (:mounir) from comment #2)
> Out of curiosity, how did you generate that list?
Iterated all the interfaces and checked which ones are "in window".
(If the interface starts with nsIDOM removed that, if with nsI removed that one from the interface name)

> Also, could you do the opposite test? Checking that all interfaces in the
> list really exist so if we remove Moz prefixed interfaces, we clean the list
> instead of keeping it full of dead interfaces.
I explicitly wanted to do it this way. Partly because we can support all the platforms
using just one list (even if some interface is exposed only in some platform, say b2g).
And also because this is really for the new stuff.
Comment 4 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2012-06-21 15:29:22 PDT
Comment on attachment 635446 [details] [diff] [review]
Something like this

Boris, what you think of this kind of approach?

I'm not sure how much this will help, but at least people should
think about whether to add nsIDOM* interfaces which are currently
automatically in the global scope.
Comment 5 Boris Zbarsky [:bz] (still a bit busy) 2012-06-21 22:33:29 PDT
Comment on attachment 635446 [details] [diff] [review]
Something like this

I like.
Comment 6 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2012-06-23 09:33:35 PDT
https://hg.mozilla.org/mozilla-central/rev/a387c891b56f
Comment 7 John Daggett (:jtd) 2012-08-01 18:09:19 PDT
Um, while adding a new CSS rule for @font-feature-values from CSS3 Fonts (see bug 549861) I'm seeing a mochitest error from this testcase.  Should I simply add the new interface or is there something I think to consider more carefully here?

A comment in the testcase advising about this would be most helpful.
Comment 8 Boris Zbarsky [:bz] (still a bit busy) 2012-08-01 21:17:28 PDT
If the new interface is meant to be exposed as a property on the global, then yes, just add it...

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