Bug 20741 - REGRESSION: ISO-8859-8-I encoding is registered incorrectly
Summary: REGRESSION: ISO-8859-8-I encoding is registered incorrectly
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.5
: P1 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-09-09 03:12 PDT by Alexey Proskuryakov
Modified: 2008-09-10 09:46 PDT (History)
2 users (show)

See Also:


Attachments
proposed fix (15.41 KB, patch)
2008-09-10 04:58 PDT, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2008-09-09 03:12:24 PDT
After landing a fix for 17537, I've noticed that it causes an error during layout tests (but no tests actually fail):

ERROR: alias ISO-8859-8-I maps to ISO-8859-8-I already, but someone is trying to make it map to ISO-8859-8
(/Users/ap/Safari/OpenSource/WebCore/platform/text/TextEncodingRegistry.cpp:138 void WebCore::checkExistingName(const char*, const char*))

The fix is obvious, but I'll need help with making a layout test that fails because of this, if it's possible to make.
Comment 1 Alexey Proskuryakov 2008-09-10 04:58:12 PDT
Created attachment 23318 [details]
proposed fix

Many thanks to Dan for helping with the tests!
Comment 2 Darin Adler 2008-09-10 07:41:11 PDT
Comment on attachment 23318 [details]
proposed fix

> && strcasecmp(atomicName, "iso-8859-8") == 0)

I generally try to avoid POSIX functions that ignore case, since they are sensitive to the POSIX locale setting. Instead we make ASCII variants of those functions. Hence we use our own ASCIICType.h rather than ctype.h, etc. In this case, we're probably OK, but I'd rather see this follow that pattern.

> This was not an issue on the Mac, as
> we also support these via TEC (which we should stop doing)

I agree! I only included those in the TEC list because I thought ICU did not support them! Is there no downside to using the MIME names?

r=me
Comment 3 Alexey Proskuryakov 2008-09-10 09:46:06 PDT
Committed <http://trac.webkit.org/changeset/36319>.

(In reply to comment #2)
> Hence we use our own ASCIICType.h rather than ctype.h, etc. In this
> case, we're probably OK, but I'd rather see this follow that pattern.

I guess we should get rid of both strncasecmp and the recently added strcasecmp in wtf/StringExtras then. For now, I kept the code intact for consistency, as we are using this function elsewhere.

> Is there no downside to using the MIME names?

None that we know of (other than the risk of disturbing the encoding name system, as evidenced by this very bug).