209113 - |Node| should grab constants on nsIDOM3Node

Status: RESOLVED FIXED

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

Description

[Christopher Aillon (sabbatical, not receiving bugmail)]

Currently, we have some DOM Level 3 Node constants which exist in nsIDOMNode
because constants placed in nsIDOM3Node aren't available to JS via |Node.FOO|.

We should fix things to work properly, and move the constants to nsIDOM3Node
where they belong.  I have a patch for this which I will attach sometime tonight.

Comment 1

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Patch

Comment 2

[Johnny Stenback (:jst)]

Comment on attachment 125486 [details] [diff] [review]
Patch

sr=jst

Comment 3

[timeless]

Comment on attachment 125486 [details] [diff] [review]
Patch

your checkin might want to note that some of the values for the constants
changed :)

Comment 4

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 125486 [details] [diff] [review]
Patch

Landed on the trunk.  Seeking 1.4 approval:

There has been a long standing issue in Mozilla where constants added to
nsIDOM3Node weren't available to script.  Because of this issue, when I
implemented some methods in the DOM3 spec that required constants, jst had me
add those constants to nsIDOMNode, even though it was frozen at the time
because it wouldn't affect backwards compatibility.  With (for example)
aaronl's work on the SDK, I think we need to fix that oddity sooner rather than
later, and move those constants out of nsIDOMNode and into nsIDOM3Node where
they really belong since its possible that they may change (the spec is not
finalized yet).  The patch attached to the bug does that, and fixes it so that
a node in script has these constants available which was the only reason that
the constants were "mis-placed" in the first place...  Therefore, jst and I
think it would be nice to have this for 1.4 since it helps "future proof" us.

The relevant changes are in nsDOMClassInfo.cpp, which is very low risk, and
nsIDOMNode.idl and nsIDOM3Node.idl -- the other changes are just updating
callers in our tree to reference nsIDOM3Node instead of nsIDOMNode.  Again,
very low risk.

Comment 5

[Johnny Stenback (:jst)]

IMO we should take this on the branch, it makes our interfaces across releases
more consistent from here on, and I don't see any risk in taking this fix on the
branch.

Comment 6

[Asa Dotzler [:asa]]

Comment on attachment 125486 [details] [diff] [review]
Patch

moving approval request forward.

Comment 7

[Christopher Aillon (sabbatical, not receiving bugmail)]

Fixed on trunk a while ago.

/me wishes this would get approval though for the 1.4 branch if anyone still
cares about that....

Comment 8

[Asa Dotzler [:asa]]

How will we go about testing this if we take it on the branch? If you can
construct a reasonable plan for testing it and can get it together pretty
quickly then we may be able to take it. 

Comment 9

[Christopher Aillon (sabbatical, not receiving bugmail)]

If it compiles, and the following test passes, it should be good.

javascript:alert(Node.DOCUMENT_POSITION_CONTAINED_BY==0x10?"pass":"fail");

Comment 10

[Asa Dotzler [:asa]]

Comment on attachment 125486 [details] [diff] [review]
Patch

a=asa (on behalf of drivers) for checkin to the 1.4 branch.

Comment 11

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 125486 [details] [diff] [review]
Patch

Landed on the branch just now.