Move DOMImplementation to Paris bindings

RESOLVED FIXED in mozilla19

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Ms2ger, Assigned: Ms2ger)

Tracking

(Blocks: 1 bug, {addon-compat})

Trunk
mozilla19
addon-compat
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 1 obsolete attachment)

Comment hidden (empty)
(Assignee)

Comment 1

5 years ago
Created attachment 668136 [details] [diff] [review]
Patch v1
Attachment #668136 - Flags: review?(bzbarsky)
Shouldn't there be some nsDocument bits here too?
(Assignee)

Comment 3

5 years ago
Created attachment 668144 [details] [diff] [review]
Patch v2

Let's try this again.
Attachment #668136 - Attachment is obsolete: true
Attachment #668136 - Flags: review?(bzbarsky)
Attachment #668144 - Flags: review?(bzbarsky)
Comment on attachment 668144 [details] [diff] [review]
Patch v2

>+++ b/content/base/src/DOMImplementation.cpp
>+  NS_INTERFACE_MAP_ENTRY(nsIDOMDOMImplementation)
>+  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIDOMDOMImplementation)

This was there in the code you copied from, but it's clearly silly.  Make the nsISupports bit not use AMBIGUOUS?

>+++ b/dom/bindings/Bindings.conf
>+'DOMImplementation': {
>+},
>+

Now that bug 792980 is fixed, you don't need that.

Given that this is not prefable, why did you leave nsIDOMDOMImplementation and associated gunk?  Seems like it can go....

r=me with those addressed.  Though if you do remove nsIDOMDOMImplementation, I'd like a look at the result.
Attachment #668144 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 5

5 years ago
Created attachment 668577 [details] [diff] [review]
Part b: Pass an nsIContent to InternalIsSupported

This interface is pretty silly. I like this better. (Turns out I don't strictly need it, as DOMImplementation still inherit from nsISupports.)
Attachment #668577 - Flags: review?(bzbarsky)
(Assignee)

Comment 6

5 years ago
Created attachment 668578 [details] [diff] [review]
Part c: Remove nsIDOMDOMImplementation
Attachment #668578 - Flags: review?(bzbarsky)
Comment on attachment 668577 [details] [diff] [review]
Part b: Pass an nsIContent to InternalIsSupported

r=me
Attachment #668577 - Flags: review?(bzbarsky) → review+
Comment on attachment 668578 [details] [diff] [review]
Part c: Remove nsIDOMDOMImplementation

r=me
Attachment #668578 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 9

5 years ago
Created attachment 671128 [details] [diff] [review]
Part d: Followup fixes

This passes try, at least. (Should've thought about that earlier.)
Attachment #671128 - Flags: review?(bzbarsky)
Do we really want to remove nsIDOMDOMImplementation? I would expect some
binary addons to use it to create documents.
(And in fact it is probably the safest way to create "data" documents to be used in web pages.)
Keywords: addon-compat
> I would expect some binary addons to use it to create documents.

How would they get their hands on an nsIDOMDOMImplementation?
From nsIDOMDocument
And in particular, note that the ability to create a DOMImplementation by contract went away in bug 675166.

So are you thinking about cases in which the binary addon would call GetImplementation on an existing document?
I guess if we think binary addons are using it, we could keep it...  It seems somewhat unfortunate, though.  :(
Comment on attachment 671128 [details] [diff] [review]
Part d: Followup fixes

r=me on this part, but I talked to smaug and we decided to keep nsIDOMDOMImplementation for now for C++ consumers.  :(  Still don't need classinfo, of course.
Attachment #671128 - Flags: review?(bzbarsky) → review+
Blocks: 802560
(Assignee)

Updated

5 years ago
Attachment #668578 - Attachment description: Part b: Remove nsIDOMDOMImplementation → Part c: Remove nsIDOMDOMImplementation
(Assignee)

Updated

5 years ago
Attachment #671128 - Attachment description: Part c: Followup fixes → Part d: Followup fixes
(Assignee)

Comment 16

5 years ago
https://hg.mozilla.org/mozilla-central/rev/b72413cdab5e
https://hg.mozilla.org/mozilla-central/rev/abe709bfc49a
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
You need to log in before you can comment on or make changes to this bug.