172515 - Some nsFontLangGroup entries missing in X11 font code

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Roland Mainz]

[Per bug 78201 comment 51]:
Brian Stell wrote:
> would you add a font-lang group for arabic?
>
>  nsFontLangGroup FLG_JA =      { "ja", nsnull };
>  nsFontLangGroup FLG_KO =      { "ko", nsnull };
> +nsFontLangGroup FLG_AR =      { "ar", nsnull };
>  nsFontLangGroup FLG_NONE =    { nsnull , nsnull };

We'll miss a bunch of other nsFontLangGroup entries, too... ;-(

Comment 1

[Roland Mainz]

Suffering is all mine... ;-(

Comment 2

[Roland Mainz]

Attachment: Prototype patch for 2002-09-29-08-trunk

Prototype patch for getting from comments.
I am sure I am missing some things here - therefore I need the feedback from
the exports, e.g. {bstell, katakai, prabhat, smontagu, rbs, shanjian, etc.} ...

Comment 3

[Roland Mainz]

s/the exports/the experts/

Comment 4

[Roland Mainz]

bstell:
What about mapping "*-iso10646-1" fonts to the "x-unicode" langgroup ?

Comment 5

[Rui Xu]

code issue? QA to [e-mail] for now.

Comment 6

[kill this account]

The patch looks good to me. Do you have specific ideas on what else might be
needed?

> What about mapping "*-iso10646-1" fonts to the "x-unicode" langgroup

Unicode doesn't have a language associated with it so it would not helpful.

Comment 7

[Roland Mainz]

Brian Stell wrote:
> The patch looks good to me. Do you have specific ideas on what else might be
> needed?

The question is whether I catched all entries in |gCharSetMap[]|.
At least the following are still mapped to |FLG_NONE|:
1. "cp1251-1" is |FLG_WESTERN|, right ?
2. "microsoft-cp1251" is |FLG_WESTERN| too, right ?
3.
http://lxr.mozilla.org/seamonkey/source/intl/uconv/src/charsetData.properties#56
maps (adobe) sybol fonts to "greek", e.g.
symbol-fontspecific"&&"adobe-fontspecific" would be mapped to |FLG_GREEK| (IMHO
this is completely wrong, but "charsetData.properties" has a different meaning
here - who is right ? :)

> > What about mapping "*-iso10646-1" fonts to the "x-unicode" langgroup
>
> Unicode doesn't have a language associated with it so it would not helpful.

I know ... that's the reason why it is not in the patch, but someone added
"x-unicode" to "charsetData.properties" (see 
http://lxr.mozilla.org/seamonkey/source/intl/uconv/src/charsetData.properties#102
for example). I was wondering if I was missing something here...

Comment 8

[Roland Mainz]

bstell:
Offtopic:
Looking at
http://lxr.mozilla.org/seamonkey/source/gfx/src/gtk/nsFontMetricsGTK.cpp#466 ...
-- snip --
466   { "hpbig5-",    &FLG_ZHTW,    &Big5          },
467   { "hproc16-",   &FLG_NONE,    &Unknown       },
-- snip --
... the '-' chars at the end don't look right for me...
... should I fix that with this patch, too ?

Comment 9

[kill this account]

I thought cp1251 was Russian so but you will need to verify this with ftang.

I would not map symbol fonts to Greek.

What would you change the hpbig5- / hproc16- values to?

Comment 10

[Roland Mainz]

bstell wrote:
> I thought cp1251 was Russian so but you will need to verify this with ftang.

ftang ?

> I would not map symbol fonts to Greek.

That's my opinion, too... :)
Any idea why "charsetData.properties" maps them to greek ?

> What would you change the hpbig5- / hproc16- values to?

I would remove the '-' (e.g. s/"hpbig5-"/"hpbig5"/ and s/"hproc16-"/"hproc16"/).
Both entries are the only examples in the list which have such a trailing dash
and I wonder if this was a mistake/typo/etc. ...

Comment 11

[kill this account]

> Any idea why "charsetData.properties" maps them to greek ?

Sorry, no. You would need to ask Erik.

> > What would you change the hpbig5- / hproc16- values to?
> 
> I would remove the '-' (e.g. s/"hpbig5-"/"hpbig5"/ and /"hproc16-"/"hproc16"/).
> Both entries are the only examples in the list which have such a trailing dash
> and I wonder if this was a mistake/typo/etc. ...

The strings have 3 parts: registry, separater, and encoding: "registry-encoding"
These appear to have a blank encoding. Its my guess that these do not have a
"encoding" hence it is blank. I recommend not removing the dash.

Comment 12

[Roland Mainz]

I just checked it, cp1251 is cyrillc - new patch follows...

Comment 13

[Roland Mainz]

Attachment: Patch for 2002-09-29-08-trunk

Comment 14

[Roland Mainz]

Requesting r=/sr= ...

Comment 15

[kill this account]

r=[e-mail]

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 101966 [details] [diff] [review]
Patch for 2002-09-29-08-trunk

The code looks fine. Changing to "static" was a good idea. sr=roc+moz

Comment 17

[Simon Montagu :smontagu]

Fix checked in.