All users were logged out of Bugzilla on October 13th, 2018

Document == Document.documentElement.parentNode fails for created Documents

VERIFIED FIXED in mozilla0.8

Status

()

P3
normal
VERIFIED FIXED
19 years ago
10 years ago

People

(Reporter: bc, Assigned: jst)

Tracking

Trunk
mozilla0.8
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [HAVE FIX], URL)

Attachments

(2 attachments)

(Reporter)

Description

19 years ago
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; m18) Gecko/20000730
BuildID:    2000073020

Document == Document.documentElement.parentNode should be an invariant

When creating Documents with or without a DTD,
Document.documentElement.parentNode returns null instead of the Document.


Reproducible: Always
Steps to Reproduce:
1. doc = document.implementation.createDocument(null, 'doc', null);
2. alert(doc.documentElement.parentNode)


Actual Results:  returns null

Expected Results:  returns Document

The native HTMLDocument document does satisfy
document.documentElement.parentNode == document

Comment 1

19 years ago
Confirmed on Linux 2000080308.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

19 years ago
OS: Windows NT → All

Updated

18 years ago
QA Contact: gerardok → janc
(Assignee)

Comment 2

18 years ago
This bug has been marked "future" because the original netscape engineer
working on this is over-burdened. If you feel this is an error, that you or
another known resource will be working on this bug,or if it blocks your work
in some way -- please attach your concern to the bug for reconsideration.
Status: NEW → ASSIGNED
Hardware: PC → All
Target Milestone: --- → Future

Comment 3

18 years ago
Reproducible with Build: 2000090104

I'll attach a test case(based in the original report)

I'm not sure about the severity of this bug, but it looks to me like standards
compliance problem, maybe we must reconsider? ;)

Comment 4

18 years ago
Created attachment 13933 [details]
test case
(Assignee)

Comment 5

18 years ago
This patch fixes this bug.

Index: layout/xml/document/src/nsXMLDocument.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/xml/document/src/nsXMLDocument.cpp,v
retrieving revision 1.94
diff -u -r1.94 nsXMLDocument.cpp
--- nsXMLDocument.cpp   2001/01/05 00:35:20     1.94
+++ nsXMLDocument.cpp   2001/01/18 08:35:30
@@ -146,14 +146,18 @@

   if (aQualifiedName.Length() > 0) {
     nsCOMPtr<nsIDOMElement> root;
-    rv = doc->CreateElementNS(aNamespaceURI, aQualifiedName, getter_AddRefs(roo
t));
+    rv = doc->CreateElementNS(aNamespaceURI, aQualifiedName,
+                              getter_AddRefs(root));
+
     if (NS_SUCCEEDED(rv)) {
-      nsCOMPtr<nsIContent> content = do_QueryInterface(root);
-      doc->SetRootContent(content);
+      nsCOMPtr<nsIDOMNode> tmpNode;
+
+      doc->AppendChild(root, getter_AddRefs(tmpNode));
     }
   }

-  return doc->QueryInterface(NS_GET_IID(nsIDOMDocument), (void**) aInstancePtrR
esult);
+  return doc->QueryInterface(NS_GET_IID(nsIDOMDocument),
+                             (void**)aInstancePtrResult);
 }

Heikki, r=?
Whiteboard: [HAVE FIX]
Target Milestone: Future → mozilla0.8
Is doc a nsCOMPtr? If not, the final return would leak in the unlikely case that
QI failed. You need to explain to me why AppendChild() works and
SetRootContent() does not, and convince me that this does not break anything
else... Also, I think it would be better to return rv if it NS_FAILED(), in this
case it would be most likely because of out of memory situation. We really
should be returning errors in critical failures...
(Assignee)

Comment 7

18 years ago
Created attachment 23137 [details] [diff] [review]
Proposed fix
(Assignee)

Comment 8

18 years ago
AppendChild() will not fail in the current code, it doesn't do any allocations
or nothing, but you're right, I should've checked the return value and the new
diff does that, and I know AppendChild() will potentially do an allocation in
the near future.

The difference between AppendChild() and SetRootContent() is that AppendChild()
does the things needed when a node is added to the document, such as setting the
parent of the new node, setting the document in the new node and removing the
node from it's old parent and SetRootContent() is simply a root pointer setter
in our document implementations (read the source :-).

As for the potential memory leak if QI fails (which it doesn't), we do that in,
like, all our NS_NewXXX methods (except the ones I've fixed :-), I also fixed
this one, I added a kung fu death grip and removed the unnecessary QI.

Please have a look at the attached patch.
Now it looks good, r=heikki.
(Assignee)

Comment 10

18 years ago
Fix checked in, marking FIXED.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
Component: DOM Level 1 → DOM HTML

Comment 11

18 years ago
QA contact Update
QA Contact: janc → desale

Comment 12

18 years ago
Updating QA contact to Shivakiran Tummala.
QA Contact: desale → stummala

Comment 13

18 years ago
verified 
Status: RESOLVED → VERIFIED

Updated

10 years ago
Component: DOM: HTML → DOM: Core & HTML
QA Contact: stummala → general
You need to log in before you can comment on or make changes to this bug.