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)
Tracking
()
VERIFIED
FIXED
Firefox 32
People
(Reporter: mstanke, Assigned: jfkthame)
References
Details
(Whiteboard: p=0 s=it-32c-31a-30b.2 [qa!])
Attachments
(4 files)
(deleted),
image/jpeg
|
Details | |
(deleted),
patch
|
Gavin
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Gavin
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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.
Comment 1•11 years ago
|
||
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)
Reporter | ||
Comment 2•11 years ago
|
||
Yes, it does on https://accounts.firefox.com/signup too.
Comment 3•11 years ago
|
||
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)
Reporter | ||
Comment 4•11 years ago
|
||
Filled on GitHub too - https://github.com/mozilla/Fira/issues/26 . Hope this will be fixed before Australis and the new Sync launch.
Updated•11 years ago
|
Updated•11 years ago
|
Flags: firefox-backlog+
Whiteboard: p=0
Comment 6•11 years ago
|
||
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.
status-firefox30:
--- → affected
status-firefox31:
--- → affected
tracking-firefox30:
--- → +
tracking-firefox31:
--- → +
Comment 7•11 years ago
|
||
:gavin the fonts available from the FirefoxOS Type Guide contains the complete font sets in a row of links at the bottom.
Assignee | ||
Comment 8•11 years ago
|
||
Attachment #8419343 -
Flags: review?(gavin.sharp)
Assignee | ||
Comment 9•11 years ago
|
||
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)
Updated•11 years ago
|
Summary: Fira Sans diacritic issue in about:accounts → Fira Sans font used on about:accounts is incomplete (issues with diacritics, latin-extended glyphs)
Updated•11 years ago
|
Attachment #8419343 -
Flags: review?(gavin.sharp) → review+
Comment 11•11 years ago
|
||
(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/.
Comment 12•11 years ago
|
||
: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 13•11 years ago
|
||
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+
Assignee | ||
Comment 14•11 years ago
|
||
(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.
Assignee | ||
Comment 15•11 years ago
|
||
(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.)
Comment 16•11 years ago
|
||
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
Assignee | ||
Comment 17•11 years ago
|
||
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
Comment 18•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/79d5411c1390
https://hg.mozilla.org/mozilla-central/rev/ec4b23a08c5e
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Whiteboard: p=0 → p=0 s=it-32c-31a-30b.2 [qa?]
Comment 19•11 years ago
|
||
(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)
Comment 20•11 years ago
|
||
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)
Assignee | ||
Comment 21•11 years ago
|
||
(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.
Comment 22•11 years ago
|
||
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+]
Comment 23•11 years ago
|
||
Assigning to Tracy since it's related to the Sync screens... Tracy let us know if you need help with it.
QA Contact: twalker
Comment 24•11 years ago
|
||
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.
Assignee | ||
Comment 25•11 years ago
|
||
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.
Comment 26•11 years ago
|
||
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
Comment 27•11 years ago
|
||
Can we get uplift nominations please so this can be in next week's beta?
Flags: needinfo?(jfkthame)
Assignee | ||
Comment 28•11 years ago
|
||
Patches folded together and rebased to mozilla-beta for uplift.
Assignee | ||
Comment 29•11 years ago
|
||
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)
Assignee | ||
Updated•11 years ago
|
Attachment #8419343 -
Flags: approval-mozilla-aurora?
Assignee | ||
Updated•11 years ago
|
Attachment #8419344 -
Flags: approval-mozilla-aurora?
Updated•11 years ago
|
Attachment #8419343 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•11 years ago
|
Attachment #8419344 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•11 years ago
|
Attachment #8428073 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Assignee | ||
Comment 30•11 years ago
|
||
Assignee | ||
Comment 31•11 years ago
|
||
Updated•11 years ago
|
status-firefox32:
--- → fixed
Comment 32•11 years ago
|
||
Updated•11 years ago
|
Updated•10 years ago
|
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.
Description
•