[webkit-reviews] review denied: [Bug 17537] Try MIME before trying IANA names when enumerating ICU converters : [Attachment 23219] patch (that passes webkit test)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Sep 7 05:02:55 PDT 2008


Alexey Proskuryakov <ap at webkit.org> has denied Jungshik Shin
<jungshik.shin at gmail.com>'s request for review:
Bug 17537: Try MIME before trying IANA names when enumerating ICU converters
https://bugs.webkit.org/show_bug.cgi?id=17537

Attachment 23219: patch (that passes webkit test)
https://bugs.webkit.org/attachment.cgi?id=23219&action=edit

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
OK, let's do this.

> Yeah, IBM codepage designations and MS-DOS codepage designations are not
> completely in sync.

Would it be possible to specify what exactly changes in this regard with this
patch, if only for posterity?

+2008-09-05  jungshik  <set EMAIL_ADDRESS environment variable>

Please fix this.

> I didn't use strcasecmp or stricmp because neither is portable

We can use strncasecmp from  wtf/StringExtras.h. Also, I think that this line
needs a comment - otherwise it's not clear from just looking at the code why we
use case-insensitive compare in this single case only.

I'm going to say r- for you to consider my comments, but really it's an r+, as
the only thing that absolutely must be fixed is the ChangeLog.


More information about the webkit-reviews mailing list