120401 - Font selection in pref. does not work (Hebrew, Arabic, CE, Baltic, Cyrillic, CJK, etc)

Status: RESOLVED FIXED

(Core Graveyard :: GFX: Mac, defect, P1)

Description

[digiart]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Macintosh; U; PPC; en-US; rv:0.9.7+) Gecko/20020111
BuildID:    200201108

When  trying to change fonts for display of text in Hebrew the change is not
reflected in the page display, and only two fonts can be chosen: Raanana Hebrew
and Ariel Hebrew. It doesn't matter what other font you choose, the result keeps
defaulting to Ariel Hebrew. 

Reproducible: Always
Steps to Reproduce:
1. Edit>Pereferences, Fonts section.
2.Change the current font under Hebrew to any font you like. 
3. Reload the page (or it reloads automatically).

Actual Results:  No change was visible in the text, except for when I chose
Raanana Hebrew and then the text changed to Raanana Hebrew.

Expected Results:  The font should be changed to the font chosen. Also, the
drawing of the font at different sizes should consider the bitmap version (which
it seems not to be doing).

Comment 1

[Simon Montagu :smontagu]

This might be a dupe of bug 110655, modulo the different Mac versions.

Comment 2

[Shoshannah Forbes]

Yep, I see the same behaver with build 2002012903

Comment 3

[Shoshannah Forbes]

The yep was for the behavier orignly reported, not for a dupe (this does not
look like a dup of that bug). Looks like only two fonts work in Hebrew- raanana
and arial. setting any other font gets ignored.

Comment 4

[Shoshannah Forbes]

I am seeing similar behavier with windows 20000/ build 2002032003: Mozilla uses
David and arial, regardless of the font I set in prefrences for Hebrew.

Comment 5

[Shoshannah Forbes]

changed url to a site with no font defenition (and fixed a typo)

Comment 6

[Prognathous]

Attachment: Win32 Mozilla using Tahoma font

Comment 7

[Prognathous]

> I am seeing similar behavier with windows 20000/ build 
> 2002032003: Mozilla uses David and arial, regardless of the 
> font I set in prefrences for Hebrew.

As is clearly displayed by the attached screenshot, this is no longer the case.
Font settings also work fine with Mozilla for BeOS.
Could someone verify that recent builds for MacOS still display this problem?

Prog.

Comment 8

[Prognathous]

CCing Shoshannah Forbes.

Comment 9

[Shoshannah Forbes]

Can't check this bug on osx due to bug 110655.
This still needs to be checked on os9, which I don't have at the moment

Comment 10

[Shoshannah Forbes]

Mozilla id 2003052208 on osx 10.2.6:  
althugh now bug #110655 is fixed, I can change the font. However, regardless
what I change the font to, it styes the default and ignores my settings.

So in practice, setting user fonts is not functional at all for Hebrew under osx

Comment 11

[rbs]

re: comment 4
Do you still see this on Windows?

Comment 12

[Shoshannah Forbes]

re: Comment #11 :
I do not have access to a windows box at the moment. Can anyone else test this,
so we can tell if this is a general bug or a Mac only one?

As the major mac blocker is fixed now, the end user still can't enjoy it due to
this bug.

Comment 13

[Prognathous]

Works fine under WinXP (1.4b/20030507) with Arial, Tahoma, Courier New, David
and Miriam. I guess it's a Mac-only issue.

Prog.

Comment 14

[rbs]

simon, any clues as to why the pref is not taking effect on the Mac. It looks
like an an oversight somewhere. Now that bug 110655 has been fixed, it would be
nice if Mac people can enjoy their own fonts.

Comment 15

[Shoshannah Forbes]

This is still broken, making testing of bugs like bug 144157 hard, as I don't
have control over the font used.

Comment 16

[Shoshannah Forbes]

perhaps bug #95978 is related?

Comment 17

[Shoshannah Forbes]

together with bug #132341, surfing Hebrew web pages on mac using gecko is really
problematic- nither the site owner nor the user can set any font.

Comment 18

[rbs]

Can any mac enthusiast try again with a more recent build?
See bug 235089 which said that fonts from prefs were ignored due to bad cast.

Comment 19

[Nir Soffer]

For me the only font used is Lucida Grande which is the default system font. No matter what 
other font I set in the preferences - only Lucida Grander is used.

Mac OS X 10.2.8, Mozilla 1.7a

Comment 20

[Asaf Romano (gone)]

Still annoying with current nightlies, any progress?

Comment 21

[Asaf Romano (gone)]

As Simon Montague suggested me, I built SeaMonkey with DDEBUG_ftang_font defined
(in order to sea the output of this method:
http://lxr.mozilla.org/seamonkey/source/gfx/src/mac/nsUnicodeMappingUtil.cpp#359 )

When I started mozilla at the first time (with no exist profile), font fields in
prefs panel are empty (for hebrew), this is the log:
===== Fri May 21 2004 ===== 06:35:31 Africa/Cairo =====
(unrelated log, problem with my JavaPluging)

font 0 2 font.name.monospace.x-western= Courier

font 32 1 font.name.sans-serif.x-unicode= Helvetica

font 3 1 font.name.sans-serif.ko= AppleGothic

font 2 2 font.name.monospace.zh-HK= Apple LiGothic Medium

font 1 1 font.name.sans-serif.ja= ヒラギノ角ゴ Pro W3

font 1 0 font.name.serif.ja= ヒラギノ明朝 Pro W3

font 3 2 font.name.monospace.ko= AppleGothic

font 2 1 font.name.sans-serif.zh-TW= Apple LiGothic Medium

font 2 1 font.name.sans-serif.zh-HK= Apple LiGothic Medium

font 29 2 font.name.monospace.x-central-euro= Courier CE

font 25 2 font.name.monospace.zh-CN= Hei

font 0 1 font.name.sans-serif.x-western= Helvetica

font 25 0 font.name.serif.zh-CN= Song

font 32 2 font.name.monospace.x-unicode= Courier

font 2 2 font.name.monospace.zh-TW= Apple LiGothic Medium

font 0 3 font.name.cursive.x-western= Apple Chancery

font 2 0 font.name.serif.zh-TW= Apple LiSung Light

font 32 0 font.name.serif.x-unicode= Times

font 29 1 font.name.sans-serif.x-central-euro= Helvetica CE

font 1 2 font.name.monospace.ja= Osaka−等幅

font 32 3 font.name.cursive.x-unicode= Apple Chancery

font 29 0 font.name.serif.x-central-euro= Times CE

font 0 0 font.name.serif.x-western= Times

font 3 0 font.name.serif.ko= AppleMyungjo

font 25 1 font.name.sans-serif.zh-CN= Hei
--------------------------------------------------------
Then I setted fonts in the hebrew fonts pannel and relaunched mozilla, that is
the log at this time:
===== Fri May 21 2004 ===== 06:40:51 Africa/Cairo =====
(unrelated log, problem with my JavaPluging)

font 0 2 font.name.monospace.x-western= Courier

font 32 1 font.name.sans-serif.x-unicode= Helvetica

font 3 1 font.name.sans-serif.ko= AppleGothic

font 2 2 font.name.monospace.zh-HK= Apple LiGothic Medium

font 1 1 font.name.sans-serif.ja= ヒラギノ角ゴ Pro W3

font 1 0 font.name.serif.ja= ヒラギノ明朝 Pro W3

font 3 2 font.name.monospace.ko= AppleGothic

font 2 1 font.name.sans-serif.zh-TW= Apple LiGothic Medium

font 2 1 font.name.sans-serif.zh-HK= Apple LiGothic Medium

font 0 4 font.name.fantasy.x-western= Abadi MT Condensed Extra Bold

font 29 2 font.name.monospace.x-central-euro= Courier CE

font 25 2 font.name.monospace.zh-CN= Hei

font 0 1 font.name.sans-serif.x-western= Helvetica

font 25 0 font.name.serif.zh-CN= Song

font 32 2 font.name.monospace.x-unicode= Courier

font 2 2 font.name.monospace.zh-TW= Apple LiGothic Medium

font 0 3 font.name.cursive.x-western= Apple Chancery

font 2 0 font.name.serif.zh-TW= Apple LiSung Light

font 32 0 font.name.serif.x-unicode= Times

font 29 1 font.name.sans-serif.x-central-euro= Helvetica CE

font 1 2 font.name.monospace.ja= Osaka−等幅

font 32 3 font.name.cursive.x-unicode= Apple Chancery

font 29 0 font.name.serif.x-central-euro= Times CE

font 0 0 font.name.serif.x-western= Times

font 3 0 font.name.serif.ko= AppleMyungjo

font 25 1 font.name.sans-serif.zh-CN= Hei

--------------------------------------------------------
As you can see in both cases hebrew is not mentioned at all (and it still there
(in the prefs pane)). Just to be clear, when I go to about:config, they do
appear there (font.name.monospace.he, font.name.sans-serif.he, font.name.serif.he).

Comment 22

[Asaf Romano (gone)]

More interesting information and some direction: After I set the hebrew fonts, I
relaunched my debug build while stepping into
nsUnicodeMappingUtil::PrefEnumCallback
(http://lxr.mozilla.org/seamonkey/source/gfx/src/mac/nsUnicodeMappingUtil.cpp#295).
For some reason (I haven't checked), every time the method is called with aName
is ".*\.he", the method being finshed in this condition
(http://lxr.mozilla.org/seamonkey/source/gfx/src/mac/nsUnicodeMappingUtil.cpp#350):
  if( (! nsDeviceContextMac::GetMacFontNumber(*fontname, fontID)) ||
  	  ((script < smUninterp) && (::FontToScript(fontID) != script)))

Odd that it is always_true_for_hebrew, this ofcourse turns the method into this:
  {
  	delete fontname;
  	return;
  }

:-/

Comment 23

[rbs]

Good job, this is the most useful information I have seen on this problem so far. 

Of the choices in the |if|, try to find the specific one that is at fault.
e.g., is it |GetMacFontNumber| that is failing?
http://lxr.mozilla.org/mozilla/source/gfx/src/mac/nsDeviceContextMac.cpp#947
 

Comment 24

[Simon Montagu :smontagu]

I had thought that this bug was just with Hebrew, but there are some very
similar issues with other languages in bug 180914 and bug 157272 (maybe also bug
212745?).  I think that Asaf is right in bug 171519 comment 21, and the fix in
bug 130443 should have been applied to GetMacFontNumber() as well. See bug 95978
comment 23 (the original home of the patch in bug 130443). We need to audit all
clients of the gFontInfoList hashtable in nsDeviceContextMac.

Comment 25

[Simon Montagu :smontagu]

Asaf pointed me to bug 95557, which basically covers what I said in the last
comment.

Comment 26

[Asaf Romano (gone)]

(In reply to comment #25)
> Asaf pointed me to bug 95557, which basically covers what I said in the last
> comment.

hmm... I don't think it is a dependence, so far, the problamtic method
(nsUnicodeMappingUtil::PrefEnumCallback) doesn't use  FontToScript at all.

Comment 27

[Asaf Romano (gone)]

Anybody knows a Crabon API to check if a font is a unicode one?

Simon found something here:
http://bugzilla.mozilla.org/attachment.cgi?id=55346&action=view
(which is a patch for bug 90804)
but it's really nasty :-/

Comment 28

[Jungshik Shin]

In light of comment #24, this bug is really serious (not quite a blocker but
surely a major bug). 


> Anybody knows a Crabon API to check if a font is a unicode one?

If nobody has figured it out, I'll ask John Jenkins at Apple. By 'Unicode font',
you meant a truetype/opentype font with Unicode cmap, right? 

Comment 29

[Asaf Romano (gone)]

change status to ASSIGNED. I`ll upload a patch soon.

Comment 30

[Asaf Romano (gone)]

Changing Summary to both Hebrew & Arabic, In fact this is relevant for every
Non-Roman language, but they also can't choose a font (see bug 95978).

Comment 31

[Jungshik Shin]

There's gonna be a patch that will fix most of font selection issues regardless
of the language group.  Let's make the summary reflect that. 

Comment 32

[Jungshik Shin]

This is  a very serious bug and should block 1.8a2 if not 1.7.  I'm not sure if
it makes sense to ask for making this bug block 1.7  (not because this bug is
not critical enough but because it's too late in the cycle). 

Comment 33

[Asaf Romano (gone)]

Jungshik,Do you know where the default fonts for the *UI* and for form widgets
are decalred?? this s the only problem left (I currently change them via
userChrome.css and userContent.css).

Comment 34

[Asaf Romano (gone)]

Also 155551 

Comment 35

[Jungshik Shin]

(In reply to comment #33)
> Jungshik,Do you know where the default fonts for the *UI* and for form widgets
> are decalred?? this s the only problem left.

I think that has to be a separate bug. I'm not familiar with Gfx:Mac. On Linux,
we take the system-wide default value. And, the same is true of Gfx:Win, I
guess. If the same is the case on Mac, that's definitely a separate bug. Anyway,
why don't you upload your patch so that I and others can play with it? Btw,
thanks for the patch.

Comment 36

[Asaf Romano (gone)]

(In reply to comment #35)
> (In reply to comment #33)
> > Jungshik,Do you know where the default fonts for the *UI* and for form widgets
> > are decalred?? this s the only problem left.
> 
> I think that has to be a separate bug. I'm not familiar with Gfx:Mac. On Linux,
> we take the system-wide default value. And, the same is true of Gfx:Win, I
> guess. If the same is the case on Mac, that's definitely a separate bug. Anyway,
> why don't you upload your patch so that I and others can play with it? Btw,
> thanks for the patch.
> 

I'm not talking about that. I'm searching for a css.. In needs to be something
in XPFE for Mozilla and toolbox for firefox, but i don't now what is the exact file.

I'm near other mac, and i'm waiting for mozilla to finish the build, after that
the patch will be here (about an hour)

After solving the bug I must force the apps to use the system font in UI and
form widgets (maybe platforms-forms.css or near), if you don't want to see just
"?"s.

Comment 37

[Asaf Romano (gone)]

Well i didn't mean system font. i mean just "font" that exist, current bug
removes an option to a bug that has fund after applying the patch.

BTW, will you be able to r/sr it?

Comment 38

[Jungshik Shin]

> I'm not talking about that. I'm searching for a css.. In needs to be something
> in XPFE for Mozilla and toolbox for firefox, but i don't now what is the exact
file.

If there's such a thing, I guess it's not supposed to list specific instances of
fonts. Instead, it's gonna just list generic fonts. 

Btw, I'm not qualified to give r to a patch for Gfx:Mac. I'm adding sfraser to
CC for that. You can ask rbs for sr. 

Comment 39

[Asaf Romano (gone)]

Attachment: Patch for the current implmentation of gfx/src/mac

In the futre we should update all gfx/src/mac to the days of Mac OS X, where
WorldScript doesn't exist

For now, I just #IFDEF XP_MAC the parts which don't need to be in this file
anymore, ye I know CFM has dead a long time ago, but until Mac GFX will be
rewriten, that's how it should be.

I also changed the default Hebrew & Arabic fonts to Lucida Granade, the hard
coded fonts are no longer exist on Mac OS X (Sorry that it has to be for both
Serif and SansSerif, There is a bug on OS X, were Carbon Apss can't see all the
fonts when scaning them by font-famlies, and Arial Hebrew and Ranana can't be
used until we workaround this bug someday..).

One thing the system foesn't do is to replace non-exist fonts with Lucida
Grande (as mentioned in patch, it does it when trying to render text with a
font that doesn't contain needed glyphs), as a result of that, there is a
*MAJOR* problem where the non-roman text in UI & form-widgets (except of
textarea) is drawn as "?"s, because of the current bug we didn't see this bug
(font always reverted to Lucida Grande). It seems they don't take the prefs
(only ui and form fields) from all.js. In my build, I workaround it using the
following changes:
1. In userChrome.css:
*
{
    font-family: Lucida Grande;
}

2. In userContent.css:
input, select
{
    font-family: Lucida Grande;
}

Ofcourse we can add that to the default CSS(s), because it doesn't overide
anything, and Lucida Granade is used anyway for roman text, but I don't know
where this CSS's are located in the tree.

Comment 40

[Asaf Romano (gone)]

One more thing:
If someone knows a place mozilla defines default fonts for mac UI, please notice
me asap, I will apply the changes and upload a new patch, that's the only thing
that block us.

Comment 41

[Asaf Romano (gone)]

Blocking 1.7=- 
This is the kind of code that needs both testing and QA, no time for 1.7 (but
maybe for the AVIARY BRANCH after shipping the upcoming versions of Firefox &
Thunderbird).

Comment 42

[Nir Soffer]

(In reply to comment #39)
> Serif and SansSerif, There is a bug on OS X, were Carbon Apss can't see all the
> fonts when scaning them by font-famlies, and Arial Hebrew and Ranana can't be
> used until we workaround this bug someday..).

This is the bug Asaf is talking about:
http://bugzilla.mozilla.org/show_bug.cgi?id=246527

Comment 43

[Jungshik Shin]

Thanks for the patch. I didn't realize that Gfx:Mac is this much behind Gfx:Win.
It seems like Gfx:Mac hasn't changed much since 1999. Hard-coding the generic
font mapping is .... (I'd not say) 

http://lxr.mozilla.org/seamonkey/source/gfx/src/mac/nsFontMetricsMac.cpp#124
http://lxr.mozilla.org/seamonkey/source/gfx/src/mac/nsUnicodeMappingUtil.cpp#129

http://lxr.mozilla.org/seamonkey/source/gfx/src/mac/nsFontMetricsMac.cpp#147

As you commented, we have to move away from the world script (used in OS9 or
earlier) asap. Nobody seems to have attempted that in earnest.

Comment 44

[Hiro]

(In reply to comment #39)
> Created an attachment (id=150610)
> Patch for the current implmentation of gfx/src/mac
> I workaround it using the
> following changes:
> 1. In userChrome.css:
> *
> {
>     font-family: Lucida Grande;
> }
> 
> 2. In userContent.css:
> input, select
> {
>     font-family: Lucida Grande;
> }
> 
> Ofcourse we can add that to the default CSS(s), because it doesn't overide
> anything, and Lucida Granade is used anyway for roman text, but I don't know
> where this CSS's are located in the tree.

Now, there is a problem of bug233781 in the Japanese environment of OSX.
Is this problem also repaired if bug120401 fixes?

Comment 45

[Asaf Romano (gone)]

I'm working on that, it seems that there is a problem in GetSystemFont which tries to search for the 
system-script. and accordng to it's results it defines the system-font (for all scripts...).

BTW, it will be nice if u can tell me what are the expected default fonts for Japanese (for serif and for 
sans-serif).

Comment 46

[Jungshik Shin]

With the patch applied, the font pref. does have an effect. However, Mozilla has
a trouble figuring out the character coverage of a font thus selected. For
instance, UnBatang font(http://chem.skku.ac.kr/~wkpark/projec/font/UnFont) has
glyphs for all 11,172 Korean syllables, but Mozilla on MacOS X appears to think
that it only covers 8,822 syllables that are NOT a part of MacKorean. It seems
like Mozilla still relies on the presence of (a) Mac-specific (and
script-specific) truetype cmap table(s). Because the font doesn't have MacKorean
truetype cmap, it doesn't recognize that it covers 2,350 Korean syllables. On
the other hand, Safari doesn't have any problem with the font. Therefore, in
addition to the patch here, I guess what Simon mentioned in comment #24 and
comment #25 have to be done. 

Comment 47

[Asaf Romano (gone)]

(In reply to comment #46)
> With the patch applied, the font pref. does have an effect. However, Mozilla has
> a trouble figuring out the character coverage of a font thus selected. For
> instance, UnBatang font(http://chem.skku.ac.kr/~wkpark/projec/font/UnFont) has
> glyphs for all 11,172 Korean syllables, but Mozilla on MacOS X appears to think
> that it only covers 8,822 syllables that are NOT a part of MacKorean. It seems
> like Mozilla still relies on the presence of (a) Mac-specific (and
> script-specific) truetype cmap table(s). Because the font doesn't have MacKorean
> truetype cmap, it doesn't recognize that it covers 2,350 Korean syllables. On
> the other hand, Safari doesn't have any problem with the font. Therefore, in
> addition to the patch here, I guess what Simon mentioned in comment #24 and
> comment #25 have to be done. 

"Think" or you see thatg in action? (meaningis there pages you see some glyhs
render in font other than the one you specified?)

(2) The bug mention in comment 25 is INVALID (imo...), The API suggested there
is broken.
(3) There is no need for this change, OS X does the work for you.. we just need
to remove the code that thinks it knows better than the OS, what font can render
what...
(4) I will be back to this bug in a day or two, currently busy with personal
stuff...

I'm happy you can check this patch (and the patches that will become here
later...) with other languages other than hebrew.. I don't have this ability :-)
So you are very welcome to do more QA

Comment 48

[Jungshik Shin]

(In reply to comment #47)

> 
> "Think" or you see thatg in action? (meaningis there pages you see some glyhs
> render in font other than the one you specified?)

Without your patch, 2,350 syllables (included in MacKorean) are always rendered
with AppleGothic (or other hard-coded fonts that have glyphs for only 2350
syllables) while 8,822 syllables (not covered by MacKorean but included in
Unicode) are rendered with a font other than AppleGothic. 

With your patch applied and the font (covering the full set of Korean syllables,
11,172 of them) selected for Korean, 2,350 syllables are rendered with hollow
boxes while 8,822 are rendered with the font selected. 

Comment 49

[Asaf Romano (gone)]

> With your patch applied and the font (covering the full set of Korean syllables,
> 11,172 of them) selected for Korean, 2,350 syllables are rendered with hollow
> boxes while 8,822 are rendered with the font selected. 
> 
Looks like the same problem with UI, to confirm this, can you check what happens
when your'e trying to use the font via userContent.css?

Comment 50

[Asaf Romano (gone)]

Attachment: Patch for QA

Thanks to Simon, the UI fonts problem is behind us (and considering the change,
it will probably solve other problems, maybe for some of the dependencies).

Jungshik, it should also fix your problem (at least you won't see "boxes" :-)
).

Soon, i will start removing code from this file, but if no major problems are
found in *this* patch, you will get the same result (Although i haven't
"removed" yet, it just doesn't mean anything for now (e.g. ScriptEnabled will
always return true...) ). 

This need some QA, your help is welcome!

Comment 51

[Asaf Romano (gone)]

Test build with the last patch:
http://www.miloberi.com/pub/firefox-powerpc-apple-darwin7.4.0.dmg.gz

Comment 52

[Prognathous]

I tested your build and my initial findings are that changes to fonts settings
do indeed "stick", but fonts with Hebrew glyphs don't appear in the list. I
looked for Arial Hebrew, New Peninim and Raanana, but none were listed. In other
words, I can't get the test URL to render using other Hebrew fonts.

Prog.

Comment 53

[Asaf Romano (gone)]

(In reply to comment #52)
> I tested your build and my initial findings are that changes to fonts settings
> do indeed "stick", but fonts with Hebrew glyphs don't appear in the list. I
> looked for Arial Hebrew, New Peninim and Raanana, but none were listed. In other
> words, I can't get the test URL to render using other Hebrew fonts.
> 
> Prog.
> 
> 
This is bug 246527 (again apple...), You should try Arial from Windows.

Comment 54

[Asaf Romano (gone)]

btw Hebrew, it also fixes bug 171519.

Comment 55

[Jungshik Shin]

(In reply to comment #50)
> Created an attachment (id=151830)

> Jungshik, it should also fix your problem (at least you won't see "boxes" :-)

Sorry it doesn't make any difference at all. Korean fonts I've been trying don't
have any Apple Cmap but only have MS Cmap tables, which shouldn't be a problem.
However, apparently, it does present a problem to Mozilla. 

Comment 56

[Asaf Romano (gone)]

(In reply to comment #55)
> (In reply to comment #50)
> > Created an attachment (id=151830)
> 
> > Jungshik, it should also fix your problem (at least you won't see "boxes" :-)
> 
> Sorry it doesn't make any difference at all. Korean fonts I've been trying don't
> have any Apple Cmap but only have MS Cmap tables, which shouldn't be a problem.
> However, apparently, it does present a problem to Mozilla. 
> 

1. Do you still see "boxes"? What happens if you choose non-windows font?
2. can you give a test-page+font.

Comment 57

[Jungshik Shin]

(In reply to comment #56)
> (In reply to comment #55)
> > (In reply to comment #50)

> > Sorry it doesn't make any difference at all. Korean fonts I've been trying don't
> > have any Apple Cmap but only have MS Cmap tables, which shouldn't be a problem.
> > However, apparently, it does present a problem to Mozilla. 
> > 
> 
> 1. Do you still see "boxes"? What happens if you choose non-windows font?

  Apple Korean cmap cannot cover the full repertoire of Korean syllables in
Unicode. To do that, a font need to have either Apple Unicode cmap or MS Unicode
cmap. Fonts I have tried have only MS Unicode cmap (PID=3, EID=1) along with MS
CP949 Cmap). Anyway, adding Apple Korean cmap to them, I guess, would solve the
problem (I'll add Apple Korean cmap to them later, but this week I'm very busy
so that it'll take a while) However, Mozilla should be fixed to handle fonts
without Apple cmap tables. As I wrote, Safari doesn't have a problem with such a
font (probably because it uses ATSUI APIs and doesn't rely on old APIs at all.)
Perhaps, this issue should be a separate bug. 

> 2. can you give a test-page+font.

You can download 'Un-series' fonts at
http://chem.skku.ac.kr/~wkpark/project/font/UnFonts/
For instance, after installing 'UnBatang' font, you can set 'monospace', 'serif'
and 'sans-serif' fonts for Korean to 'UnBatang' (it's not a monospace font but
for this experiment, it should do) in the font pref and try to view 

http://jshin.dyndns.org/i18n/utf8_kr.html 

The fact that syllables covered by Apple Korean cmap (2,350 of them) are
rendered with hollow boxes indicates that Mozilla recognizes that the font
covers them as well but fails to map them to the corresponding glyph IDs for
some reason. This may as well be a problem with old Mac APIs and may not be
solved without 'resorting' (well, it's not 'resorting'...) to ATSUI APIs.
There's a related bug, but I can't find it at the moment.

Comment 58

[Asaf Romano (gone)]

I'm worried about this code, and it maybe related to the problem you mentioned:
http://lxr.mozilla.org/seamonkey/source/widget/src/mac/nsMacNativeUnicodeConverter.cpp#114

Another time it checks for smScriptEnabled which (due to apple bug..) is false
for most of the scripts.

Comment 59

[Simon Montagu :smontagu]

(In reply to comment #58)
> I'm worried about this code, and it maybe related to the problem you mentioned:
>
http://lxr.mozilla.org/seamonkey/source/widget/src/mac/nsMacNativeUnicodeConverter.cpp#114
> 
> Another time it checks for smScriptEnabled which (due to apple bug..) is false
> for most of the scripts.

That only seems to be called from widget/src/mac/nsClipboard.cpp and
widget/src/mac/nsDragService.cpp, so I would expect it to cause a bug in copying
and pasting rather than display.

Comment 60

[Simon Montagu :smontagu]

Attachment: Remove scriptEnabled altogether

This should have the same effect as the last patch, but it removes the
scriptEnabled code altogether.

Comment 61

[Simon Montagu :smontagu]

I see there is some special-casing for Korean in nsUnicodeRenderingToolkit.cpp,
e.g.
http://lxr.mozilla.org/seamonkey/source/gfx/src/mac/nsUnicodeRenderingToolkit.cpp#1383
and
http://lxr.mozilla.org/seamonkey/source/gfx/src/mac/nsUnicodeRenderingToolkit.cpp#1704.
Maybe that conflicts with the patches here.

Comment 62

[Asaf Romano (gone)]

just an addtion: for Hebrew, there is no pronlem with characters that are not
included in Mac Hebrew (RLM/High Dash etc.)

Comment 63

[Asaf Romano (gone)]

(In reply to comment #61)
> I see there is some special-casing for Korean in nsUnicodeRenderingToolkit.cpp,
> e.g.
>
http://lxr.mozilla.org/seamonkey/source/gfx/src/mac/nsUnicodeRenderingToolkit.cpp#1383
> and
>
http://lxr.mozilla.org/seamonkey/source/gfx/src/mac/nsUnicodeRenderingToolkit.cpp#1704.
> Maybe that conflicts with the patches here.

I had put breakpoints in both place and it didn't get there, so I guess it's not
related... (neither on launching nor on rendering) :-/

Comment 64

[Jungshik Shin]

(In reply to comment #61)
> I see there is some special-casing for Korean in nsUnicodeRenderingToolkit.cpp,
> e.g.
>
http://lxr.mozilla.org/seamonkey/source/gfx/src/mac/nsUnicodeRenderingToolkit.cpp#1383

That fallback is to render Korean syllables not covered by Mac Korean (e.g.
U+AC02) in terms of Korean Jamos (letters : vowels and consonants). It was added
back in late 1990's when there's no Korean font available on Mac covering the
full Korean repertoire in the Unicode. 
However, what I'm observing is that those syllables (8,822 of them) are rendered
perfectly well while 2,350 syllables (that ARE covered by Mac Korean) are
rendered with hollow boxes. In the file, there are multiple fallbacks. For
syllables covered by Mac Korean, none of fallbacks is hit. That is, they're
supposed to be rendered by 'TEC-based' routine. For syllables not covered by Mac
Korean, ATSUI fallback is used and ATSUI has no problem rendering them (ATSUI
can make use of MS Unicode cmap - PID=3, EID=1). So, what's happening (I guess)
is the old-API based routine is invoked but it can't map Unicode (or Mac Korean
which is more or less equivalent to EUC-KR) code points to glyph IDs in the font
because there's no Apple Korean cmap table. Not being able to find Apple Korean
cmap in the font,  the old-textdrawing API just picks the default glyph
(hollow-box). 

A work-around is to add Apple Korean cmap to Un-fonts. However, we really need
to do something here. Most of fallbacks in nsUnicodeRenderingToolkit.cpp would
be unnecessary if we can just use ATSUI APIs in the first place. Then, there's a
speed issue....

Comment 65

[Simon Montagu :smontagu]

A hacky solution to the Korean problem is to add smKorean to the condition at
http://lxr.mozilla.org/mozilla/source/gfx/src/mac/nsUnicodeFontMappingMac.cpp#67

The real fix may be to make nsUnicodeFontMappingMac::FontEnumCallback pass
smUnicodeScript to the ctor of nsUnicodeFontMappingEntry when the font has a
Unicode cmap.

Comment 66

[Jungshik Shin]

(In reply to comment #65)
> A hacky solution to the Korean problem is to add smKorean to the condition at
> http://lxr.mozilla.org/mozilla/source/gfx/src/mac/nsUnicodeFontMappingMac.cpp#67

  I'm afraid the same 'hack' is necessary for Japanese and Chinese fonts without
Apple cmap tables but with MS Unicode and MS 'codepage-based' cmap tables.
Actually, that may be true of virtually all truetype fonts made for Windows.
Extending the hack to other scripts means that we just use ATSUI for them....

> The real fix may be to make nsUnicodeFontMappingMac::FontEnumCallback pass
> smUnicodeScript to the ctor of nsUnicodeFontMappingEntry when the font has a
> Unicode cmap.
 
  It seems to work, but we don't know yet how to tell whether a font has a
Unicode cmap or not....

Comment 67

[Jungshik Shin]

Maybe, we have to resolve this bug with attachment 151902 [details] [diff] [review] and open a new bug
about making use of truetype fonts without Apple cmap tables (but with MS
Unicode cmap table). In the meantime, I'll ask the author of Un-series fonts to
add Apple  Korean cmap. We also have to release-note that truetype fonts without
Apple cmap tables (for Chinese, Japanese, Korean, Russian, .....) don't work. 

Comment 68

[Asaf Romano (gone)]

(In reply to comment #66)

>   It seems to work, but we don't know yet how to tell whether a font has a
> Unicode cmap or not....
> 

Look in FillFontInfoFromCMAP in this file:
http://bugzilla.mozilla.org/attachment.cgi?id=55346&action=view

Comment 69

[Jungshik Shin]

(In reply to comment #68)
> (In reply to comment #66)
> 
> >   It seems to work, but we don't know yet how to tell whether a font has a
> > Unicode cmap or not....
> 
> Look in FillFontInfoFromCMAP in this file:
> http://bugzilla.mozilla.org/attachment.cgi?id=55346&action=view

http://lxr.mozilla.org/seamonkey/source/gfx/src/mac/nsMacUnicodeFontInfo.cpp#290
would be a better reference. I knew about it thanks to your comment #27 :-)

  Anyway, I was talking about a Mac OS API that lets us do it without poking
inside a truetype font. Anyway, if there isn't any (I haven't heard back from
Apple), we have to use it (just as we do in Gfx:Win). 

Comment 70

[Asaf Romano (gone)]

Attachment: patch

We checked that again, and maybe there is no need for the Qucickdraw converter 
(which is buggy for non-roman scripts), this patch removes it (in addition to
what has been done so far).

We need this to be tested (jshin...), especially from the performance aspect.

If there are no performance issues (or any other problems), i'll request r/sr.

Comment 71

[Asaf Romano (gone)]

Attachment: patch

Oops..I chose the wrong file

Comment 72

[Asaf Romano (gone)]

Attachment: patch

Oops..I chose the wrong file

Comment 73

[Simon Montagu :smontagu]

There is a test build with the last patch at
http://smontagu.org/mozilla/testbuilds/bug120401/mozilla-powerpc-apple-darwin7.4.0.dmg.gz

Comment 74

[Asaf Romano (gone)]

Attachment: Patch (same as before, but restoting the changes for Hebrew fonts)

Oops, I forgot to include the changes for the Hebrew fonts from previous
patches, you can continue test the build ;). If someone test the patch for
Hebrew, just chnage the following prefs in the Appearance->Fonts pane (for
Hebrew):
--Serif & Sans-Serif -> Lucida Grande
--Monospace -> Monaco

Comment 75

[Shoshannah Forbes]

> --Monospace -> Monaco

Monaco has no Hebrew glyphs. How can someone test with it?

Comment 76

[Asaf Romano (gone)]

(In reply to comment #75)
> > --Monospace -> Monaco
> 
> Monaco has no Hebrew glyphs. How can someone test with it?

I know, but since there is no monospace hebrew font in OS X, the OS return
glyphs from another font, and the problems form bug 171519 just disappear (and
that's not what happens when you choose Lucida Grande for example), anyway you
can always choose font that does include hebrew CMAP.

What I want to be tested is if there is any performance issue here, since we
removed the QuickDraw converter.

Comment 77

[Asaf Romano (gone)]

Attachment: Hebrew in textareas after applying last patch

Shosh, this is for you.

Notice that this is nothing more than a workaround (the changes for the default
hebrew fonts) until bug 246527 is sloved (and Monaco (default monospace font in
OS X) will include an hebrew CMAP).

Comment 78

[Shoshannah Forbes]

Tested with:
http://www.miloberi.com/pub/firefox-powerpc-apple-darwin7.4.0.dmg.gz

There is an improvment. Hoowever, some problems still remain:
* It still ignors the font setting at my site ( http://www.xslf.com ). My site
calls for Raanana (a Mac font) but FF ignores it and uses Lucida Grande.
* On many pages, the puncuation/English text overlaps the Hebrew text (I am
attaching a screenshot). It seems that FF is using Western fonts for
English/Punctuation (althugh I have told it to use a font which does contain
roman glyphs).
* At some sites, Hebrew text near forms elements overlaps the form elements

Comment 79

[Shoshannah Forbes]

Attachment: notice the jaggy alignment and text overlap

Comment 80

[Shoshannah Forbes]

Attachment: Text overlaps form

Comment 81

[Nir Soffer]

(In reply to comment #78)
> * It still ignors the font setting at my site ( http://www.xslf.com ). My site
> calls for Raanana (a Mac font) but FF ignores it and uses Lucida Grande.

Mozilla/FireFox does not know about Raanana or Ariel Hebrew because of bug 246521 
http://bugzilla.mozilla.org/show_bug.cgi?id=246527
You can vote for this bug or use only Windows TrueType fonts like Arial, Time New Roman and like.

It would be interesting to see how Camino handle this as it uses a standard font panel and can access 
all the fonts in the system.

Comment 82

[Jungshik Shin]

(In reply to comment #76)

> What I want to be tested is if there is any performance issue here, since we
> removed the QuickDraw converter.

Does it mean that we don't use QuickDraw string-drawing routine at all and
instead we exclusively use ATSUI drawing-routine? Your patch doesn't do that
although you removed QuickDraw converter. 

Anyway, somehow all the Korean fonts (both Windows truetype and Apple dfork
truetype) except for 'AppleGothic' 'disappeared' from Mac OS X font-??-tool and
I couldn't add them back (they're on the disk, but I couldn't make them
available to any of application including Safari). This happened after I applied
your patch, built a new binary and tested it once. I'll try to figure it out and
get back here with more info. 

Comment 83

[Simon Montagu :smontagu]

The overlapping text may be a similar issue to bug 143557

Comment 84

[Asaf Romano (gone)]

Shosh, can you tell us if you see the problems after changing default font size
to 15 (For Hebrew & Western)? I don't

Comment 85

[Simon Montagu :smontagu]

Attachment: Add change to default font size (as in bug 143557)

Comment 86

[Asaf Romano (gone)]

Comment on attachment 155201 [details] [diff] [review]
Add change to default font size (as in bug 143557)

Requsting r/sr for this. I will add some comments here later.

Comment 87

[Asaf Romano (gone)]

Thanks for superreview.

/me wants this for aviary1.0, requesting approval and blocking-aviary1.0mac.

Comment 88

[Asaf Romano (gone)]

Simon, can you please check it in? (but don't mark it as FIXED, since we want
this for AVIARY branch)

Comment 89

[Simon Montagu :smontagu]

Fix checked in.

Comment 90

[Asa Dotzler [:asa]]

Comment on attachment 155201 [details] [diff] [review]
Add change to default font size (as in bug 143557)

a=asa for aviary checkin.

Comment 91

[Asaf Romano (gone)]

(CCing Mike Pinkerton)

Mike: can we land this to Camino 0.8.1 branch?

Comment 92

[Simon Montagu :smontagu]

Backed out: Tp went through the roof on all Mac tinderboxen after the checkin.

Comment 93

[Jungshik Shin]

I took a look at the patch a while ago, but I forgot the details. IIRC, with the
patch, Mozilla uses ATSUI text-drawing APIs all the time, which will for sure
slow down Mozilla significantly. My understanding is that ATSUI text-drawing
APIs have a quite heavy overhead because they do a lot of things (layout
including BiDI, line, word breaking, etc). That overhead is not much a problem
if we hand them a relatively large chunk of text (e.g. a paragraph). However,
with the patch, we throw at them rather small chunks of text repetitively so
that the overhead is multipled many times and Mozilla gets significantly slower. 

I'm writing this from memory so that I'd stand corrected if I'm wrong. 

Comment 94

[rbs]

So, how is Safari dealing with this problem, then?

Comment 95

[Jungshik Shin]

Safari doesn't have to because it hands over a paragraph (or an even longer
chunk) at a time to ATSUI APIs so that the overhead is minimized. Mozilla does
most of 'chores' necessary for layout before it calls ATSUI APIs while Safari
delegates them to ATSUI APIs. Of course, this is just my speculation, but I'm
pretty sure that's the case. 

Comment 96

[rbs]

Looks like your speculation may be overstretched. After all, no matter what, one
has to split/color <a>links</a>, <b>bold</b>, etc. The layout code has to be
designed for that. It is therefore hard to see how one can pass a large
paragragh, although that could be done when needed at the price of more
complexity -- which I doubt Safari has. My own speculation is that there may be
better APIs that Safari is using, no?

Comment 97

[Asaf Romano (gone)]

I asume this is related to the quickdraw-converter disable, which has problem
with langs other than Roamn :-/

We can workaround in by using supporting only fonts with Unicode CMAP instead of
just disable the converter (see comment 65 - 67).

Comment 98

[rbs]

...BTW, drawing on the experience of what happened in GfxWin, we had to optimize
things such as:

myfunction1() { 
::SelectObject(font)
--do something
}

myfunction2() { 
::SelectObject(font)
--do something
}

These |selectObject| ensured correctness, but it turned out that there were
costing a *lot* on some flavors of Windows... We had to be take them out on a
case-by-case basis depending on whether or not the calling sequence really
changed the font.

What I am trying is that it took some careful scrutiny to make the most of the
APIs even in GfxWin. Some iterations were needed to get where we are.

It may be that GfxMac needs some careful eyes on things that could seem harmless
at first.

Comment 99

[Asaf Romano (gone)]

(In reply to comment #94)
> So, how is Safari dealing with this problem, then?

ATSUII is a carbon-only API, Safari and othee Cocoa apps don't use it.

For Cocoa:
http://developer.apple.com/documentation/Cocoa/Conceptual/FontHandling/index.html

Comment 100

[Asaf Romano (gone)]

Hmm... can we land it again with the quickdraw-converter restored? (Since it was
kind of off topic issue here... see commments 50 - 70), and open a separate bug
for not supporting fonts without a unicode CMAP (see commnet 65 for details).

Comment 101

[Asaf Romano (gone)]

Jshin: accoding to my last comment, it's up to you...

Comment 102

[Nir Soffer]

(In reply to comment #99)
> ATSUII is a carbon-only API, Safari and othee Cocoa apps don't use it.

According to this document: 
http://developer.apple.com/documentation/Carbon/Conceptual/ATSUI_Concepts/index.html
"ASTUI is at the heart of all text drawing in Mac OS X", and used by all parts of the system. Cocoa 
applications does have to use it because they use the text system or Webkit, which use lower level APIs.

Maybe we can learn from the open source parts of Webkit?
http://developer.apple.com/documentation/Cocoa/Conceptual/DisplayWebContent/

Comment 103

[Nir Soffer]

(In reply to comment #102)
Soory, wrong link - here is a link to WebCore source:
http://www.opensource.apple.com/darwinsource/tarballs/other/WebCore-125.tar.gz

Comment 104

[Jungshik Shin]

(In reply to comment #102)
> (In reply to comment #99)
> > ATSUII is a carbon-only API, Safari and othee Cocoa apps don't use it.
> 
> According to this document: 
>
http://developer.apple.com/documentation/Carbon/Conceptual/ATSUI_Concepts/index.html
> "ASTUI is at the heart of all text drawing in Mac OS X", and used by all parts
of the system. 

Yup, that's what I've been hearing from people at Apple. 

re : comment #101

Perhaps, we have to for now. (comment #67).


As for using ATSUI, see bug 121540 and bug 205476

Comment 105

[Asaf Romano (gone)]

Attachment: Same patch, but with the converter restored

Sorry for everybody who taught we are going to fix all mac-bugs in one patch,
this is not going  to happen, not this time...

Comment 106

[Asaf Romano (gone)]

Comment on attachment 155356 [details] [diff] [review]
Same patch, but with the converter restored

Moving r/sr/a

Comment 107

[Asaf Romano (gone)]

Jshin: after the last patch will be checked in, open a separate bug please (for
the problm with ms-korean-cmaps).

Comment 108

[Jungshik Shin]

(In reply to comment #107)


> Jshin: after the last patch will be checked in, open a separate bug please (for
> the problm with ms-korean-cmaps).

Filed bug 254585.

The problem is not about MS Korean cmap but about Mac:Gfx that can't make use of
MS Unicode Cmap (PID=3, EID=1). Mac:Gfx doesn't need to parse MS codepage-based
cmaps whether they're Korean, Japanese, Russian, whatever because MS Unicode
Cmap has all the necessary information. 

Comment 109

[Asaf Romano (gone)]

I'm not sure, at least for English, fonts like Arial works fine... see bug
254585 comment 1 , but i suggest we contiue the discussion there.

We also know this is becuase of the unicode converter, maybe the problem is
relevant for complex scripts only, we already disable the converter for Hebrew
and Arabic, maybe we need to do the same for other scripts as well.

btw, can you check it in please? :-) (/me doesn't have cvs commit privileges...)

Comment 110

[Asaf Romano (gone)]

in my last comment: unicode converter=quickdraw converter

Comment 111

[Jungshik Shin]

(In reply to comment #109)
> I'm not sure, at least for English, fonts like Arial works fine... see bug
> 254585 comment 1 , but i suggest we contiue the discussion there.

No, see bug 254585 comment #3 as to why Arial works on Mac. 

> We also know this is becuase of the unicode (==> quickdraw) converter, maybe
the problem is
> relevant for complex scripts only, 

 See my comment to bug 254585.  

> btw, can you check it in please? :-) (/me doesn't have cvs commit privileges...)

 All right. I'll do

Comment 112

[Asaf Romano (gone)]

(In reply to comment #111)

> > btw, can you check it in please? :-) (/me doesn't have cvs commit privileges...)
> 
>  All right. I'll do

Thank you! (but don't mark it as fixed unless this is alspo checked in for
aviary as well)

Comment 113

[Asaf Romano (gone)]

Comment on attachment 155201 [details] [diff] [review]
Add change to default font size (as in bug 143557)

obselting the previous patch.

Comment 114

[Jungshik Shin]

fix rechecked into the trunk.
attachment 155356 [details] [diff] [review] doesn't get cleanly applied to aviary-1.0. Can you iron that
out and upload a new patch for aviary-1.0 branch? While you're at it, why don't
you see if your new patch can be applied to 1.7branch as well? 

Comment 115

[Asaf Romano (gone)]

Thanks for checkin.

The problem is with init/all.js, right?

...will be done tonight, stay tuned :-)

Comment 116

[Jungshik Shin]

(In reply to comment #115)

> The problem is with init/all.js, right?

No. I got a couple of rejects in gfx/src/mac. If it had been all.js, I'd have
just fixed it myself. I could have fixed gfx/src/mac as well, but I didn't bother. 

Comment 117

[Asaf Romano (gone)]

Attachment: same, against AVIARY branch

Comment 118

[Asaf Romano (gone)]

Comment on attachment 155391 [details] [diff] [review]
same, against AVIARY branch

moving approval from asa


Jshin: when checking-in, can you also check if it applies against 1.7 (if you
have it on you machine...) so we can ask for approval1.7.3?

Comment 119

[Asaf Romano (gone)]

Comment on attachment 155391 [details] [diff] [review]
same, against AVIARY branch

oops, i put ? instead of +

Comment 120

[Jungshik Shin]

(In reply to comment #118)
> (From update of attachment 155391 [details] [diff] [review])
> moving approval from asa

checked into aviary-1.0 branch with the following typo fixed:

-pref("font.name.cursive.zh-HK", "XXX.cursive");
+pref("font.name.cursive.zh-HK", "XXX.cursive");s

> Jshin: when checking-in, can you also check if it applies against 1.7 (if you
> have it on you machine...) so we can ask for approval1.7.3?

 You don't need to keep a separate tree just to see if it can be applied to a
branch. 'man cvs' will help you (see '-r' option for update/diff/commit). 
Anyway, except for all.js (which seems to already have your fix), the patch for
aviary-1.0 can be applied to 1.7 branch.

Comment 121

[Asaf Romano (gone)]

init/all.js doesn't have my fix:
http://lxr.mozilla.org/mozilla1.7/source/modules/libpref/src/init/all.js

Comment 122

[Jungshik Shin]

(In reply to comment #96)
> Looks like your speculation may be overstretched. After all, no matter what, one
> has to split/color <a>links</a>, <b>bold</b>, etc. The layout code has to be
> designed for that. It is therefore hard to see how one can pass a large
> paragragh, although that could be done when needed at the price of more

ATSUI APIs accept a 'layout object' with all the necessary layout attributes for
a paragraph  in parallel with the text to render. See bug 121540 comment #91,
bug 121540 comment #113 and bug 157967. We may have to overhaul Mozilla's layout
system to make better use of ATSUI, Pango, Uniscribe, and STSF. 

Anyway, I believe I sorta figured out why it slowed down mozilla so much with
the previous check-in. Our use of ATSUI is extremly inefficient (a lot more
inefficient than necessary) because we're calling it with one character at a
time. (it was added as a fallback so that perf. was not a concern).  See
http://lxr.mozilla.org/seamonkey/source/gfx/src/mac/nsUnicodeRenderingToolkit.cpp#1313

1313     // Cannot precess by TEC, process one char a time by fallback mechanism

Sometimes we call a string drawing API with one character at a time even without
the patch, but doing it always for sure would have a huge perf. loss. 

Comment 123

[Asaf Romano (gone)]

(In reply to comment #28)

> > Anybody knows a Crabon API to check if a font is a unicode one?
> 
> If nobody has figured it out, I'll ask John Jenkins at Apple. By 'Unicode font',
> you meant a truetype/opentype font with Unicode cmap, right? 
> 

Well, can you ask him about checking if a font has an apple cmap (without
"getting into the font").

Comment 124

[Asaf Romano (gone)]

CCing Asa,

Asa, as this is already checked in for aviary, can we also land it for 1.7.3?

Comment 125

[Mike Kaply [:mkaply]]

Comment on attachment 155391 [details] [diff] [review]
same, against AVIARY branch

a=mkaply for 1.7.3

Comment 126

[Asaf Romano (gone)]

Jshin, can you please check it in for 1.7.3? (we the needed updates for all.js)

Comment 127

[Asaf Romano (gone)]

(In reply to comment #126)
> Jshin, can you please check it in for 1.7.3? (we the needed updates for all.js)

Oh, we=with, sorry

Comment 128

[Asaf Romano (gone)]

I just marked bug 95978 as RESOLVED FIXED.

Comment 129

[Asaf Romano (gone)]

FIXED.

Thanks to everybody who helped fixing this bug.