[webkit-reviews] review granted: [Bug 49714] Yensign hack should work with Shift_JIS encoding : [Attachment 74536] Patch 6

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Kent Tamura
<tkent at chromium.org>'s request for review:
Bug 49714: Yensign hack should work with Shift_JIS encoding
https://bugs.webkit.org/show_bug.cgi?id=49714

Attachment 74536: Patch 6
https://bugs.webkit.org/attachment.cgi?id=74536&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=74536&action=review

> WebCore/ChangeLog:10
> +	   IE chooses a font which shows a yensign for 0x5c code point for a
page
> +	   encoded in x-mac-japanese, ISO-2022-JP, EUC-JP, Shift_JIS,
Shift_JIS_X0213-2000,
> +	   x-sjis, and Windows-31J.

I thought IE didn't support Shift_JIS_X0213-2000.

> WebCore/ChangeLog:16
> +	   Also, we move the HashSet initialization for isJapanese() and
> +	   backslashAsCurrencySymbol() to TextEncodingRegistry.cpp because of
> +	   ease of making them multi-thread safe.

This is not a great reason - TextEncodingRegistry does not and should not care
about backslashes.

> WebCore/platform/text/TextEncoding.cpp:166
>  bool TextEncoding::isJapanese() const

This function is misplaced - all it is needed for is to make a decision on
whether to invoke charset detection. So it should be in TextResourceDecoder
(not something for this patch, of course).