Closed Bug 986153 Opened 6 years ago Closed 6 years ago

Consolidate browser fonts

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 31

People

(Reporter: vtsatskin, Assigned: vtsatskin)

References

Details

Attachments

(1 file)

For the network error refresh, we've chosen to use Fira Sans and Clear Sans for the fonts. We will want these included with Firefox since these are not fonts usually found on the system.

Fira: https://www.mozilla.org/en-US/styleguide/products/firefox-os/typeface/
Clear Sans: https://01.org/clear-sans

Related: bug 958879
Status: NEW → ASSIGNED
Component: Untriaged → General
OS: Mac OS X → All
Hardware: x86 → All
Version: unspecified → Trunk
As mentioned on IRC, we already include them for about:accounts:

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/aboutaccounts/fonts/

So we should just move them if they need to be used by other pages.
After investigating this some more and hearing feedback, I think a good approach will be to consolidate all of the fonts to one place to prevent duplication. I'm thinking they should live in browser/base/content/fonts.
Summary: Include Fira and Clear Sans as fonts → Consolidate browser fonts
Assignee: nobody → vtsatskin
Attached patch 986153.patchSplinter Review
Attachment #8407146 - Flags: review?(zack.carter)
Attachment #8407146 - Flags: review?(bmcbride)
Attachment #8407146 - Flags: review?(zack.carter) → review+
Attachment #8407146 - Flags: review?(bmcbride) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/de4559acdd58
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 31
Why are we shipping both Fira and Clear Sans fonts in the same product? Surely we can choose one family or the other, and stick with it. They're not so visually different that we can usefully contrast them, like with serif vs sans-serif.
And are you sure the versions of the fonts checked in work well across locales? The versions I see look like they've been subsetted and that's going to cause problems in locales using Latin script with lots of diacritics or non-Latin locales.
(In reply to John Daggett (:jtd) from comment #9)
> And are you sure the versions of the fonts checked in work well across
> locales? The versions I see look like they've been subsetted and that's
> going to cause problems in locales using Latin script with lots of
> diacritics or non-Latin locales.

Ugh - yes, that's not good at all. Locales like Polish or Romanian (or plenty of others) will be liable to get a "ransom-note" effect when glyphs such as ł or ś or ț fall back to some other font.
Valentin, it was my understanding that the new network error pages should now use Fira Sans and Clear Sans as typefaces. Using Nightly 32.0a1 (2015-05-22) [1], I noticed that the "server not found" error page for example, uses the system font instead and simply inspecting the error page did not help me confirm the usage of Fira Sans and/or Clear Sans, but perhaps that's not a good approach in terms of manual testing here (?).

I was able to confirm though, that the two typefaces were successfully added to .../browser/base/content/fonts/ according to Comment 3, by cloning mozilla-central on my local machine. My main concern here is the way this fix affects other locales, as John mentioned in Comment 9. Please let me know what are your thoughts on this, so that this fix can be properly verified and covered by regression testing.


[1] Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:32.0) Gecko/20100101 Firefox/32.0
Flags: needinfo?(vtsatskin)
(In reply to Jonathan Kew (:jfkthame) from comment #10)
> (In reply to John Daggett (:jtd) from comment #9)
> > And are you sure the versions of the fonts checked in work well across
> > locales? The versions I see look like they've been subsetted and that's
> > going to cause problems in locales using Latin script with lots of
> > diacritics or non-Latin locales.
> 
> Ugh - yes, that's not good at all. Locales like Polish or Romanian (or
> plenty of others) will be liable to get a "ransom-note" effect when glyphs
> such as ł or ś or ț fall back to some other font.

I totally agree that this is a problem we need to fix. Thank you for being excellent typography watchdogs :)

These problems were actually identified in bug 992650 after you both voiced your concerns about the issue. Are these concerns still issues after the work in the aforementioned bug landed?


(In reply to Andrei Vaida, QA [:avaida] from comment #11)
> Valentin, it was my understanding that the new network error pages should
> now use Fira Sans and Clear Sans as typefaces. Using Nightly 32.0a1
> (2015-05-22) [1], I noticed that the "server not found" error page for
> example, uses the system font instead and simply inspecting the error page
> did not help me confirm the usage of Fira Sans and/or Clear Sans, but
> perhaps that's not a good approach in terms of manual testing here (?).
> 

Good catch on identifying the inconsistency here. The design was not followed entirely to spec because of technical and typographical limitations found during development. You can follow the multifaceted discussion about the changes starting with bug 982347 comment 23.

> I was able to confirm though, that the two typefaces were successfully added
> to .../browser/base/content/fonts/ according to Comment 3, by cloning
> mozilla-central on my local machine. My main concern here is the way this
> fix affects other locales, as John mentioned in Comment 9. Please let me
> know what are your thoughts on this, so that this fix can be properly
> verified and covered by regression testing.
> 
> 
> [1] Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:32.0) Gecko/20100101
> Firefox/32.0

Please see reply at the top of the message. I'm going to defer this to the experts.
Flags: needinfo?(vtsatskin)
Flags: needinfo?(jfkthame)
Flags: needinfo?(jdaggett)
For reference, one way to identify font usage is via NSPR logging.

  export NSPR_LOG_MODULES=textrun:5,textrunui:5,fontinit:5
  export NSPR_LOG_FILE=textrun.out

That will tell you which fonts are initialized and when system fallback occurs (and for which character).  Fonts with a name like "ufXXXXX..." are downloadable fonts.

Lots of data in those logs, definitely disable them after you run a test...
Flags: needinfo?(jdaggett)
(In reply to Valentin Tsatskin [:vt] from comment #12)
> (In reply to Jonathan Kew (:jfkthame) from comment #10)
> > (In reply to John Daggett (:jtd) from comment #9)
> > > And are you sure the versions of the fonts checked in work well across
> > > locales? The versions I see look like they've been subsetted and that's
> > > going to cause problems in locales using Latin script with lots of
> > > diacritics or non-Latin locales.
> > 
> > Ugh - yes, that's not good at all. Locales like Polish or Romanian (or
> > plenty of others) will be liable to get a "ransom-note" effect when glyphs
> > such as ł or ś or ț fall back to some other font.
> 
> I totally agree that this is a problem we need to fix. Thank you for being
> excellent typography watchdogs :)
> 
> These problems were actually identified in bug 992650 after you both voiced
> your concerns about the issue. Are these concerns still issues after the
> work in the aforementioned bug landed?

For the network error page, after extensive discussion in bug 982347, we ended up using system fonts rather than those bundled with the browser, so that addresses these concerns; they should work well when localized into any language for which the system fonts have adequate character coverage - which is much more comprehensive than what we have in Fira at present.

(Bug 992650 was similar, but for the about:accounts page, and updated that page as well to use system fonts instead of bundled.)

> 
> 
> (In reply to Andrei Vaida, QA [:avaida] from comment #11)
> > Valentin, it was my understanding that the new network error pages should
> > now use Fira Sans and Clear Sans as typefaces. Using Nightly 32.0a1
> > (2015-05-22) [1], I noticed that the "server not found" error page for
> > example, uses the system font instead and simply inspecting the error page
> > did not help me confirm the usage of Fira Sans and/or Clear Sans, but
> > perhaps that's not a good approach in terms of manual testing here (?).

The Inspector (or the Font Information add-on, for a light-weight option) works OK on these pages, and will confirm what font is being used; but as per the discussion in bug 982347, we ended up *not* adopting Fira/Clear Sans here. (If there's a design document somewhere that describes these pages, perhaps it should have a note added to reflect this.)
Flags: needinfo?(jfkthame)
(In reply to Jonathan Kew (:jfkthame) from comment #14)
> (If there's a design document somewhere that describes these pages,
> perhaps it should have a note added to reflect this.)

I will update the design to reflect this change. Thanks!
Valentin, John, Jonathan - thank you all for clarifying this. I spot-checked a few locales using the latest l10n nightly builds and tracked my work in an etherpad [1]. 

At this point, I don't think we have any major issues, only some potential ones that might just be related to a specific typeface character coverage. Please take a look and let me know what you think. Also, if there's anything else I should look at here, don't hesitate to ask.


[1] https://etherpad.mozilla.org/NetErrorPages-Fonts
(In reply to Andrei Vaida, QA [:avaida] from comment #16)
> Valentin, John, Jonathan - thank you all for clarifying this. I spot-checked
> a few locales using the latest l10n nightly builds and tracked my work in an
> etherpad [1]. 
> 
> At this point, I don't think we have any major issues, only some potential
> ones that might just be related to a specific typeface character coverage.
> Please take a look and let me know what you think. Also, if there's anything
> else I should look at here, don't hesitate to ask.
> 
> 
> [1] https://etherpad.mozilla.org/NetErrorPages-Fonts

Really good work being thorough.

Please go ahead and open bugs for each of the issues found (if you haven't already) and CC me on them. Some of these might be existing issues from before the redesign of the network error pages had that we may have not known about until now.

Unfortunately I'm not entirely sure how to report localization issues, but my guess is to make a bug here: https://bugzilla.mozilla.org/enter_bug.cgi?product=Mozilla%20Localizations

For issue 1, please assign the bug to me and add it as a blocker to bug 982347.
Thank you! I filed new bugs and CCed/assigned you as requested.
Status: RESOLVED → VERIFIED
(In reply to Andrei Vaida, QA [:avaida] from comment #18)
> Thank you! I filed new bugs and CCed/assigned you as requested.

No problem. Were the lack of localization for issues 3 and 4 filed anywhere? Just want to make sure those don't slip through the cracks.
Flags: needinfo?(andrei.vaida)
(In reply to Valentin Tsatskin [:vt] from comment #19)
> (In reply to Andrei Vaida, QA [:avaida] from comment #18)
> > Thank you! I filed new bugs and CCed/assigned you as requested.
> 
> No problem. Were the lack of localization for issues 3 and 4 filed anywhere?
> Just want to make sure those don't slip through the cracks.

No worries, I filed new issues for those too and CCed you, I just needed a bit more time to investigate them. Please let me know if there's anything else I can help with here.
Flags: needinfo?(andrei.vaida)
You need to log in before you can comment on or make changes to this bug.