[webkit-reviews] review granted: [Bug 25415] [GTK][ATK] Please implement support for get_text_at_offset : [Attachment 33259] textencoding.patch

[bugzilla-daemon at webkit.org]

Jan Alonzo <jmalonzo at gmail.com> has granted Xan Lopez <xan.lopez at gmail.com>'s
request for review:
Bug 25415: [GTK][ATK] Please implement support for get_text_at_offset
https://bugs.webkit.org/show_bug.cgi?id=25415

Attachment 33259: textencoding.patch
https://bugs.webkit.org/attachment.cgi?id=33259&action=review

------- Additional Comments from Jan Alonzo <jmalonzo at gmail.com>
> +static gchar* utf8_substr(const gchar* string, gint start, gint end)

Style - utf8Substr or similar.

>  {
>      gchar* startPtr = g_utf8_offset_to_pointer(string, start);
>      gsize lenInBytes = g_utf8_offset_to_pointer(string, end) -  startPtr +
1;
> @@ -581,27 +530,23 @@ static gchar* g_substr(const gchar* string, gint start,
gint end)
>  // internals of WebCore's text presentation.
>  static gchar* convertUniCharToUTF8(const UChar* characters, gint length, int
from, int to)
>  {
> -    gchar* utf8 = 0;
> -    gint newLength = 0;
> -    UTF16ToUTF8(characters, length, utf8, newLength);
> -    if (!utf8)
> -	   return NULL;
> -
> -    gchar *pos = g_substr(utf8, from, to);
> -    g_free(utf8);
> -    gint len = strlen(pos);
> +    CString stringUTF8 = UTF8Encoding().encode(characters, length,
QuestionMarksForUnencodables);
> +    gchar* utf8 = utf8_substr(stringUTF8.data(), from, to);
> +    if (!g_utf8_validate(utf8, -1, NULL))
> +	   return 0;

Please change utf8 to something like utf8String. Also maybe free utf8 as well?
It may not validate but that doesn't mean utf8 is NULL either.

> +    gint len = strlen(utf8);

Use gsize here too.

r=me.