67732 - X font should recognize font subsetted (plane'd) groups

Status: VERIFIED FIXED

(Core :: Layout, defect, P1)

Description

[kill this account]

Font vendors are subsetting or plane'ing the fonts; eg: cns11643-1, cns11643-2, 
etc. Each plane contain glyphs for a range of the characters. 

When the user specifies the font to use for a language the display system should  
first try to find a glyph in any of the subsets of that foundry-family before 
looking for a fallback.

For example: if a user specifies "sun-ming-cns11643-1" as the font for 
traditional Chinese and the document has chars that are in plane 2 the display 
system should try to use "sun-ming-cns11643-2" before looking for a fallback.

For efficiency we probably want to have a list of the planes in a font that each 
nsFontNode can point to so chars in other planes can find the related fonts 
quickly.

Some of the changes probably need to go here:

        3017 nsFontMetricsGTK::FindGenericFont(PRUnichar aChar)
        3018 {
        3019   if (mTriedAllGenerics) {
        3020     return nsnull;
        3021   }
        3022   nsCAutoString prefix("font.name.");
        3023   prefix.Append(*mGeneric);
        3024   if (mLangGroup) {
        3025     nsCAutoString pref = prefix;
        3026     pref.Append(char('.'));
        3027     const PRUnichar* langGroup = nsnull;
        3028     mLangGroup->GetUnicode(&langGroup);
        3029     pref.AppendWithConversion(langGroup);
        3030     char* value = nsnull;
        3031     gPref->CopyCharPref(pref.GetBuffer(), &value);
        3032     nsCAutoString str;
        3033     nsFontGTK* font;
        3034     if (value) {
        3035       str = value;
        3036       nsMemory::Free(value);
        3037       value = nsnull;
        3038       font = TryNode(&str, aChar);
        3039       if (font && font->SupportsChar(aChar)) {
        3040         return font;
        3041       }
the char is not in primary plane so try the other planes
        3042     }
        3043     value = nsnull;

Comment 1

[Linda Baliman]

Sun considers this a show-stopper bug

Setting priority P1 and adding keywords intl and nsbeta1

Comment 2

[Masaki Katakai]

Yes, due to this problem, zh_TW plane2 can not be displayed properly.

Comment 3

[Masaki Katakai]

I've started debugging around the codes of gfx/gtk.

Comment 4

[Frank Tang]

Is that true you sometime also want some font in different base name be used
than other ? For example, you may want to try GBK font after you try GB2312
fonts, in this case, GBK font are not subplan of GB font.

Comment 5

[Brian Yuan]

Frank,
	I don't know whether you are asking me, but since I once asked Mozilla to use
GB2312/GBK/GB18030 fonts for Simplified Chinese locales, I am trying to answer
your question:

	Yes, for Simplified Chinese, most of the webpages are encoded in GB2312, so if
we use GB2312 fonts by default whenever it's available, the performance will be
much better, however, sometimes there do have a lot of webpages such as politicl
news are encoded in GBK, so GBK fonts should be used if any CJK characters
cannot be found in GB2312 fonts.
	Also GB18030 should be considerred whenever GBK font is not enough because more
and more CJK characters will be added into the new GB18030 standard.

Thanks.
Brian@Sun

Comment 6

[kill this account]

Attachment: patch that addes searches of sub-planes and related language groups

Comment 7

[kill this account]

Katakai: would you please review the attachment: 
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=33776

Comment 8

[Frank Tang]

r=ftang

Comment 9

[Masaki Katakai]

I've reviewed the codes and have verified it works
properly on Solaris and Linux platform. I tested
Japanese locale on Solaris 8, Kondara, TurboLinux,
LASER5 Linux and Redhat 7.

There is only one thing I found problem on RedHat6.2J.
On that linux distribution, invalid jisx0201 fonts called
wadalab fonts are provided and those fonts do not contain
any jisx0201 glyphs.

watanabe-mincho.ttf -wadalab-mincho-medium-r-normal--0-0-0-0-c-0-jisx0201.1976-0

It the same problem with Solaris's cns fonts, bug 67732.

I'm seeing this problem on default environment. The font is
picked up by font fallback. Can anyone in Netscape try this
patch on your Linux? My RedHat 6.2J is not official version.

I don't think this is serious because I can not see this
problem on Japanese RedHat 7. Also, it's problem in the
distribution side. The workaround is very easy that to disable
such invalid fonts.

Comment 10

[kill this account]

*** Bug 73697 has been marked as a duplicate of this bug. ***

Comment 11

[Christopher Blizzard (:blizzard)]

+FFRESubstituteCharset(nsCAutoString &aFFREName, char *aReplacementCharset)

If I remember correctly, passing ns*AutoString on the stack is usually a bad
idea.  But that might be only when you pass by value, not by reference as you
are doing here.

Other than that sr=blizzard

( adding brendan + scc to the cc list so they can comment on the auto string
issue that may or may not be an issue. )

Comment 12

[Brendan Eich [:brendan]]

These FFRESubstitute* functions are default-static, so there's no public API
issue, but nsCAutoString& parameter types are a bad pattern, and we don't want
anyone imitating them.  The type is too narrow: nsAWritableString& is the right
width, and it's a legal XPCOM parameter type (in IDL, out domstring).

/be

Comment 13

[kill this account]

Would it be okay if I change the nsCAutoString parameters to 
nsAWritableCString and and open a separate bug requesting widening the 
data width of these nsFontMetricsGTK.cpp internal-only convenience functions?

This is a fairly big patch already and the FFRE 
(Foundry-Family-Registry-Encoding font specifier) data is used in a lot of 
places. I'd like to get put the font-lang-group code to bed and deal with the 
data width issue separately if that is okay.

Comment 14

[Brendan Eich [:brendan]]

Sorry, I miscommunicated -- I only mean "wide" in the abstract-is-wider class
sense, not in the PRUnichar-is-wider-than-char sense.  So I only meant: use
nsAWritableCString&, but forgot to type the C!  Sorry again. 
sr=[e-mail] (to supplement blizzard's sr= and not let doubt remain)
if you just change the parameter type.

/be

Comment 15

[kill this account]

fix checked in

Comment 16

[Chris Petersen]

Marking verified per last comments

Comment 17

[Chris Petersen]

verified