GetSystemFontInfo() of GTK2 just returns pango_font_description_get_family(desc) as family name. Usually the family name does not fit to X Window fonts. Actually "sans" is returned on my desktop running Russian locale. I want Russian fonts for rendering on UI, but LoadFont() returns korean fonts because korean font was found before Russian and it convers the code points of Russian. Also the same problem of bug 149692 would happen. We can take the same approach of bug 149692 here. We need to give more information founday, family, charset encoding and registry when we use X fonts.

Some discussion in bug 181053.

This problem is serious on Solaris Russian locale. With localized GUI message, Mozilla now uses 2byte-font of Korean.

same approach of bug 149692 - get XLFD names by pango_x_font_subfont_xlfd() - return FFRE I don't think XFT needs the changes. So I use ifndef MOZ_ENABLE_XFT.

Are you guys actually using Gtk2 without Xft?

Yes, we are not using XFT.

Comment on attachment 116181 [details] [diff] [review] patch >-#ifdef MOZ_WIDGET_GTK >- > static void > AppendFontFFREName(nsString& aString, const char* aXLFDName) > { gtk2 w/ xft doesn't need this function - in fact, xft ignores FFRE fonts that are passed to it in the FFRE format. You might just want to use MOZ_ENABLE_COREFONTS here instead of using the widget + xft settings to detect if core fonts are compiled in. > #ifdef MOZ_WIDGET_GTK2 >+#ifndef MOZ_ENABLE_XFT Once again, use MOZ_ENABLE_COREFONTS instead. > aFont->name.Truncate(); >+#ifdef MOZ_ENABLE_XFT > aFont->name.AppendWithConversion(pango_font_description_get_family(desc)); >+#else >+ xlfd_from_pango_font_description(aWidget, desc, aFont->name); >+#endif Technically you should figure out if you're using Xft or not to determine which function to use and use the COREFONTS define as above to determine when to compile it in. This is included as part of the gfx module (see NS_IsXftEnabled in nsFontMetricsUtils.cpp - it should be safe to use.) > aFont->weight = pango_font_description_get_weight(desc); > > gint size; >@@ -1052,3 +1063,128 @@ > return 0; > } > #endif /* MOZ_ENABLE_XFT */ >+ >+#ifdef MOZ_WIDGET_GTK2 >+#ifndef MOZ_ENABLE_XFT COREFONTS, as above.

Thank you for review, I have attached the 2nd patch, please review. I have changed to the following. When Xft is enabled, even if COREXFONT is compiled in, xlfd_from_pango_font_description() will be called. +#ifdef MOZ_ENABLE_XFT + if (NS_IsXftEnabled()) { + aFont->name.AppendWithConversion(pango_font_description_get_family(desc)); + } +#endif /* MOZ_ENABLE_XFT */ + +#ifdef MOZ_ENABLE_COREXFONTS + // if name already set by Xft, do nothing + if (!aFont->name.Length()) { + xlfd_from_pango_font_description(aWidget, desc, aFont->name); + } +#endif /* MOZ_ENABLE_COREXFONTS */

Comment on attachment 116401 [details] [diff] [review] 2nd patch r=blizzard

Comment on attachment 116401 [details] [diff] [review] 2nd patch sr=rbs, my own style would have been |if (!...)| here: + if (aFontDesc == NULL) + if (fontmap == NULL) { + if (font == NULL) { ...

Thank you very much for quick review and super-review. Sure. I'll correct == NULL and will check-in.

I'm very sorry I had a mistake. We don't need the last line aFont->name.AppendWithConversion(pango_font_description_get_family(desc)); We already call it in if (NS_IsXftEnabled()) {}. I have removed the line and corrected if ( == NULL). aFont->name.Truncate(); #ifdef MOZ_ENABLE_XFT if (NS_IsXftEnabled()) { aFont->name.AppendWithConversion(pango_font_description_get_family(desc)); } #endif /* MOZ_ENABLE_XFT */ #ifdef MOZ_ENABLE_COREXFONTS // if name already set by Xft, do nothing if (!aFont->name.Length()) { xlfd_from_pango_font_description(aWidget, desc, aFont->name); } #endif /* MOZ_ENABLE_COREXFONTS */ aFont->name.AppendWithConversion(pango_font_description_get_family(desc));

Comment on attachment 116525 [details] [diff] [review] 3rd patch Sorry for dup work. I have removed the line below because it's duplicate. - aFont->name.AppendWithConversion(pango_font_description_get_family(desc)); Could you r= and sr= again?

Comment on attachment 116525 [details] [diff] [review] 3rd patch r=blizzard

patch checked in to Trunk. Thank you very much for review and super-review.