| Summary: | TextCodec refinements | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Darin Adler <darin> | ||||||||||
| Component: | Text | Assignee: | Darin Adler <darin> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | achristensen, beidson, benjamin, cdumez, cmarcelo, esprehn+autocc, ews-watchlist, kangil.han, keith_miller, mark.lam, mmaxfield, msaboff, saam, sam, tzagallo, webkit-bug-importer | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | WebKit Nightly Build | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | All | ||||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=216280 | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Darin Adler
2020-09-05 14:44:42 PDT
Created attachment 408101 [details]
Patch
Created attachment 408102 [details]
Patch
Created attachment 408111 [details]
Patch
Earlier versions of the patch had the BOM logic exactly backwards! Should be all better now. Created attachment 408115 [details]
Patch
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. All green EWS now, so I guess it’s time for review. 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. 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); Committed r266681: <https://trac.webkit.org/changeset/266681> All reviewed patches have been landed. Closing bug and clearing flags on attachment 408115 [details]. |