[webkit-reviews] review granted: [Bug 6092] REGRESSION: dom/xhtml/level2/html//HTMLInputElement01.xhtml crashes : [Attachment 5093] keep current node ref'd in the XML tokenizer/parser as in the HTML parser

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Thu Dec 15 09:34:26 PST 2005


Eric Seidel <macdome at opendarwin.org> has granted Darin Adler
<darin at apple.com>'s request for review:
Bug 6092: REGRESSION: dom/xhtml/level2/html//HTMLInputElement01.xhtml crashes
http://bugzilla.opendarwin.org/show_bug.cgi?id=6092

Attachment 5093: keep current node ref'd in the XML tokenizer/parser as in the
HTML parser
http://bugzilla.opendarwin.org/attachment.cgi?id=5093&action=edit

------- Additional Comments from Eric Seidel <macdome at opendarwin.org>
Reading through it a second time for real:

~XMLTokenizer() has strange spacing.
setCurrentNode() (already mentioned) has strange spacing.

If startElementImpl fails to add a node, it can just call stopParsing() or? 
That woudl get rid of the need for your FIXMEs

     while (m_currentNode->implicitNode())
-	 m_currentNode = m_currentNode->parentNode();

could be re-written to use a local, and avoid the (admittedly minimal) refcount
thrash.

I'm not sure why it's not OK to clear the parent node in this case:
+    if (NodeImpl* par = m_currentNode->parentNode())
+	 setCurrentNode(par);

That code will never be reached (parsing would have aborted by now), but even
so, clearing m_currnetNode should be OK...

To solve your second FIXME in exitText, I think it's OK to stop parsing in
enterText in the failure case as well.

Otherwise looks fine.  Darin should land this.	r=me.



More information about the webkit-reviews mailing list