Bug 171128 – Regression(r204605): support for "cp874" charset alias was inadvertently dropped which may cause issues on certain Thai sites

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Chris Dumez]

text on http://www1.odn.ne.jp/pasto_cbe26550/ looks garbled after r204605, when the system's default language is Japanese. This is because we inadvertently dropped support for "cp932" character encoding alias.

[Chris Dumez]

<rdar://problem/31526844>

[Chris Dumez]

Created attachment 307771 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 307771 [details]
Patch

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

> Source/WebCore/platform/text/TextCodecICU.cpp:96
> +DECLARE_ALIASES(windows_949, "euc-kr", "cseuckr", "csksc56011987", "iso-ir-149", "korean", "ks_c_5601-1987", "ks_c_5601-1989", "ksc5601", "ksc_5601", "ms949", "x-KSC5601", "x-windows-949", "x-uhc", "cp949");

This won't work - cp949 is a different encoding in ICU than CFString thinks it is.

I don't see why we would add support for non-standard encoding aliases here. Why not just special case them in defaultTextEncodingNameForSystemLanguage()? That's the only place where they should be supported, not in Web content.

[Chris Dumez]

Created attachment 308016 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 308016 [details]
Patch

r=me assuming that this is the same encoding that we used to map cp874 to in the past.

[Chris Dumez]

(In reply to Alexey Proskuryakov from comment #5)
> Comment on attachment 308016 [details]
> Patch
> 
> r=me assuming that this is the same encoding that we used to map cp874 to in
> the past.

I am no sure it is. If you look at http://demo.icu-project.org/icu-bin/convexp. It says that "cp874" alias was using ibm-874_P100-1995 internal encoder in ICU. However, it would now use windows-874-2000. I do not know how big an issue that is.

[Chris Dumez]

(In reply to Chris Dumez from comment #6)
> (In reply to Alexey Proskuryakov from comment #5)
> > Comment on attachment 308016 [details]
> > Patch
> > 
> > r=me assuming that this is the same encoding that we used to map cp874 to in
> > the past.
> 
> I am no sure it is. If you look at
> http://demo.icu-project.org/icu-bin/convexp. It says that "cp874" alias was
> using ibm-874_P100-1995 internal encoder in ICU. However, it would now use
> windows-874-2000. I do not know how big an issue that is.

http://demo.icu-project.org/icu-bin/convexp?conv=windows-874-2000&s=ALL
http://demo.icu-project.org/icu-bin/convexp?conv=ibm-874_P100-1995&s=ALL

[Alexey Proskuryakov]

The table attached to bug 160931 says "cp874: (got: windows-874)". Looking at the ICU links that you posted, and at r204605, I don't see how that could be the case.

But the table is accurate, then windows-874 is definitely the way to go.

[Alexey Proskuryakov]

> But the table

But *if* the table

[WebKit Commit Bot]

Comment on attachment 308016 [details]
Patch

Clearing flags on attachment: 308016

Committed r215711: <http://trac.webkit.org/changeset/215711>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.