816387 - Turn on WebIDL binding generation for Document and hook it up to quickstubs

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

Just Do It.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Hook up the WebIDL bindings for Document.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Hook up the Document WebIDL API to quickstubs.

Comment 3

[Peter Van der Beken [:peterv]]

Comment on attachment 686442 [details] [diff] [review]
part 1.  Hook up the WebIDL bindings for Document.

Review of attachment 686442 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsDOMClassInfoID.h
@@ +55,1 @@
>  DOMCI_CASTABLE_INTERFACE(nsDocument, nsIDocument, 5, _extra)                  \

So I think you're going to have to add

NEW_BINDING(nsIDocument, Document)

and

NEW_BINDING(nsDocument, Document)

to nsDOMQS.h so that we can unwrap new binding objects for documents to nsIDocument and nsDocument by checking for the prototype id for Document.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> So I think you're going to have to add

Yeah, indeed.  Done.

Comment 5

[Boris Zbarsky [:bzbarsky]]

So I ran into one test failure with this, which I don't recall, but can't see why it would have failed to happen before.  Specifically, we have a test that does this:

  var l3 = document.createTouchList();

but the spec IDL doesn't quite allow that... though there has been talk of changing the spec to accept "Touch... touch", which would allow no args.  Doing that as I land this is trivial; should I just do so?  Matt?

Comment 6

[Boris Zbarsky [:bzbarsky]]

Oh, the failing test is content/events/test/test_bug648573.html

Also, I guess I need to make this [PrefControlled] just like we did HTMLElement, right, Peter?

Comment 7

[Peter Van der Beken [:peterv]]

(In reply to Boris Zbarsky (:bz) from comment #6)
> Also, I guess I need to make this [PrefControlled] just like we did
> HTMLElement, right, Peter?

Ah, I forgot about that. Yes, for now you'll also need to comment out all the touch properties and methods because the quickstubs hook up doesn't deal with pref controlled things. And uncomment them again in bug 803542.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Oh, right.  Ok, I'll comment those out for now and we can figure out what to do with createTouchList() with no args in bug 803542.

Comment 9

[Boris Zbarsky [:bzbarsky]]

   https://hg.mozilla.org/integration/mozilla-inbound/rev/b190d329f7f3
   https://hg.mozilla.org/integration/mozilla-inbound/rev/b7bb8951d3e2

Comment 10

[Boris Zbarsky [:bzbarsky]]

And https://hg.mozilla.org/integration/mozilla-inbound/rev/9db20066eb63 to fix the orange because more things got added to the imptest .json files that needed to go away.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b190d329f7f3
https://hg.mozilla.org/mozilla-central/rev/b7bb8951d3e2
https://hg.mozilla.org/mozilla-central/rev/9db20066eb63

Comment 12

[Matt Brubeck (:mbrubeck)]

(In reply to Boris Zbarsky (:bz) from comment #5)
> So I ran into one test failure with this, which I don't recall, but can't
> see why it would have failed to happen before.  Specifically, we have a test
> that does this:
> 
>   var l3 = document.createTouchList();
> 
> but the spec IDL doesn't quite allow that... though there has been talk of
> changing the spec to accept "Touch... touch", which would allow no args. 
> Doing that as I land this is trivial; should I just do so?  Matt?

Yes, changing our IDL to accept "Touch... touch" is a good thing, and matches WebKit.  We will change the spec too, or publish errata, or something.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Matt, thanks!

Filed bug 824824 on that.