Really make Fira Sans the default font

VERIFIED FIXED in Firefox 26, Firefox OS v1.1hd

Status

()

Core
Widget: Gonk
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: mwu, Assigned: jfkthame)

Tracking

unspecified
mozilla27
All
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:hd+, firefox25 wontfix, firefox26 fixed, firefox27 fixed, b2g18 wontfix, b2g-v1.1hd fixed, b2g-v1.2 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Created attachment 808932 [details] [diff] [review]
Set the default font in more places

Apparently prefs only specify defaults for when people ask for things like sans-serif and serif. When nothing is specified at all, we fall back on Roboto. This patch fixes that. It also fixes the font setting in nsLookAndFeel.cpp, though I don't think that's really used..
Attachment #808932 - Flags: review?(jfkthame)
(Reporter)

Comment 1

5 years ago
I also noticed that gfxFT2FontList.cpp cares about some list of standard fonts. Should we care about that list? It seems like every font in /system/fonts/ should be considered a standard font for that phone.
(Assignee)

Comment 3

5 years ago
Comment on attachment 808932 [details] [diff] [review]
Set the default font in more places

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

Oops - yeah, missed these earlier.

Maybe we should have a central method (in gfxPlatform, perhaps) to return the platform's default font, and then nsLookAndFeel could simply use that, so it wouldn't need to be maintained in both places. Sounds like a possible followup.
Attachment #808932 - Flags: review?(jfkthame) → review+
(Assignee)

Comment 4

5 years ago
(In reply to Michael Wu [:mwu] from comment #1)
> I also noticed that gfxFT2FontList.cpp cares about some list of standard
> fonts. Should we care about that list? It seems like every font in
> /system/fonts/ should be considered a standard font for that phone.

I doubt it's important at this point. That was done (in bug 636042) to work around problems on some Android phones where the device manufacturer ships confused/broken collections of fonts, but for b2g we should simply not ship such a mess!
(Reporter)

Comment 5

5 years ago
I'm going to skip text-font-lang.html on B2G. All other tests seem fine.

(In reply to Jonathan Kew (:jfkthame) from comment #4)
> I doubt it's important at this point. That was done (in bug 636042) to work
> around problems on some Android phones where the device manufacturer ships
> confused/broken collections of fonts, but for b2g we should simply not ship
> such a mess!

Ah. We should probably disable that then, since we don't want to prefer Droid Sans/Roboto over our real defaults.
(Reporter)

Comment 7

5 years ago
Patryk, once this lands and is available in a build, I'd like to get verification that we've really switched all the fonts over to Fira Sans.
Flags: needinfo?(padamczyk)
https://hg.mozilla.org/mozilla-central/rev/55e06972ede0
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Yes please let me know when it lands, I tried Mozilla Central Sept 25th and it didn't looked fixed.
Flags: needinfo?(padamczyk) → needinfo?(mwu)
(Reporter)

Comment 10

5 years ago
Hm, I just tried the latest n4 build and it looks fixed in the notifications area.
Flags: needinfo?(mwu) → needinfo?(padamczyk)
Just loaded moz central on my unagi...  IT WORKS!!!!!!!!!!!!!!! So happy notification's Clear All tag was broken since the beginning of time, so I am really happy its finally fixed!
Flags: needinfo?(padamczyk)
er... So happy notification's Clear All tag is now fixed, was broken since the beginning of time, so I am really happy its finally fixed!
(Reporter)

Comment 13

5 years ago
\o/
Status: RESOLVED → VERIFIED
Blocks: 915894
This escaped triage, and its required on v1.1hd for the font update.
blocking-b2g: hd? → hd+
Keywords: checkin-needed

Updated

5 years ago
Whiteboard: [checkin-needed-v1.1hd]
https://hg.mozilla.org/releases/mozilla-aurora/rev/52f24889dccc
https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/19e4b6acf344
status-b2g18: --- → wontfix
status-b2g-v1.1hd: --- → fixed
status-b2g-v1.2: --- → fixed
status-firefox25: --- → wontfix
status-firefox26: --- → fixed
status-firefox27: --- → fixed
Keywords: checkin-needed
Whiteboard: [checkin-needed-v1.1hd]
status-b2g-v1.2: fixed → affected
(Reporter)

Comment 18

5 years ago
Ah. That test was disabled in the commit but not in the patch posted here.
Easy enough, I'll re-land on Aurora with that test disabled. Leaving the v1.1hd patch up to you, though :)
Flags: needinfo?(mwu)
Keywords: branch-patch-needed
(Assignee)

Comment 20

5 years ago
Created attachment 822912 [details] [diff] [review]
Really make Fira Sans the default font, b2g18 version.

Ah, b2g18 doesn't have bug 821442, which changed the return type of GetDefaultFont(). So this version of the patch ought to be what's needed there.
Attachment #822912 - Flags: review?(mwu)
(Assignee)

Updated

5 years ago
Assignee: mwu → jfkthame
(Reporter)

Comment 21

5 years ago
Comment on attachment 822912 [details] [diff] [review]
Really make Fira Sans the default font, b2g18 version.

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

Thanks!
Attachment #822912 - Flags: review?(mwu) → review+
(Assignee)

Comment 22

5 years ago
Re-landed, hoping I did this correctly for the v1.1hd tree:

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/59120fd99bbf
LGTM, thanks :)
status-b2g-v1.1hd: affected → fixed
Flags: needinfo?(mwu)
Keywords: branch-patch-needed
Aurora attempt #2:
https://hg.mozilla.org/releases/mozilla-aurora/rev/0ac1262bd75f
status-b2g-v1.2: affected → fixed
status-firefox26: affected → fixed

Updated

4 years ago
Depends on: 1010529
You need to log in before you can comment on or make changes to this bug.