Now, we don't implement the font name resolver on Mac. # The function is existing, but it's not working. This bug may be fixed by the patch of bug 361601

(In reply to comment #0) > This bug may be fixed by the patch of bug 361601 We need another patch for this bug. I'm working on it now.

This patch fixes the bug 357555 and bug 361601. So this is very important patch for Mac. Please review this first, Stuart. However, if we use this patch, we have some remain bugs. We need more work after this...

Oops, sorry. I attached different page's screenshot.

Comment on attachment 248527 [details] [diff] [review] Patch rv1.0 I found some another feature, please wait for next patch.

This patch is a draft of next patch. I need more works for some issues.

I need more works for current font finding code.

I have some trouble the current way that is only using NSFontManager and NSFont of cocoa. I think that we should keep to use ATSUI font for Font Enum...

Sorry for the delay. This patch uses ATSUI instead of cocoa font manager. Because the list of families by cocoa is not compatible the font list of Fx2. E.g., Fx2 has one or more families per a font family on cocoa. And cocoa cannot list up the *all* alias names of all fonts. But ATSUI can do it. This patch cannot render the italic and bold font in some fonts. But I think that it is another bug. We should fix this bug first.

(In reply to comment #10) > Created an attachment (id=249251) [edit] > Patch rv3.0 > Minefield with patch applied: now all text is displayed using Helvetica. I does not matter what font-family I specify in the style sheet, it is not recognised. The one exception that is recognised is 'Georgia'. For Japanese text, Hiragino is recognised (when using the Japanese name) Text in the chrome part (Bookmark bar, tabs, location bar is also using Helvetica).

> For Japanese text, Hiragino is recognised (when using the Japanese name) > > Text in the chrome part (Bookmark bar, tabs, location bar is also using > Helvetica). The font switching and getting the system fonts issues are out of scope. If you find some problems, please file the new bugs :-)

Comment on attachment 249251 [details] [diff] [review] Patch rv3.0 I found a weight choosing issue. I need more work.

(I'm the right person to review this, btw) What breaks if we use the Cocoa font listing instead of going through ATSU? We'll end up with a different font list than most other apps on the system this way, no? That might not be so bad, but if it's just an issue of breaking older prefs, we might want to figure out a different way of fixing that.

Vlad: (In reply to comment #14) > (I'm the right person to review this, btw Welcome back :-) > What breaks if we use the Cocoa font listing instead of going through ATSU? > We'll end up with a different font list than most other apps on the system this > way, no? That might not be so bad, but if it's just an issue of breaking older > prefs, we might want to figure out a different way of fixing that. If we are using font list of cocoa for the GetFontList, we have following issues: 1. Maybe, we should need to change the pref names for fonts. Because we cannot inherit from old prefs. 2. If a font has localized names, that is localized only at running on the same locale system. I.g., on Japanese locale, the Japanese font names are localized by [NSFont displayName], but Chinese/Korean fonts are not localized. We are using all.js in all locales, so CJK fonts are localized in all.js, but the pref dialog cannot resolve the localized name <-> ascii name. How do you think?

Um.. The second problem is also being on Window. We may fix the bug on pref dialog and all.js.

And there is a third problem. 3. Cocoa doesn't list-up the "Osaka－等幅" that is a monospace font for Japanese. But the font is a member of "Osaka". So, a family may have both variable pitch font and monospace font. But this issue is fixed on "Patch rv2.5". This issue can fix on cocoa too. But the list of fonts will be different from other cocoa applications. I think that we should ignore this issue. Because CSS cannot specify the font pitch.

This patch is using cocoa for listing up the font families for GetFontList. But this is still using ATSUI for getting all font names(including locailzed/alias names). I think that this issue cannot be fixed by cocoa. And this patch changes the calculating the avg character width of the font. Because the current way returns too wide value for CJK fonts. Because they have 2 or more wider characters than ASCII characters. Therefore, on my environment, many input elements are too wide.(e.g., bugzilla-jp and google). I uses the 'x' for the calculating it, because old gfx uses 'x'. I think that this is better than 'a' that is using in current code if the system doesn't return the ave char width. By this patch, the prefs dialog and prefs are not compatible before fx2. We need more work for prefs. But this issue should be separated to another bug.

Comment on attachment 249371 [details] [diff] [review] Patch rv4.0 >Index: gfx/thebes/src/gfxQuartzFontCache.h >=================================================================== >+ void GenerateFontListKey(const nsAString& aKeyName, nsAString& aResult); >+ void GenrateWithTraitsKey(FontEntry* aFont, NSString* aTraits, >+ nsAString& aResult); Misspelled, also not implemented/used; delete this declaration > #endif /* GFXQUARTZFONTCACHE_H_ */ >Index: gfx/thebes/src/gfxQuartzFontCache.mm >=================================================================== >+PRBool >+gfxQuartzFontCache::AppendFontFamily(NSFontManager *aFontManager, >+ NSString *aName, >+ PRBool aNameIsPostscriptName) >+{ >+ NSString *name; >+ if (!aNameIsPostscriptName) { >+ // we should use localzed simple family name. >+ name = [aFontManager localizedNameForFamily:aName face:nil]; >+ } else { >+ // we should use display name that is localized the family name and >+ // extra name. (e.g., "mono") >+ NSFont *font = [NSFont fontWithName:aName size:10.0]; >+ name = [font displayName]; >+ } Kind of a nit -- change this around so it's an if (aNameIsPostscriptName) block instead of having the negation. The rest of this looks great, though I didn't test it -- we really need some CSS font selection tests, we've been mostly working blind with this stuff on all platforms, just fixing bugs as they come up. :(

Thank you, Vald, for your review!

checked-in, thank you.

I filed bug 364678 for the problems I mentioned in comment 11. (and sorry for not coming back to this earlier - I had some urgent 'client matters' to take care of).

This changed the default font for the browser chrome and also makes a bunch of the text unaligned (the bookmark bar is a good example). Unfortunately this will probably need to be backed out if it cannot be fixed soon.

Masayuki - thanks for taking on this issue, despite the problems with the current patch!

Google Reader, and Gmail are broken with this patch in place. Can someone confirm it was this patch and not the JEP patch that broke things. On the upside, I like the font changes! Much smoother.

None of the above: those would be the since-backed-out bug 310405.

Starting up Firefox gives me about 20 of these assertions: ###!!! ASSERTION: A font id has two or more postscript name!: 'names.mPostscriptName.IsEmpty()', file mozilla/gfx/thebes/src/gfxQuartzFontCache.mm, line 321 Afaict fontName is empty when that happens. It's very annoying when running with break-on-assert.

Since this patch landed, view-source and Bugzilla comments no longer display in a monospace font for me. I also still get the same (bad) font fallback for Japanese artist names on Last.fm.

Ts regression: Firefox trunk xserve06 1460ms-1540ms (5%) http://build-graphs.mozilla.org/graph/query.cgi?testname=startup&units=ms&tbox=xserve06.build.mozilla.org_Fx-Trunk&autoscale=1&days=7&avg=1&showpoint=2006:12:21:20:10:18,1468 Firefox trunk bm-xserve08 860ms-950ms (10%) http://build-graphs.mozilla.org/graph/query.cgi?testname=startup&units=ms&tbox=bm-xserve08.build.mozilla.org_Fx-Trunk&autoscale=1&days=7&avg=1&showpoint=2006:12:21:20:32:17,936 Camino trunk pawn 3000ms-8000ms (167%; tests purposely being run on old hardware; many test runs are timing out) http://build-graphs.mozilla.org/graph/query.cgi?testname=startup&tbox=pawn.office.mozilla.org&autoscale=1&days=7&avg=1&showpoint=2006:12:21:20:56:44,7148 Camino trunk maya 880ms-1070ms (20%) no graphs Camino trunk bm-xserve10 490ms-570ms (15%) no graphs

bug 364678 is landed. I think that the many problems on non-CJK locale system, now works fine.

For ts regression: This is a design. But we can win in the tp value. This patch creates system font name tables at starting up. I think that this is a necessary process.

Typeface selection for Cyrillic text is still quite poor. E.g. attachment 206776 [details] on bug 121540. Should this patch have helped that? If appropriate I'll open a new bug about it.