Bug 215983 – Make Big5 encoder conform to the specification and behavior of Chrome and Firefox

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alex Christensen]

Make Big5 encoder conform to the specification and behavior of Chrome and Firefox

[Alex Christensen]

Created attachment 407562 [details]
Patch

[Darin Adler]

Comment on attachment 407562 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=407562&action=review

> Source/WebCore/platform/text/TextCodecICU.cpp:409
> +static Vector<uint8_t> eucJPEncode(StringView string, Function<void(UChar32, Vector<uint8_t>&)> unencodableHandler)

Repeating what I said last time: These functions don’t belong in TextCodecICU. The TextCodec architecture is designed to use separate TextCodec classes for each type of encoding/decoding. This table-based encoding should be a new class. I understand that we require it because of deficiencies in ICU but that does not mean it should be in the ICU source file and class.

Please consider making that change.

> Source/WebCore/platform/text/TextCodecICU.cpp:416
> +        UChar32 c = *iterator;

I prefer auto here.

> Source/WebCore/platform/text/TextCodecICU.cpp:464
> +        UChar32 c = *iterator;

auto

[Alex Christensen]

(In reply to Darin Adler from comment #2)
> Comment on attachment 407562 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=407562&action=review
> 
> > Source/WebCore/platform/text/TextCodecICU.cpp:409
> > +static Vector<uint8_t> eucJPEncode(StringView string, Function<void(UChar32, Vector<uint8_t>&)> unencodableHandler)
> 
> Repeating what I said last time: These functions don’t belong in
> TextCodecICU. The TextCodec architecture is designed to use separate
> TextCodec classes for each type of encoding/decoding. This table-based
> encoding should be a new class. I understand that we require it because of
> deficiencies in ICU but that does not mean it should be in the ICU source
> file and class.
Ah, I see what you're saying now.  I'm going to commit this here, then as my next step I'll implement decoding to make this into its own codec class.

[Alex Christensen]

http://trac.webkit.org/r266354

[Radar WebKit Bug Importer]

<rdar://problem/68078902>