[webkit-reviews] review denied: [Bug 88936] StringImpl::characters can return NULL for an empty string : [Attachment 160578] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied Myles C. Maxfield
<litherum at gmail.com>'s request for review:
Bug 88936: StringImpl::characters can return NULL for an empty string
https://bugs.webkit.org/show_bug.cgi?id=88936

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

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=160578&action=review


I would:
-modify Collator::collate to take two String or StringImpl instead of the raw
data
-add the special cases based on the string classes. Why do we even have to
invoke ucol_strcoll if one of the string is empty?
-Add a layout test and if possible add a test through Webkit test API.

Do we need ucol_strcoll if the two strings are 8bits anyway?

> Source/WTF/ChangeLog:8
> +	   Additional information of the change such as approach, rationale.
Please add per-function descriptions below (OOPS!).

No description.

> Source/WTF/wtf/unicode/icu/CollatorICU.cpp:98
> +

blank line.

> Source/WTF/wtf/unicode/icu/CollatorICU.cpp:99
> +    UChar dummy;

UChar dummy = 0;