Closed Bug 73681 Opened 19 years ago Closed 9 years ago

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

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: pbwiz, Assigned: Ms2ger)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Whiteboard: [C][range][embed])

Attachments

(3 files)

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
Attached file Test Case
Blocks: 73552
Status: UNCONFIRMED → NEW
Ever confirmed: true
This is hardly a blocker.

I have no time to fix this, patches accepted as always if this is important.
Severity: blocker → normal
OS: other → All
Hardware: PC → All
Target Milestone: --- → Future
it is a blocker johnny, it blocks 73552.  its coo though, i know your swamped, 
ill try to get around to it.

anthonyd
moving to 0.9.2, gotta get range sorted out.

anthonyd
Status: NEW → ASSIGNED
Summary: Child Nodes of Attr Nodes have a parentNode property set to null. → RANGE:Child Nodes of Attr Nodes have a parentNode property set to null.
fooled you! now im REALLY setting it to 0.9.2.
anthonyd
Target Milestone: Future → mozilla0.9.2
Updating QA contact to Shivakiran Tummala.
QA Contact: desale → stummala
Moving to mozilla0.9.3
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Anthony, seems like you know what the deal is here, over to you.
Assignee: jst → anthonyd
Status: ASSIGNED → NEW
Target Milestone: mozilla0.9.3 → mozilla1.0
setting to 0.9.3

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

anthonyd
Status: NEW → ASSIGNED
Target Milestone: mozilla1.0 → mozilla0.9.3
Target Milestone: mozilla0.9.3 → mozilla1.0
see 73552
Whiteboard: [C][range][embed]
--> kin
Assignee: anthonyd → kin
Status: ASSIGNED → NEW
Bulk move of mozilla1.0 bugs to mozilla.1.0.1. I will try to pull some of these
back in if I can.
Target Milestone: mozilla1.0 → mozilla1.0.1
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.
Priority: -- → P4
Target Milestone: mozilla1.0.1 → Future
This does make javascript manipulation of optgroup form elements a might bit
frustrating.
How does this affect optgroups at all??
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?
I might be able to work on this.  nsITextContent::SetParentAttr(nsIAttribute * parentAttr), maybe
Assignee: kinmoz → general
Priority: P4 → --
QA Contact: stummala → ian
Target Milestone: Future → ---
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.
Component: DOM: Core → DOM: Core & HTML
QA Contact: ian → general
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?
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.
Keywords: qawanted
Depends on: AttrExodus
Attached patch MochitestSplinter Review
Assignee: general → Ms2ger
Status: NEW → ASSIGNED
Attachment #515468 - Flags: review?(bzbarsky)
Why do we want that test?  Aren't we aiming to make Attr not be Node?
Yes! Let's not put any more work into Attr-nodes before we make them not be Nodes.
WFM
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Attachment #515468 - Flags: review?(bzbarsky)
Keywords: qawanted
You need to log in before you can comment on or make changes to this bug.