Remove TEC decoders that duplicate ICU decoders. Some of them define additional aliases, but we can just copy those. There's no need to implement encodings twice.
I would love to do this.
This is the relevant data for when Mac has duplicate encodings and possible extra names: EXTRA NAME: macOS WebKit knows extra nonstandard name isoceltic for ISO-8859-14 EXTRA NAME: macOS WebKit knows extra nonstandard name iso8859101992 for ISO-8859-10 EXTRA NAME: macOS WebKit knows extra nonstandard name isoir226 for ISO-8859-16 EXTRA NAME: macOS WebKit knows extra nonstandard name iso8859141998 for ISO-8859-14 EXTRA NAME: macOS WebKit knows extra nonstandard name iso8859162001 for ISO-8859-16 EXTRA NAME: macOS WebKit knows extra nonstandard name l10 for ISO-8859-16 EXTRA NAME: macOS WebKit knows extra nonstandard name isoir157 for ISO-8859-10 EXTRA NAME: macOS WebKit knows extra nonstandard name isoir199 for ISO-8859-14 EXTRA NAME: macOS WebKit knows extra nonstandard name latin10 for ISO-8859-16 EXTRA NAME: macOS WebKit knows extra nonstandard name latin8 for ISO-8859-14 EXTRA NAME: macOS WebKit knows extra nonstandard name l8 for ISO-8859-14 MULTIPLE CODECS: macOS WebKit supports encoding csisolatin6 through multiple implementations ['TEC', 'ICU'] MULTIPLE CODECS: macOS WebKit supports encoding iso-8859-16 through multiple implementations ['TEC', 'ICU'] MULTIPLE CODECS: macOS WebKit supports encoding iso-8859-14 through multiple implementations ['TEC', 'ICU'] MULTIPLE CODECS: macOS WebKit supports encoding iso-8859-11 through multiple implementations ['TEC', 'ICU'] MULTIPLE CODECS: macOS WebKit supports encoding iso-8859-10 through multiple implementations ['TEC', 'ICU'] MULTIPLE CODECS: macOS WebKit supports encoding latin6 through multiple implementations ['TEC', 'ICU'] MULTIPLE CODECS: macOS WebKit supports encoding koi8-u through multiple implementations ['TEC', 'ICU'] MULTIPLE CODECS: macOS WebKit supports encoding l6 through multiple implementations ['TEC', 'ICU']
Created attachment 326548 [details] Patch
Comment on attachment 326548 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=326548&action=review Fewer encodings in this category than I expected, but I understand that this is just one step on the path. > Source/WebCore/platform/text/TextEncodingRegistry.cpp:125 > + ASSERT_WITH_MESSAGE(!oldAtomicName, "Duplicate text encoding name %s for %s (previously registered as %s)", alias, atomicName, oldAtomicName); Technically, making this an assertion requires changing the #if above to ASSERT_DISABLED rather than ERROR_DISABLED. Makes me think our Assertions.h macros are a bit too complex.
Comment on attachment 326548 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=326548&action=review >> Source/WebCore/platform/text/TextEncodingRegistry.cpp:125 >> + ASSERT_WITH_MESSAGE(!oldAtomicName, "Duplicate text encoding name %s for %s (previously registered as %s)", alias, atomicName, oldAtomicName); > > Technically, making this an assertion requires changing the #if above to ASSERT_DISABLED rather than ERROR_DISABLED. Makes me think our Assertions.h macros are a bit too complex. Another way to do it is to get rid of this function and at the single call site put this one long line of code: ASSERT_WITH_MESSAGE(!textEncodingNameMap->contains(alias), "Duplicate text encoding name %s for %s (previously registered as %s)", alias, atomicName, textEncodingNameMap->get(alias)); We needed a function before only because the code was not a simple one-liner.
(In reply to Darin Adler from comment #5) > > Another way to do it is to get rid of this function and at the single call > site put this one long line of code: > > ASSERT_WITH_MESSAGE(!textEncodingNameMap->contains(alias), "Duplicate > text encoding name %s for %s (previously registered as %s)", alias, > atomicName, textEncodingNameMap->get(alias)); > > We needed a function before only because the code was not a simple one-liner. I like that suggestion, I'll do it that way.
Created attachment 326605 [details] Patch for landing
Comment on attachment 326605 [details] Patch for landing Clearing flags on attachment: 326605 Committed r224700: <https://trac.webkit.org/changeset/224700>
All reviewed patches have been landed. Closing bug.
This change broke the debug builds: /Volumes/Data/slave/highsierra-debug/build/Source/WebCore/platform/text/TextEncodingRegistry.cpp:140:150: error: format specifies type 'char *' but the argument has type 'bool' [-Werror,-Wformat] https://build.webkit.org/builders/Apple%20High%20Sierra%20Debug%20%28Build%29/builds/1292
http://trac.webkit.org/r224701
(In reply to Alex Christensen from comment #11) > http://trac.webkit.org/r224701 Thanks for fixing it for me. I probably should have let the final path run through EWS.
<rdar://problem/35562144>