Closed Bug 992650 Opened 11 years ago Closed 11 years ago

Fira Sans font used on about:accounts is incomplete (issues with diacritics, latin-extended glyphs)

Categories

(Firefox :: Sync, defect)

30 Branch
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 32
Tracking Status
firefox30 + verified
firefox31 + verified
firefox32 --- verified

People

(Reporter: mstanke, Assigned: jfkthame)

References

Details

(Whiteboard: p=0 s=it-32c-31a-30b.2 [qa!])

Attachments

(4 files)

Attached image screenshot (deleted) —
I tried setting new Sync account on Fx30, and I found out, that the used font Fira Sand used on about:accounts does not render well letters with diacritics - see the enclosed screenshot.
Component: General → Sync
Michal, does this same issue occur on https://accounts.firefox.com/signup as well? Sean, I'm not sure who would be a Fira Sans expert - wondering if this is a known issue with the font.
Flags: needinfo?(smartell)
Hm, apparently this font is on Github! Michal, can you submit an issue at https://github.com/mozilla/Fira/issues ? There appear to be other similar issues filed already.
Flags: needinfo?(smartell)
Filled on GitHub too - https://github.com/mozilla/Fira/issues/26 . Hope this will be fixed before Australis and the new Sync launch.
OS: Windows 7 → All
Hardware: x86_64 → All
Flags: firefox-backlog+
Whiteboard: p=0
In the short-term, let's move away from Fira Sans and use a "normal" font. ckarlof says we can switch https://accounts.firefox.com as well.
:gavin the fonts available from the FirefoxOS Type Guide contains the complete font sets in a row of links at the bottom.
This switches about:accounts to simply use the system "message-box" font, which seems the most appropriate choice for content like this, and should have much better i18n support than the embedded, subsetted fonts that were being used.
Attachment #8419344 - Flags: review?(gavin.sharp)
Blocks: 1007639
Summary: Fira Sans diacritic issue in about:accounts → Fira Sans font used on about:accounts is incomplete (issues with diacritics, latin-extended glyphs)
Attachment #8419343 - Flags: review?(gavin.sharp) → review+
(In reply to Shane Tomlinson [:stomlinson] from comment #7) > :gavin the fonts available from the FirefoxOS Type Guide contains the > complete font sets in a row of links at the bottom. And you're fixing that for accounts.firefox.com in https://github.com/mozilla/fxa-content-server/issues/860, it looks like? Seems like perhaps we should do the same, and just update our versions of the fonts in http://mxr.mozilla.org/mozilla-central/source/browser/base/content/fonts/.
:gavin Yup, I have two fxa-content-server PRs (two different approaches) to serve the correct fonts for the locale. https://github.com/mozilla/fxa-content-server/pull/1087 https://github.com/mozilla/fxa-content-server/pull/1092 I prefer 1092 but am waiting for feedback from other team members on the approach.
Comment on attachment 8419344 [details] [diff] [review] eliminate use of "embedded" fonts in about:accounts. >diff --git a/browser/base/content/aboutaccounts/main.css b/browser/base/content/aboutaccounts/main.css > body { > color: #424f59; >- font-family: "Clear Sans", "Helvetica Neue", Helvetica, Arial, sans-serif; >+ font: message-box; Why specify the font at all? I don't know what it defaults here, but I would assume that should work fine?
Attachment #8419344 - Flags: review?(gavin.sharp) → review+
(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #13) > Comment on attachment 8419344 [details] [diff] [review] > eliminate use of "embedded" fonts in about:accounts. > > >diff --git a/browser/base/content/aboutaccounts/main.css b/browser/base/content/aboutaccounts/main.css > > > body { > > color: #424f59; > >- font-family: "Clear Sans", "Helvetica Neue", Helvetica, Arial, sans-serif; > >+ font: message-box; > > Why specify the font at all? I don't know what it defaults here, but I would > assume that should work fine? If we don't specify anything here, you'll get the browser's default sans-serif font[1] for web content; unless the font prefs have been adjusted, that's most likely Helvetica on OS X, Arial on Windows; Linux configurations may vary. By specifying font:message-box, we get the system's user interface font for "message box" elements, which will be Lucida Grande on OS X, Segoe UI on Win7/8, Tahoma on XP (I think), etc. So it depends how we want this page to be treated. ISTM that from a user's point of view, it's more like part of the browser UI than web content. Hence, I thought specifying font:message-box was preferable to omitting it entirely. Ideally the styling here would also be coordinated with https://accounts.firefox.com (which ends up getting displayed in the same context via an iframe, AIUI). We should follow up on that somehow. [1] sans-serif, not the ultimate default of serif, because normalize.css specifies html { font-family: sans-serif } and so overrides the UA default generic.
(In reply to Jonathan Kew (:jfkthame) from comment #14) > Ideally the styling here would also be coordinated with > https://accounts.firefox.com (which ends up getting displayed in the same > context via an iframe, AIUI). We should follow up on that somehow. I've created https://github.com/mozilla/fxa-content-server/pull/1128 to propose how this could be done - removing all use of downloadable fonts, and relying on font:message-box to provide the platform-appropriate choice. (Note that this PR is not tested; I don't know anything about how the fxa server code is actually built, deployed and run.)
Moving assignee to Johnathan as bugs tracking for the upcoming release shouldn't be assigned to Nobody, and will want to be sure this gets considered for uplift as soon as it's ready.
Assignee: nobody → jfkthame
Landed the reviewed patches here on inbound, to resolve this on the browser side: https://hg.mozilla.org/integration/mozilla-inbound/rev/79d5411c1390 https://hg.mozilla.org/integration/mozilla-inbound/rev/ec4b23a08c5e The question of deploying a similar fix for https://accounts.firefox.com remains open (comments 2, 11, 12, 14, 15); awaiting decisions on what to do with the various PRs for fxa-content-server.
Target Milestone: --- → Firefox 32
Whiteboard: p=0 → p=0 s=it-32c-31a-30b.2 [qa?]
(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #6) > In the short-term, let's move away from Fira Sans and use a "normal" font. > ckarlof says we can switch https://accounts.firefox.com as well. Yes, I apologize if this was misleading. We *can* switch fonts, but I view it to be the designers' call to determine if we should. I'll let jgruen voice his own opinion on this.
Flags: needinfo?(jgruen)
My vote is for NOT changing accounts.firefox.com wholesale err on the side of sacrificing continuity. We probably have a bit more flexibility about locale specific styles on the web where we can't use preferred fonts. Ultimately we should consider moving about:accounts off of the client so we don't have to deal with these issues in future. I'm also curious if this diacritic issue is specific to Fira in general or simply a problem with the version of Fira being used by about:accounts. Since Fira is the default system font for Firefox OS, I can't imagine this issue hasn't been addressed previously.
Flags: needinfo?(jgruen)
(In reply to John Gruen from comment #20) > I'm also curious if this diacritic issue is specific to Fira in general or > simply a problem with the version of Fira being used by about:accounts. Both - about:accounts was using a subsetted version of Fira, which exacerbated the problem, but even the (current) "complete" version lacks characters for some important locales such as Vietnamese, as well as for lesser-known minority languages (e.g. in Africa). > Since Fira is the default system font for Firefox OS, I can't imagine this > issue hasn't been addressed previously. Fira is still a work in progress; it's known to have much less complete character coverage than today's major system fonts. (Yes, that's a problem for Firefox OS; we're working on it.) But we shouldn't impose Fira's current limitations on a site that's being served to desktop systems as well as FxOS phones.
Qa+'ing for a spot check and some exploratory testing.
Whiteboard: p=0 s=it-32c-31a-30b.2 [qa?] → p=0 s=it-32c-31a-30b.2 [qa+]
Assigning to Tracy since it's related to the Sync screens... Tracy let us know if you need help with it.
QA Contact: twalker
I installed Vietnamese Fx as I am familiar with the diacritical marks of that language. However, the only word translated on about:accounts was Sync : Đồng bộ. The diacritical marks look correct to me. I'll need clear STR's to repro and verify.
If you try a Nightly Vietnamese build from before this fix (e.g. http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-05-20-03-02-02-mozilla-central-l10n/firefox-32.0a1.vi.mac.dmg) on OS X 10.7, and look at about:accounts, you should see that two of the letters (ừ and ớ) in "Chào mừng đến với Đồng bộ" are much bolder than the rest. The effect may vary on different platforms/OS versions, depending on available fonts. I suspect it'll be pretty visible on Windows; perhaps less so on newer versions of OS X. Compare the latest Nightly (Vietnamese), and all the text "Chào mừng đến với Đồng bộ" should be consistently styled.
Ah, yes, thank you for describing the issue. It's almost imperceptible, but I do so see slightly bolder ừ, ớ, đ and Đ. on 10.9. It's much more pronounced on Win XP. With latest Nightly, the font style of translated words on about:accounts is consistently bold on both machine.
Status: RESOLVED → VERIFIED
Can we get uplift nominations please so this can be in next week's beta?
Flags: needinfo?(jfkthame)
Patches folded together and rebased to mozilla-beta for uplift.
Comment on attachment 8428073 [details] [diff] [review] eliminate use of "embedded" fonts in about:accounts. Note: the trunk patches transplant cleanly to aurora; for beta, minor rebasing was needed, so I've attached a separate patch. [Approval Request Comment] Bug caused by (feature/regressing bug #): 965117 - custom fonts on about:accounts User impact if declined: poor text rendering for some locales with non-English characters (e.g. central European, Vietnamese, etc) Testing completed (on m-c, etc.): landed & verified on m-c Risk to taking this patch (and alternatives if risky): minimal risk, just style changes on about:accounts String or IDL/UUID changes made by this patch: none
Attachment #8428073 - Flags: approval-mozilla-beta?
Flags: needinfo?(jfkthame)
Attachment #8419343 - Flags: approval-mozilla-aurora?
Attachment #8419344 - Flags: approval-mozilla-aurora?
Attachment #8419343 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8419344 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8428073 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Whiteboard: p=0 s=it-32c-31a-30b.2 [qa+] → p=0 s=it-32c-31a-30b.2 [qa!]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: