Closed Bug 399284 Opened 17 years ago Closed 17 years ago

Some charsets have no langgroup defined

Categories

(Core :: Internationalization, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: smontagu, Assigned: smontagu)

References

Details

Attachments

(3 files)

Attached patch Patch (deleted) — Splinter Review
The patch is based on the output from grep LangGroup intl/uconv/src/charsetData.properties | cut -dL -f1 | sed -e "s/.$//" | sort -f > langgroups.txt grep NS_UCONV_REG_UNREG[^_] intl/uconv/src/nsUConvModule.cpp | cut -d \" -f2 | sort -f > charsets.txt diff -i --old-line-format="" --unchanged-line-format="" langgroups.txt charsets.txt
Attachment #284273 - Flags: review?(dbaron)
Comment on attachment 284273 [details] [diff] [review] Patch Should ISO-8859-10 be x-baltic or x-western? (I'm not sure I see the reason to distinguish in the first place, though.) I can't even find any information about what a few of these are (t.61-8bit). Nothing will be surprised to get x-gujr or x-guru, right? r=dbaron
Attachment #284273 - Flags: review?(dbaron) → review+
And could you add a test that fails if any charsets are missing based on the test in comment 0?
Attached patch Test (deleted) — Splinter Review
I think x-western is better than x-baltic for ISO-8859-10: it's supposed to cover "Nordic" languages, and we use x-western for Danish, Icelandic, Norwegian, and Swedish; and x-baltic for Estonian, Lithuanian and Latvian, which are covered by ISO-8859-4 and ISO-8859-13. (You may be right though that the distinction is moot: x-baltic and x-central-european are possibly both unnecessary). t.61-8bit is a kind of pan-European character set, see http://ra.dkuug.dk/i18n/charmaps/T.61-8BIT. I believe it's used by LDAP. x-gujr and x-guru shouldn't be a problem, we already use them for content tagged with lang="gu" and lang="pa[-IN]" respectively.
Comment on attachment 284273 [details] [diff] [review] Patch Requesting approval. This is a very low-risk change, and blocks bug 391868, which is a blocker. Even though it is marked fixed, the fix checked in is a fallback and this is the real fix for the issue. See comments there, especially bug 391868 comment 22 and bug 391868 comment 23
Attachment #284273 - Flags: approval1.9?
Comment on attachment 284613 [details] [diff] [review] Test Hmm, this test isn't great: it will happily pass even if the langGroup in the properties file is non-existent or misspelled. I'll add a pref check for font.size.variable.[langGroup]
Attached patch Another test (deleted) — Splinter Review
Slight change of plan: there are several langGroups that have no default font-family or font-size, and this isn't necessarily a problem as long as we fall back to something sane. This is a mochitest that checks that text in all charsets ends up with a non-zero font size.
Attachment #284273 - Flags: approval1.9? → approval1.9+
Checked in.
Status: NEW → RESOLVED
Closed: 17 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Depends on: 481692
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: