Bug 216219 – TextCodec refinements

• Status: RESOLVED
• Resolution: FIXED
• Component: Text

[Darin Adler]

TextCodec refinements

[Darin Adler]

Created attachment 408101 [details]
Patch

[Darin Adler]

Created attachment 408102 [details]
Patch

[Darin Adler]

Created attachment 408111 [details]
Patch

[Darin Adler]

Earlier versions of the patch had the BOM logic exactly backwards! Should be all better now.

[Darin Adler]

Created attachment 408115 [details]
Patch

[Darin Adler]

Comment on attachment 408115 [details]
Patch

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

> Source/WebCore/platform/text/TextCodecCJK.cpp:195
> +    auto range = std::equal_range(jis0212, jis0212 + std::size(jis0212), std::pair<uint16_t, UChar>(pointer, 0), [](const auto& a, const auto& b) {

Should use std::begin/end here instead of std::size.

[Darin Adler]

All green EWS now, so I guess it’s time for review.

[Sam Weinig]

Comment on attachment 408115 [details]
Patch

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

> Source/WebCore/platform/text/TextCodecCJK.cpp:542
> +            static constexpr std::array<UChar32, 63> iso2022JPKatakana {

Annoying that you have to provide the 63 here, but it looks like that is necessary if you want to use UChar32 explicitly, otherwise, you could do 

`static constexpr std::array iso2022JPKatakana { 0x3002, 0x300C, 0x300D ... }

and it would deduce the size for you.

> Source/WebCore/platform/text/TextCodecCJK.cpp:690
> +        auto table = new EUCKREncodingIndex;
> +        for (size_t i = 0; i < std::size(eucKRDecodingIndex); i++)
>              (*table)[i] = { eucKRDecodingIndex[i].second, eucKRDecodingIndex[i].first };

One of these days I will get around to adding a zip iterator to make this a bit nicer.

[Darin Adler]

Comment on attachment 408115 [details]
Patch

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

>> Source/WebCore/platform/text/TextCodecCJK.cpp:542
>> +            static constexpr std::array<UChar32, 63> iso2022JPKatakana {
> 
> Annoying that you have to provide the 63 here, but it looks like that is necessary if you want to use UChar32 explicitly, otherwise, you could do 
> 
> `static constexpr std::array iso2022JPKatakana { 0x3002, 0x300C, 0x300D ... }
> 
> and it would deduce the size for you.

I suspect that instead we could use a standard C array instead of a std::array here. Or maybe we should write this:

    std::array<UChar32, (0xFF9F - 0xFF61) + 1> ...

Or this:

    static_assert(std::size(iso2022JPKatakana) == (0xFF9F - 0xFF61) + 1);

[EWS]

Committed r266681: <https://trac.webkit.org/changeset/266681>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 408115 [details].

[Radar WebKit Bug Importer]

<rdar://problem/68431646>