1163487 - determine whether LANGUAGE value should affect font selection within fontconfig font lookup code

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[John Daggett (:jtd)]

The existing gfxFontconfigUtils code has some relatively tricky logic to try and map Mozilla internal language groups to specific languages for use with fontconfig lookups:

http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxFontconfigUtils.cpp#439

The code within the 'if (gLangService) {' block is trying to map from language groups like 'x-western' to specific languages like 'fr'. In bug 1056479, comment 78, Karl thinks the code in gfxFcPlatformFontList should mimic this:

> The simplification has removed the key code and kept just the fallback
> code. AFAIK it is still better to use the user's language so please
> keep that code.
> 
> If the static you refer to is gLangService, then that is OK as long as
> the static Shutdown() method is called.
> 
> If using the existing method is not practical then this may be best as
> a follow-up.

I think this code isn't quite right for a number of reasons. The Mozilla-specific language groups are only used for chrome content and not general web content. Rather than trying to guess the language for chrome content based on a general lang group like 'x-western', we should be explicitly setting the language of the content. That will guarantee that any font-specific lookups work correctly without relying on guessing via environment variables like this.

I also think this sort of logic doesn't really belong in gfx, it belongs in the code that's mapping UI font names to actual fonts with the look and feel code.

This bug is a placeholder in the event that we determine the LANGUAGE variable logic is in fact required for some reason.

Comment 1

[John Daggett (:jtd)]

Attachment: patch, sniff the LANGUAGE when determining fclang

This integrates in the logic from gfxFontconfigUtils::GetSampleLangForGroup that sniffs the LANGUAGE env variable when determining how to map mozilla lang groups to a lang string.

Comment 2

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e74ebb8a008d

Comment 3

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e74ebb8a008d

Comment 4

[John Daggett (:jtd)]

Comment on attachment 8687863 [details] [diff] [review]
patch, sniff the LANGUAGE when determining fclang

Approval Request Comment
[Feature/regressing bug #]: This is one of the bugs that need to be fixed to enable the new fontconfig back end for beta/release builds. Enabling the new fontconfig back end will allow us to release unicode-range support across all platforms (bugs 1180560, 1119062).
[User impact if declined]: unicode-range support can't be enabled until FF45
[Describe test coverage new/current, TreeHerder]: landed on trunk last week, no issues reported
[Risks and why]: low risk
[String/UUID change made/needed]: none

Comment 5

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8687863 [details] [diff] [review]
patch, sniff the LANGUAGE when determining fclang

This has been on Nightly for a few days, seems safe to uplift to Aurora44.

Comment 6

[John Daggett (:jtd)]

Pushed to aurora:
https://hg.mozilla.org/releases/mozilla-aurora/rev/30cb37010709

Comment 7

[Carsten Book [:Tomcat]]

(In reply to John Daggett (:jtd) from comment #6)
> Pushed to aurora:
> https://hg.mozilla.org/releases/mozilla-aurora/rev/30cb37010709

setting flags