919845 - Really make Fira Sans the default font

Status: VERIFIED FIXED

(Core Graveyard :: Widget: Gonk, defect)

Description

[Michael Wu [:mwu]]

Attachment: 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..

Comment 1

[Michael Wu [:mwu]]

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.

Comment 2

[Michael Wu [:mwu]]

https://tbpl.mozilla.org/?tree=Try&rev=ad9c4482145c

Comment 3

[Jonathan Kew [:jfkthame]]

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.

Comment 4

[Jonathan Kew [:jfkthame]]

(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!

Comment 5

[Michael Wu [:mwu]]

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.

Comment 6

[Michael Wu [:mwu]]

https://hg.mozilla.org/integration/b2g-inbound/rev/55e06972ede0

Comment 7

[Michael Wu [:mwu]]

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.

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/55e06972ede0

Comment 9

[Patryk Adamczyk [:patryk] UX]

Yes please let me know when it lands, I tried Mozilla Central Sept 25th and it didn't looked fixed.

Comment 10

[Michael Wu [:mwu]]

Hm, I just tried the latest n4 build and it looks fixed in the notifications area.

Comment 11

[Patryk Adamczyk [:patryk] UX]

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!

Comment 12

[Patryk Adamczyk [:patryk] UX]

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!

Comment 13

[Michael Wu [:mwu]]

\o/

Comment 14

[Wayne Chang [:wchang]]

This escaped triage, and its required on v1.1hd for the font update.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/52f24889dccc
https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/19e4b6acf344

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Backed out. Needs some fixing up for v1.1hd.
https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/a5e528bf81cd

https://tbpl.mozilla.org/php/getParsedLog.php?id=29730177&tree=Mozilla-B2g18-v1.1.0hd

Comment 17

[Phil Ringnalda (:philor)]

Backed out of aurora in https://hg.mozilla.org/releases/mozilla-aurora/rev/8c5185e4bb21 for the reftest failure in https://tbpl.mozilla.org/php/getParsedLog.php?id=29733611&tree=Mozilla-Aurora

Comment 18

[Michael Wu [:mwu]]

Ah. That test was disabled in the commit but not in the patch posted here.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Easy enough, I'll re-land on Aurora with that test disabled. Leaving the v1.1hd patch up to you, though :)

Comment 20

[Jonathan Kew [:jfkthame]]

Attachment: 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.

Comment 21

[Michael Wu [:mwu]]

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

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

Thanks!

Comment 22

[Jonathan Kew [:jfkthame]]

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

Comment 23

[Ryan VanderMeulen [:RyanVM]]

LGTM, thanks :)

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Aurora attempt #2:
https://hg.mozilla.org/releases/mozilla-aurora/rev/0ac1262bd75f