Last Comment Bug 766326 - ###!!! ASSERTION: invalid default font returned by GetDefaultFont: 'defaultFont'
: ###!!! ASSERTION: invalid default font returned by GetDefaultFont: 'defaultFont'
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics: Text (show other bugs)
: unspecified
: ARM Android
: -- normal (vote)
: mozilla16
Assigned To: Jonathan Kew (:jfkthame)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-19 14:38 PDT by Brad Lassey [:blassey] (use needinfo?)
Modified: 2012-06-29 14:51 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed
.N+
14+


Attachments
patch, try Roboto as well as Droid Sans for Android default font (1.18 KB, patch)
2012-06-20 13:12 PDT, Jonathan Kew (:jfkthame)
blassey.bugs: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Brad Lassey [:blassey] (use needinfo?) 2012-06-19 14:38:39 PDT
I'm seeing this assertion very often on Android. If it really is an error condition, we should fix it. If it is not, we should remove the assertion.
Comment 1 Joe Drew (not getting mail) 2012-06-20 12:49:52 PDT
Is this a bad assertion?
Comment 2 Brad Lassey [:blassey] (use needinfo?) 2012-06-20 12:51:48 PDT
https://mxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxFont.cpp#2953
Comment 3 Jonathan Kew (:jfkthame) 2012-06-20 13:08:08 PDT
It's not a bad assertion, in that we really really expect that GetDefaultFont() will return something usable, to avoid the risk of ending up with a fontGroup that has no actual fonts at all, leading to all kinds of sadness when we ask for metrics, etc.

Hmm... on Android, gfxFT2FontList::GetDefaultFont() is hardcoded to look for Droid Sans. That's probably not a good thing on ICS, where I believe it has been replaced by Roboto. I'll post a patch...
Comment 4 Jonathan Kew (:jfkthame) 2012-06-20 13:12:35 PDT
Created attachment 635027 [details] [diff] [review]
patch, try Roboto as well as Droid Sans for Android default font

I think this should help, if I'm understanding the cause correctly.
Comment 5 Brad Lassey [:blassey] (use needinfo?) 2012-06-20 15:08:51 PDT
Comment on attachment 635027 [details] [diff] [review]
patch, try Roboto as well as Droid Sans for Android default font

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

nit, reverse the order of Droid Sans and Roboto. If both happen to be on a device, I think we'd rather have Roboto
Comment 6 Jonathan Kew (:jfkthame) 2012-06-20 23:51:07 PDT
Pushed to inbound, rearranged to try Roboto first, then Droid Sans.
https://hg.mozilla.org/integration/mozilla-inbound/rev/e24a42876bc5

I'm assuming this will stop these assertions on ICS devices, but don't have one on hand for testing; please re-open if it doesn't solve the problem.
Comment 7 Ed Morley [:emorley] 2012-06-21 04:02:33 PDT
https://hg.mozilla.org/mozilla-central/rev/e24a42876bc5
Comment 8 John Daggett (:jtd) 2012-06-21 04:49:00 PDT
Brad, does this fix the assertions you were seeing?
Comment 9 Brad Lassey [:blassey] (use needinfo?) 2012-06-27 12:40:21 PDT
Comment on attachment 635027 [details] [diff] [review]
patch, try Roboto as well as Droid Sans for Android default font

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch:
Comment 10 Lukas Blakk [:lsblakk] use ?needinfo 2012-06-27 15:39:11 PDT
Comment on attachment 635027 [details] [diff] [review]
patch, try Roboto as well as Droid Sans for Android default font

so tiny and low risk it's not worth an approval request form completion, eh? :)

Note You need to log in before you can comment on or make changes to this bug.