[webkit-reviews] review denied: [Bug 13138] StringImpl::isLower calls |islower| for non-ASCII characters : [Attachment 13799] path

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Jungshik Shin
<jungshik.shin at gmail.com>'s request for review:
Bug 13138: StringImpl::isLower calls |islower| for non-ASCII characters
http://bugs.webkit.org/show_bug.cgi?id=13138

Attachment 13799: path
http://bugs.webkit.org/attachment.cgi?id=13799&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
This is not acceptable as-is. We need the high speed loop for all ASCII and the
loop can't have a branch in it or it won't be high speed. I suggest simply
and-ing with 0x7F before calling islower. We will ignore the result of islower
if the characters are not all ASCII.

    allLower &= islower(c & 0x7F);
    data[i] = tolower(c & 0x7F);


We use "FIXME:", not "XXX" for comments like that one about non-BMP.

+    // XXX Still broken for non-BMP characters represented in surrogate pairs

What non-BMP characters represented as surrogate pairs are lowercase? I don't
believe there are any. If there are, you should add a regression test to
demonstrate the symptom of the incorrect logic.