[webkit-reviews] review denied: [Bug 74800] Perform case insensitive matching on MIME types in XHR : [Attachment 119862] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Jarred Nicholls <jarred at webkit.org>'s
request for review:
Bug 74800: Perform case insensitive matching on MIME types in XHR
https://bugs.webkit.org/show_bug.cgi?id=74800

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

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


Patch is OK, but I’d like to see one that takes a different approach.

> Source/WebCore/xml/XMLHttpRequest.cpp:227
> -	   bool isHTML = equalIgnoringCase(responseMIMEType(), "text/html");
> +	   bool isHTML = responseMIMEType() == "text/html";

Lets not change this. I don’t want to move to the approach where we lowercase
the MIME type.

Instead, we should use equalIgnoringCase in XMLHttpRequest::didReceiveData and
work around the isXMLMIMEType issue.

> Source/WebCore/xml/XMLHttpRequest.cpp:932
> +	   mimeType.makeLower();

I see this as a workaround for a bug in DOMImplementation::isXMLMIMEType so:

1) The lowercasing should be done inside XMLHttpRequest::responseIsXML.
2) A comment should mention that it's a workaround for that bug.
3) We should remove the lowercasing when we fix
DOMImplementation::isXMLMIMEType.