[webkit-reviews] review denied: [Bug 5264] document.createElementNS() should not allow to insert a second <html> element : [Attachment 8349] More complete patch

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Tue May 16 10:12:06 PDT 2006


Darin Adler <darin at apple.com> has denied Darin Adler <darin at apple.com>'s
request for review:
Bug 5264: document.createElementNS() should not allow to insert a second <html>
element
http://bugzilla.opendarwin.org/show_bug.cgi?id=5264

Attachment 8349: More complete patch
http://bugzilla.opendarwin.org/attachment.cgi?id=8349&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
Getting better!

ChangeLog for LayoutTests needs to mention changes to test results, not just
newly added tests. Patch should include expected results for new tests as well
as the tests themselves.

And as long as I'm looking at this code for the third time, I suggest
tightening it a tiny bit:

+	 case ELEMENT_NODE: {
+	     // Documents may contain a maximum of one Element child
+	     Node *c;
+	     for (c = firstChild(); c; c = c->nextSibling()) {
+		 if (c->isElementNode())
+		     return false;
+	     }
+	     return true;
+	 }
+	 case DOCUMENT_TYPE_NODE: {
+	     // Documents may contain a maximum of one DocumentType child
+	     Node *c;
+	     for (c = firstChild(); c; c = c->nextSibling()) {
+		 if (c->nodeType() == DOCUMENT_TYPE_NODE)
+		     return false;
+	     }
+	     return true;
+	 }

Like this:

	case DOCUMENT_TYPE_NODE:
	case ELEMENT_NODE:
	    // Documents may contain a maximum of one child of each of these
types.
	    // (One Element child and one DocumentType child.)
	    for (Node* c = firstChild(); c; c = c->nextSibling())
		if (c->nodeType() == type)
		    return false;
	    return true;



More information about the webkit-reviews mailing list