[webkit-reviews] review granted: [Bug 11049] XMLHttpRequest always defaults Content-Type to application/xml, while it should depend on data type : [Attachment 169849] Patch 1

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Alexander Shalamov
<alexander.shalamov at gmail.com>'s request for review:
Bug 11049: XMLHttpRequest always defaults Content-Type to application/xml,
while it should depend on data type
https://bugs.webkit.org/show_bug.cgi?id=11049

Attachment 169849: Patch 1
https://bugs.webkit.org/attachment.cgi?id=169849&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=169849&action=review


Thanks for tackling this. Looks good. I have a few suggestions for improvement.


> Source/WebCore/xml/XMLHttpRequest.cpp:562
> +	   String documentMIMEType = document->isHTMLDocument() ? "text/html" :
"application/xml";

There is no need to use a String just to hold one or two different ASCII
literals. Should just use const char* instead.

Also seems unnecessary to do this out here since it’s only used if contentType
is empty. Should move this code closer to where it’s used.

> Source/WebCore/xml/XMLHttpRequest.cpp:592
> +	   if (!encoding.isValid() || encoding == UTF16BigEndianEncoding() ||
encoding == UTF16LittleEndianEncoding())

The function TextEncoding::closestByteBasedEquivalent() takes care of mapping
UTF-16 to UTF-8, and that is what we should use here instead of specifically
special casing UTF-16.