[webkit-reviews] review granted: [Bug 19215] REGRESSION: transformToDocument fails when xsl includes   : [Attachment 21358] proposed fix

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 19215: REGRESSION: transformToDocument fails when xsl includes &#160;
http://bugs.webkit.org/show_bug.cgi?id=19215

Attachment 21358: proposed fix
http://bugs.webkit.org/attachment.cgi?id=21358&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
+    Vector<UChar> s;
+    appendEscapedContent(s, make_pair(in.characters(), in.length()),
escapeNBSP);
+    return String::adopt(s);

I would use a longer name than "s" for the string buffer.

Do we want to have a path where we reuse the string when there is no text to
escape?

+	     // FIXME: Comment content is not escaped, but some APIs calling
this should raise an exception if it includes "-->".

I don't know what "some APIs calling this" refers to.

+	 case Node::DOCUMENT_TYPE_NODE: {
+	     const DocumentType* n = static_cast<const DocumentType*>(node);

Moving the code inside this file seems fine, but putting it all inside the case
statement instead of factored into a separate function seems like a step
backwards.

     if (Document *doc = document())
	 if (DocumentType *doctype = doc->doctype())
-	     return doctype->toString();
+	     return createMarkup(doctype);
 
     return String();

Should use early returns instead of nested ifs. Or add braces if leaving this
nested.

r=me