[Webkit-unassigned] [Bug 47896] [GTK] Use GCharsetConverter instead of g_iconv in TextCodecGtk

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=47896


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #71308|review?                     |review-
               Flag|                            |




--- Comment #4 from Martin Robinson <mrobinson at webkit.org>  2010-10-20 20:50:06 PST ---
(From update of attachment 71308)
View in context: https://bugs.webkit.org/attachment.cgi?id=71308&action=review

I have a couple more nits, but it's very close.

> WebCore/platform/text/gtk/TextCodecGtk.cpp:348
> +        ASSERT(m_iconvDecoder);
> +        if (!m_iconvDecoder) {

Sorry, I missed this before. Either we should remove the ASSERT here or not handle the failure case.

> WebCore/platform/text/gtk/TextCodecGtk.cpp:393
> +                // there is not enough input to fully determine what the conversion should produce
> +                // save it to a buffer to prepend it to the next input

Nice comment, but it should be a full sentence with punctuation, etc.

> WebCore/platform/text/gtk/TextCodecGtk.cpp:399
> +            } else if (g_error_matches(error.get(), G_IO_ERROR, G_IO_ERROR_NO_SPACE)) {
> +                bufferWasFull = true;
> +            } else if (g_error_matches(error.get(), G_IO_ERROR, G_IO_ERROR_INVALID_DATA)) {

WebKit style dictates that one-line blocks omit their surrounding curly braces (as much as I dislike it).

> WebCore/platform/text/gtk/TextCodecGtk.cpp:410
> +                m_numBufferedBytes = 0; // reset state for subsequent calls to decode

This comment should also be in sentence form.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.