Closed
Bug 399284
Opened 17 years ago
Closed 17 years ago
Some charsets have no langgroup defined
Categories
(Core :: Internationalization, defect)
Core
Internationalization
Tracking
()
RESOLVED
FIXED
People
(Reporter: smontagu, Assigned: smontagu)
References
Details
Attachments
(3 files)
(deleted),
patch
|
dbaron
:
review+
damons
:
approval1.9+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | 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 1•17 years ago
|
||
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+
Comment 2•17 years ago
|
||
And could you add a test that fails if any charsets are missing based on the test in comment 0?
Assignee | ||
Comment 3•17 years ago
|
||
Assignee | ||
Comment 4•17 years ago
|
||
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.
Assignee | ||
Comment 5•17 years ago
|
||
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?
Assignee | ||
Comment 6•17 years ago
|
||
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]
Assignee | ||
Comment 7•17 years ago
|
||
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.
Updated•17 years ago
|
Attachment #284273 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Comment 8•17 years ago
|
||
Checked in.
Status: NEW → RESOLVED
Closed: 17 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•