add pref for limit on generic substitutions in fontconfig platform fontlist

RESOLVED FIXED in Firefox 44

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jtd, Assigned: jtd)

Tracking

Trunk
mozilla45
Unspecified
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox44 fixed, firefox45 fixed, b2g-v2.5 fixed)

Details

Attachments

(2 attachments)

No description provided.
The patches on bug 1173260 enable multiple fonts to be mapped to generics via fontconfig under Linux. To limit the time spent checking fonts during font matching, I put in an arbitrary limit of three but this should probably be in a pref so that users and/or distros that are doing something more complicated and account for longer substitution lists.
I expect the only time that there would be many families before the first with support for the language (bug 1056479 comment 42) would be when there are no families that support the language.
Convert a constant limit into a pref value.
Attachment #8688274 - Flags: review?(m_kato)
Tweak the logic for looking up generic fonts under fontconfig. Add fonts into the array until the pattern has the lang, up until the pref-defined limit (default = 3). If no pattern has the lang, only use the first font (e.g. for obscure languages that don't match).
Attachment #8688278 - Flags: review?(karlt)
Comment on attachment 8688274 [details] [diff] [review]
patch p1, add pref for max generic substitutions

Review of attachment 8688274 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/thebes/gfxPlatformGtk.cpp
@@ +375,5 @@
> +    } else {
> +        gfxPlatform::FontsPrefsChanged(aPref);
> +        return;
> +    }
> +

I think that the following is better.

    if (strcmp(GFX_PREF_MAX_GENERIC_SUBSTITUTIONS, aPref)) {
        gfxPlatform::FontsPrefsChanged(aPref);
        return;
    }

    mMaxGenericSubstitutions = UNINITIALIZED_VALUE;
Attachment #8688274 - Flags: review?(m_kato) → review+
Blocks: 1180560
https://hg.mozilla.org/mozilla-central/rev/17ad6b5b7eb8
https://hg.mozilla.org/mozilla-central/rev/b058350c9753
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Comment on attachment 8688274 [details] [diff] [review]
patch p1, add pref for max generic substitutions

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, only affects users with special configurations
[String/UUID change made/needed]: none
Attachment #8688274 - Flags: approval-mozilla-aurora?
Comment on attachment 8688278 [details] [diff] [review]
patch p2, modify logic of generic font lookups

Approval Request Comment

-- same as above --
Attachment #8688278 - Flags: approval-mozilla-aurora?
Comment on attachment 8688274 [details] [diff] [review]
patch p1, add pref for max generic substitutions

Having a pref to limit number of matches we find is a good idea, Aurora44+
Attachment #8688274 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8688278 [details] [diff] [review]
patch p2, modify logic of generic font lookups

Aurora44+
Attachment #8688278 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.