Add a test to check existence of each nsIDOM* interface in window scope

RESOLVED FIXED

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: smaug, Assigned: smaug)

Tracking

Trunk
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
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.
Assignee: nobody → bugs
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.
(Assignee)

Comment 3

5 years ago
(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.
(Assignee)

Comment 4

5 years ago
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.
Attachment #635446 - Flags: review?(bzbarsky)
Comment on attachment 635446 [details] [diff] [review]
Something like this

I like.
Attachment #635446 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 6

5 years ago
https://hg.mozilla.org/mozilla-central/rev/a387c891b56f
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Comment 7

5 years ago
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.
If the new interface is meant to be exposed as a property on the global, then yes, just add it...
Depends on: 873939
You need to log in before you can comment on or make changes to this bug.