821606 - Turn on WebIDL bindings for Element and HTMLElement

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Just to get this on file so we can track it...

We'll need to actually SetIsDOMBinding() on Element and override on its various immediate subclasses, because there are elements that have Element.prototype as the proto; see bug 817420.

This depends on getting reparenting and SOWs in, of course.

Comment 1

[Peter Van der Beken [:peterv]]

Attachment: v1

Comment 2

[Peter Van der Beken [:peterv]]

(In reply to Boris Zbarsky (:bz) from comment #0)
> We'll need to actually SetIsDOMBinding() on Element and override on its
> various immediate subclasses

I don't think that's true, we just need to SetIsDOMBinding() on nsXMLElement.

Comment 3

[Peter Van der Beken [:peterv]]

And I'm actually tempted to make these all non-prefable.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> I don't think that's true, we just need to SetIsDOMBinding() on nsXMLElement.

Ah, didn't realize we had a concrete class for "random Element".  Great!

> And I'm actually tempted to make these all non-prefable.

I agree.

Comment 5

[Peter Van der Beken [:peterv]]

Attachment: v1.1

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 694076 [details] [diff] [review]
v1.1

Those various WrapNode methods should be MOZ_OVERRIDE, right?

>+++ b/dom/bindings/Bindings.conf
> 'Element': {
>+    'concrete': True,

Why is that needed?  That should be the default.

>+'HTMLDivElement': {
>+    'concrete': True,

And here.  Furthermore, I don't think you need hasXPConnectImpls for HTMLDivElement.  There aren't any of those, since nothing subclasses it, right?

> 'HTMLElement': {
>+    'concrete': True,

Again, should not be needed.

> 'HTMLUnknownElement': {
>+    'concrete': True,

Or here.  And again, HTMLUnknownElement does not need hasXPConnectImpls.

r=me with the above nits picked.  I assume you don't need the NEW_BINDING thing for HTMLDivElement and HTMLUnknownElement is because no one ever uses them as argument types?

Comment 7

[Masatoshi Kimura [:emk]]

(In reply to Boris Zbarsky (:bz) from comment #6)
> >+'HTMLDivElement': {
> >+    'concrete': True,
> 
> And here.  Furthermore, I don't think you need hasXPConnectImpls for
> HTMLDivElement.  There aren't any of those, since nothing subclasses it,
> right?

Let's use MOZ_FINAL to ensure that.

Comment 8

[Boris Zbarsky [:bzbarsky]]

> Let's use MOZ_FINAL to ensure that.

Excellent idea.

Comment 9

[Peter Van der Beken [:peterv]]

(In reply to Boris Zbarsky (:bz) from comment #6)
> I assume you don't need the NEW_BINDING
> thing for HTMLDivElement and HTMLUnknownElement is because no one ever uses
> them as argument types?

No. We need a NEW_BINDING for every interface that's in the bitmaps (see DOMCI_CASTABLE_INTERFACES) that can also be implemented by a new binding. Because we special-case unwrapping for interfaces in the bitmaps and there's no fallback if we don't have a bitmap, which will happen with a new binding object, I added a fallback that uses the prototype ID. NEW_BINDING enables that fallback. And it turned out that I forgot to hook up EventTarget to it, so I added that.

https://hg.mozilla.org/integration/mozilla-inbound/rev/a6d996266cfe

Comment 10

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/a6d996266cfe

Comment 11

[Jean-Yves Perrier [:teoli]]

As this lead to several compatibility problems with Web sites I would like to be sure to document it into:
https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_20

But I can't really grasp what changed (.style removed? On which DOM object) due to the WebIDL migration of these elements. Could you help me?

Comment 12

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to Jean-Yves Perrier [:teoli] from comment #11)
> As this lead to several compatibility problems with Web sites I would like
> to be sure to document it into:
> https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_20
> 
> But I can't really grasp what changed (.style removed? On which DOM object)
> due to the WebIDL migration of these elements. Could you help me?

No attributes should have been removed. The main compat-affecting changes are the following:

* Where we used to have all members of the entire inheritance chain (e.g. HTMLDivElement -> HTMLElement -> Element -> Node -> EventTarget) on the interface prototype object of the leaf class (e.g. HTMLDivElement.prototype === document.createElement("div").__proto), we now put the members of HTMLElement just on HTMLElement.prototype (=== HTMLDivElement.prototype.__proto__). It turns out that Optimizely cleared the members of HTMLElement.prototype. This used to be no problem, because those members would be on HTMLDivElement.prototype, but caused a number of sites to break. (E.g., setting fooElement.innerHTML would no longer have an effect, because the setter that did the work was deleted.)

* Passing |null| to a function that takes a |DOMString| (as opposed to a |DOMString?|, which can take |null|), will stringify to "null" instead of the empty string, as XPConnect did. (In fact, this happened in bug 814195 and bug 818219 already.) In particular, element.setAttribute("foo", null) is apparently somewhat popular.

* (WebIDL object).QueryInterface(any interface) now always succeeds, regardless of whether the object implements the passed interface. I believe we'll fix this (bug 827546).

I may be forgetting something, but those are the important ones, at least.

Comment 13

[Boris Zbarsky [:bzbarsky]]

A few more that may be worth mentioning:

* instanceof now follows the spec for some elements in that "foo instanceof HTMLFooElement" will test false if the RHS and LHS come from different windows.  This is not the case for all elements that we have converted, but for some of them.

* QueryInterface is no longer visible to web content on the elements that have been converted.

* The exceptions thrown in various cases are now TypeErrors instead of a different kind of exception.

* For chrome code only, members that you had to use QueryInterface to get to are now always visible.  This doesn't affect site compat, of course, except insofar as sites can never get to these anymore.