[webkit-reviews] review granted: [Bug 29103] typeAheadFind in select element is case-sensitive for Cyrillic and Greek : [Attachment 41558] The second quick fix for SelectElement::typeAheadFind()

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Hironori Bono <hbono at chromium.org>'s
request for review:
Bug 29103: typeAheadFind in select element is case-sensitive for Cyrillic and
Greek
https://bugs.webkit.org/show_bug.cgi?id=29103

Attachment 41558: The second quick fix for SelectElement::typeAheadFind()
https://bugs.webkit.org/attachment.cgi?id=41558&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +    // Retrive the folding equivalent of this prefix string before finding a
matching element.

Spelling error or typo here: "Retrieve".

I would call it the "case-folded equivalent"

> +    // This is a workaround that String::startWith() cannot fold non-ASCII
characters, so
> +    // this workaround is to be removed when the above problem is fixed.

My comment wording would be more like this:

    // Compute a case-folded copy of the prefix string before beginning the
search for
    // a matching element. This code uses foldCase to work around the fact that

    // String::startWith does not fold non-ASCII characters. This code can be
changed
    // to use startWith once that is fixed.

> +    String prefixFoldCase(prefix.foldCase());

I would call this prefixWithCaseFolded.

> +	   // Fold the option string and check its prefix is equal to the
folded prefix.

"check its prefix" -> "check if its prefix"

review+ since these are only comments on the comments -- you could land it with
your existing comments or fix them as I suggested.