73681 - RANGE:Child Nodes of Attr Nodes have a parentNode property set to null.

Status: RESOLVED FIXED

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

Description

[Jeff Yates]

I have found that ALL child nodes of an Attr node have there parentNode set to 
null.  This should not be.  Only the Attr node should have a parentNode set to 
null.

I believe this bug is causing bug #73552.  Without this bug fixed I cannot 
patch up the Range object.

Attaching test case.


Jeff Yates

Comment 1

[Jeff Yates]

Attachment: Test Case

Comment 2

[Johnny Stenback (:jst)]

This is hardly a blocker.

I have no time to fix this, patches accepted as always if this is important.

Comment 3

[anthonyd]

it is a blocker johnny, it blocks 73552.  its coo though, i know your swamped, 
ill try to get around to it.

anthonyd

Comment 4

[anthonyd]

moving to 0.9.2, gotta get range sorted out.

anthonyd

Comment 5

[anthonyd]

fooled you! now im REALLY setting it to 0.9.2.
anthonyd

Comment 6

[Prashant Desale]

Updating QA contact to Shivakiran Tummala.

Comment 7

[Johnny Stenback (:jst)]

Moving to mozilla0.9.3

Comment 8

[Johnny Stenback (:jst)]

Anthony, seems like you know what the deal is here, over to you.

Comment 9

[anthonyd]

setting to 0.9.3

getting range spec up to snuff seems to be a never ending battle.

anthonyd

Comment 10

[anthonyd]

see 73552

Comment 11

[rubydoo123]

--> kin

Comment 12

[kinmoz]

Bulk move of mozilla1.0 bugs to mozilla.1.0.1. I will try to pull some of these
back in if I can.

Comment 13

[Fabian Guisset]

Need more info, but this seems to be not so important anymore. Marking P4.
If you need this functionality please speak up, and we will reconsider.
This *might* get fixed by the attribute revamp, whenever that happens.

Comment 14

[Shawn Walker]

This does make javascript manipulation of optgroup form elements a might bit
frustrating.

Comment 15

[Boris Zbarsky [:bzbarsky]]

How does this affect optgroups at all??

Comment 16

[Fabian Guisset]

Attachment: Possible fix to set attributes as parent nodes of their text value

If people still want this, I'm willing to do it..
I've got a working patch, but I'm not sure it's the right way to do it.
The problem is that nsDOMAttribute doesn't implement nsIContent, so
nsIContent::SetParent(nsIContent *) doesn't work.. so I have added
SetParentAttribute to nsITextContent and mParentAttr to nsGenericDataNode.
Though I'm sure that's not an option because of space concerns, I don't see any
other way.

Thoughts?

Comment 17

[Alex Vincent [:WeirdAl]]

I might be able to work on this.  nsITextContent::SetParentAttr(nsIAttribute * parentAttr), maybe

Comment 18

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

The first thing that needs to be changed for this is to make nsIContent::BindToTree take an nsINode* rather than an nsIContent* as aParent argument. Possibly we could then remove the nsIDocument* aDocument argument too.

Next thing is to check that none of the callers to GetNodeParent doesn't assume that the returned node is an nsIDocument if it isn't an nsIContent. But I think that should be the case.

Once that is done we could even make the attribute observe itself and call SetAttr if the underlying textnode is changed.

Comment 19

[Alex Vincent [:WeirdAl]]

Testcase passes in a stock Firefox 3.6 build:

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.8) Gecko/20100722 Firefox/3.6.8

WFM?

Comment 20

[Alex Vincent [:WeirdAl]]

qawanted:  Obviously, we need to test to see if this happens on the other platforms.  I wonder if we have a mochitest for this already.  If not, I'd like to see this bug's testcase converted to a mochitest.

Comment 21

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

Attachment: Mochitest

Comment 22

[Boris Zbarsky [:bzbarsky]]

Why do we want that test?  Aren't we aiming to make Attr not be Node?

Comment 23

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Yes! Let's not put any more work into Attr-nodes before we make them not be Nodes.

Comment 24

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

WFM