[webkit-reviews] review granted: [Bug 18270] treating x-user-defined different from windows-1252 breaks some Indian web sites : [Attachment 24281] patch with layout test

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Jungshik Shin <jshin at chromium.org>'s
request for review:
Bug 18270: treating x-user-defined different from windows-1252 breaks some
Indian web sites
https://bugs.webkit.org/show_bug.cgi?id=18270

Attachment 24281: patch with layout test
https://bugs.webkit.org/attachment.cgi?id=24281&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
+	 * ChangeLog:

The ChangeLog itself should not be mentioned in the ChangeLog.

The strcasecmp function is generally not good to use anywhere in WebKit because
its behavior depends on the current locale (as in the standard C setlocale
function). But I see it used in two other places inside the text encoding
machinery, so I suppose I can't reject this use. The correct thing to do would
be to make an ASCII version of this function like the functions in
<wtf/ASCIICType.h>, which exist for the same reason. It could maybe go in WTF,
perhaps <wtf/ASCIICString.h> or maybe in PlatformString.h as an overload of
equalIgnoringCase.

+    if (source == EncodingFromMetaTag
+	 && strcasecmp(encoding.name(), "x-user-defined") == 0)
+	 m_decoder.reset("windows-1252"); 
+    else if (source == EncodingFromMetaTag || source == EncodingFromXMLHeader
|| source == EncodingFromCSSCharset)	    

I think this first if would look better all on one line. It wouldn't be any
longer than the next if statement below it. I usually indent the "&&" one extra
level to avoid lining it up perfectly with the statement inside the if

r=me