1221263 - Font "Hoefler Text" has odd word spacing on mac in Firefox 42

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[matthew.nahmias]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2490.71 Safari/537.36

Steps to reproduce:

Used the css p{font-family:"Hoefler Text", arial}


Actual results:

Spacing character between words is double normal width

http://codepen.io/anon/pen/ZbjmBJ


Expected results:

Spacing should be a single character wide

Comment 1

[Benjamin Smedberg]

Is Hoefler Text a font that comes with MacOS, or is it a font that we could try on other OSes through direct installation or even a webfont?

Comment 2

[matthew.nahmias]

Hoefler is a macOS system font, but also has a webfont at http://www.typography.com/fonts/hoefler-text/webfonts/ 

The webfont is unaffected.

Comment 3

[Jonathan Kew [:jfkthame]]

The webfont may not be the same as the font shipped with OS X, which uses AAT layout features. (The webfont is likely to use OpenType layout features instead.) So it's not surprising they give different results.

Comment 4

[Jonathan Kew [:jfkthame]]

The problem here occurs on FF42 and 43 (beta), but did not occur on FF41, and also not with current Developer Edition (44) or Nightly (45).

What's happening is that the space characters are being assigned the Hoefler Text Ornaments face, instead of the regular Hoefler Text face. Ornaments doesn't support the actual letters, so those still use the Regular face, but it does have a space, and its space is wider than the one in Regular. :(

The first Nightly build where I could reproduce this bug was 2015-09-03, and then it is fixed again in the build from 2015-10-09. This points to a regression range:

  https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=fb720c90eb49&tochange=a6786bf8d71d

and a subsequent fix range of:

  https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a955ea9382af&tochange=d01dd42e654b

Given those ranges, I believe this was broken in bug 1185812, which reversed the order in which we iterate over font faces within a family during style matching; and then (accidentally) fixed in bug 1201318, though I haven't looked at the patches there in detail.

What I think bug 1185812 should have done, to avoid the breakage in the first place, would have been to reverse the comparator used by gfxFontFamily::SortAvailableFonts(), as that's what gfxMacPlatformFontList relies on to give priority to the standard faces in a family like Hoefler. Reversing the iteration order during style matching, without reversing the sort, is what caused the bug.

If we don't uplift the whole of bug 1201318 to beta (I see the patches there are currently marked with an approval request), then I think we should take a simple, safe patch here to just fix this issue as described above.

We should also add a reftest here.

Comment 5

[Jonathan Kew [:jfkthame]]

Attachment: Sort 'standard' faces ahead of others within Mac OS font families

Here's a patch for beta; in the event we don't uplift bug 1201318, I think we should take this as a minimal fix.

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: Reftest to check that we use the Regular face of Hoefler Text, even for spaces

And here's a simple reftest that fails on beta, and passes once this bug is fixed. (It should pass harmlessly on other platforms where Hoefler Text isn't present.)

Comment 7

[John Daggett (:jtd)]

Comment on attachment 8683330 [details] [diff] [review]
Sort 'standard' faces ahead of others within Mac OS font families

(In reply to Jonathan Kew (:jfkthame) from comment #4)

r+ for landing on mozilla-beta

> Given those ranges, I believe this was broken in bug 1185812, which reversed
> the order in which we iterate over font faces within a family during style
> matching; and then (accidentally) fixed in bug 1201318, though I haven't
> looked at the patches there in detail.

Font style matching went from matching the *last* face with the same style distance to matching all faces in the order matched. That effectively reversed which face was matched. To fix that, without altering the order for userfonts, I explicitly reversed the order. When writing the patch for the OSX system font bug, I noticed that this messed up the ordering of regular vs. non-regular faces since the San Francisco family on OSX 10.11 has multiple faces for each style combination. There was nothing "accidental" about the fix.

Comment 8

[Jonathan Kew [:jfkthame]]

Comment on attachment 8683330 [details] [diff] [review]
Sort 'standard' faces ahead of others within Mac OS font families

Approval Request Comment

[Feature/regressing bug #]: bug 1185812

[User impact if declined]: bad word spacing when using the OS X font "Hoefler Text"; other font families that include non-standard faces could also be affected

[Describe test coverage new/current, TreeHerder]: new reftest ready to land along with the patch

[Risks and why]: minimal, OS X-only patch to restore the font precedence we were using prior to bug 1185812

[String/UUID change made/needed]: n/a

NOTE: the issue here is already fixed (as part of bug 1201318) on trunk and Aurora, so the proposed patch here is Beta-only. If the Beta uplift request in bug 1201318 is accepted, it will supersede this one; but if that is considered too large/risky for Beta, we should take this minimal fix for the regression here.

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

jtd, any luck here? We could still take a patch for beta 4 or 5.

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Ah, wrong bug, I meant comment 10 for bug 1201318.

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

Fixed by the uplift in bug 1201318.

Comment 13

[John Daggett (:jtd)]

Resolving as fixed. Will reopen if there's a problem with bug 1201318

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8683330 [details] [diff] [review]
Sort 'standard' faces ahead of others within Mac OS font families

This was handled in another bug, so the uplift isn't needed.