Crash because of trying to match non-existing fonts

RESOLVED FIXED in mozilla1.9.1b3

Status

()

Core
Graphics
--
critical
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: Peter Weilbacher, Assigned: Peter Weilbacher)

Tracking

({crash, fixed1.9.1})

Trunk
mozilla1.9.1b3
x86
OS/2
crash, fixed1.9.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

9 years ago
The line
   mAliasForMultiFonts.IndexOfIgnoreCase(fontname)
causes all fonts to be matches of an alias because IndexOfIgnoreCase returns -1 on failure.
(Assignee)

Comment 1

9 years ago
Oops, wanted to add that this causes crashes (at least on OS/2) because the gfxOS2Font class then unsuccessfully tries to create and match fonts that don't exist on the system.
(Assignee)

Comment 2

9 years ago
Created attachment 350301 [details] [diff] [review]
fix typo

Karl, I guess this is what you originally wanted to do in gfxFontconfigUtils::ResolveFontName. At least it fixes the alias name detected and the OS/2 crashes for me.
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Attachment #350301 - Flags: superreview?(roc)
Attachment #350301 - Flags: review?(mozbugz)
Thanks for diagnosing this, Peter.

This should be fixed for 1.9.1.

It won't cause crashes on Linux as gfxPangoFontGroup doesn't use ResolveFontName, but will confuse MathML on Linux when STIX fonts are not installed (as the code will think they are installed).
Flags: blocking1.9.1?
Comment on attachment 350301 [details] [diff] [review]
fix typo

(In reply to comment #2)
> Karl, I guess this is what you originally wanted to do in
> gfxFontconfigUtils::ResolveFontName.

Yes, that is what I intended.

Using kNotFound (defined to be -1 in nsAString.h) would be nice, but I see
that nsCStringArray::IndexOfIgnoreCase actually returns a bare -1, and this
will be sorted out when nsCStringArray is replaced with nsTArray<nsCString>
(bug 461047), so I don't mind what you choose.
Attachment #350301 - Flags: review?(mozbugz) → review+
I'm wondering how the mAliasForMultiFonts changes in bug 449356 will affect
OS/2 when a family from

  pref("font.alias-list", "sans,sans-serif,serif,monospace");

is explicitly specified in CSS, enclosed in quotes. e.g.

  body { font-family: "sans-serif" }

The previous behavior would have been to resolve "sans-serif" to a sorted list
of every family installed, whereas the new behavior would simply leave this as
"sans-serif".

ISTR that OS/2 has its own brand of fontconfig so may behave differently to
the brand that I know.

Does the OS/2 fontconfig have a concept of sans,sans-serif,serif,monospace?
That question is similar to: Is FcConfigSubstitute anything more than a no-op
on OS/2?

If the answer to those questions is "No", then I think the best thing to do
would be to remove the "font.alias-list" pref on OS/2.
(Assignee)

Comment 6

9 years ago
Karl, yes, when the OS/2 version of FC is asked for one of the generic fonts, it matches them to default system fonts (unless the user has overridden that). So leaving sans-serif after the substitution is OK.

But you should not worry about specifics of our implementation. We have implemented the FC functions that cairo and Mozilla (the only two users on OS/2) currently use. We usually fix it to work again every time we find a problem.
Attachment #350301 - Flags: superreview?(roc) → superreview+
(Assignee)

Updated

9 years ago
Attachment #350301 - Flags: approval1.9.1?
(Assignee)

Comment 7

9 years ago
Pushed fix to trunk:
http://hg.mozilla.org/mozilla-central/rev/6156d0a39763
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Attachment #350301 - Flags: approval1.9.1? → approval1.9.1+
Comment on attachment 350301 [details] [diff] [review]
fix typo

a191=beltzner
(Assignee)

Comment 9

9 years ago
Pushed fix to mozilla-1.9.1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/1fa38a636427
Whiteboard: fixed1.9.1
Keywords: fixed1.9.1
Whiteboard: fixed1.9.1
Target Milestone: --- → mozilla1.9.1b3
Flags: blocking1.9.1?
You need to log in before you can comment on or make changes to this bug.