611855 - h1 and h2 headers are displayed in italic (after OS X 10.6.5 update)

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[John Wayne Hill (UX Intern)]

Firefox 4 Beta 7

H1 and H2 headers display text in italic even when specific to display text as normal.

Examples:
http://www.johnwaynehill.com/blog/

http://bjoyski.com/blog/

Neither of these headers (large green and orange links) should be in italic, but they are.

Comment 1

[j.j.]

Please attach a testcase, otherwise a layout bug should be UNCONFIRMED rather than NEW.
The appropriate Product/Component for this bug is  Core/Layout (I think).

Comment 2

[philippe (part-time)]

This appears to be specific to 'helvetica', combined with 'font-weight: lighter'.

I can reproduce that on Gecko 1.9.2 (at least back to 2010.06.15 builds) and Gecko 2.0.

Comment 3

[philippe (part-time)]

Attachment: testcase

disable either 'helvetica' or 'font-weight: lighter' and the text displays normally.

Firebug says that the computed style for font-style is 'normal'.

Comment 4

[philippe (part-time)]

Tested on 10.6.5
works
Gecko/20090816 Minefield/3.7a1pre
http://hg.mozilla.org/mozilla-central/rev/ef2e4699b319

fails
Gecko/20090817 Minefield/3.7a1pre
http://hg.mozilla.org/mozilla-central/rev/d9742d839d7d

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ef2e4699b319&tochange=d9742d839d7d

--> bug 493280 ?

I can't reproduce this on OS X 10.5. 

Did this display correctly on versions of OS X 10.6 prior to 10.6.5 ? I don't remember ever seeing this with a bunch of test cases I have on my local server.

Comment 5

[Alex Miller]

I can't reproduce in Ubuntu 10.10

Comment 6

[Boris Zbarsky [:bzbarsky]]

I can't reproduce, using 10.6.4.

Comment 7

[philippe (part-time)]

The 10.6.5 release notes contains this snippet: 'Resolves spacing issues with OpenType fonts'.
http://support.apple.com/kb/HT4250

The security part of the package also mention changes to ATS.
http://support.apple.com/kb/HT4435

Comment 8

[Jonathan Kew [:jfkthame]]

Confirmed that this does not happen with 10.6.4, but does after updating to 10.6.5.

The issue seems to be that 10.6.5 introduces new Light and LightOblique faces of Helvetica, but Cocoa does not return correct font-traits for the LightOblique face. It fails to set the NSItalicFontMask bit in the traits entry of the array returned by [availableMembersOfFontFamily] (even though the italic bit IS set in the macStyle field of the 'head' table), and so our code sees both the Light and LightOblique faces as having identical attributes, and the oblique one happens to "win".

Comment 9

[Boris Zbarsky [:bzbarsky]]

We need to get a bug reported to Apple here, right?  No matter what we do?

Comment 10

[Jonathan Kew [:jfkthame]]

Yes. Filed rdar://8666610.

Note that the underlying problem can also be demonstrated using TextEdit.app, as follows:

1. Launch TextEdit, and enter "Hello World" in a new document. Select all the text, and open the Fonts panel.

2. Click on the Helvetica family: the text will appear in Helvetica Regular, as expected.

3. Click on the Light face: text changes to Helvetica Light, as expected.

4. Now press Command-I to apply italic (or oblique) style: text changes to Helvetica Oblique (i.e. in Regular weight) instead of the expected Helvetica Light Oblique.

5. Click on the Light Oblique face, to explicitly choose Helvetica Light Oblique: displays correctly.

6. Now press Command-I to toggle the oblique style off: text changes to Helvetica Oblique instead of the expected Helvetica Light. (And pressing Command-I again then changes to Helvetica Regular.)

Comment 11

[Jonathan Kew [:jfkthame]]

Attachment: patch, v1 - check style name in case 'italic' traits not set properly

Although this is really an Apple bug, I think it's worth adding a small workaround in our code so that people don't end up with unexpected italics like this. The reports here show that it's an issue that occurs "in the wild" on real sites.

This patch just checks whether the style name ends with "Italic" or "Oblique", and marks the face as italic even if the relevant font-traits bit was not set. (Note that the NSFontManager documentation specifies that the style name is NOT localized, so just checking the English names should be OK here.)

An alternative would be to load the 'head' table and check the macStyle field, but I'm inclined to go for the simplest code at this point.

Comment 12

[John Daggett (:jtd)]

Comment on attachment 490462 [details] [diff] [review]
patch, v1 - check style name in case 'italic' traits not set properly

Looks like this only affects Helvetica Light Italic.  Weirdly, the font name table has "Helvetica Light Oblique", maybe this name table oddness that relates to the problem.  All the other italic flags (macStyle in 'head', fsSelection in 'OS/2') look like they're set correctly.  

I completely hate the idea of putting in this sort of thing to fix a single font but I guess it's the best we can do.  I imagine fixing this will be a low priority at Apple.

Comment 13

[Jonathan Kew [:jfkthame]]

(In reply to comment #12)

> Looks like this only affects Helvetica Light Italic.  Weirdly, the font name
> table has "Helvetica Light Oblique", maybe this name table oddness that relates
> to the problem.  All the other italic flags (macStyle in 'head', fsSelection in
> 'OS/2') look like they're set correctly.  

Yes, I checked those thinking it could be a bug in the actual font, but couldn't find a cause there. Which makes me suspect that it's a bug in how the Cocoa font system manages families and faces, and so it could potentially affect other families as well if they happen to have the right collection of faces to stumble on the error.

> I completely hate the idea of putting in this sort of thing to fix a single
> font but I guess it's the best we can do.  I imagine fixing this will be a low
> priority at Apple.

Yup, I'm afraid so. It's not the first time we've had to do such a thing - and probably won't be the last! The one "good" thing in this case is that the bug manifests as user-visible incorrect behavior in Apple apps such as TextEdit, too.

Comment 14

[Jonathan Kew [:jfkthame]]

Comment on attachment 490462 [details] [diff] [review]
patch, v1 - check style name in case 'italic' traits not set properly

Requesting approval to land for 2.0, as this causes incorrect styling on real-world sites, and OS X 10.6.5 will rapidly become widespread via Apple Software Update, so that an increasing number of users will see the bug.

Comment 15

[philippe (part-time)]

Can we get this on Gecko 1.9.2 as well, please ?

Comment 16

[Daniel Veditz [:dveditz]]

Not going to block a branch release on this, but if you get it tested on the trunk and request branch approval we can consider taking it in a branch release. Code-freeze for the current release is Thursday this week (11/18).

Comment 17

[Jonathan Kew [:jfkthame]]

Pushed to trunk:
http://hg.mozilla.org/mozilla-central/rev/fedc658ef505

Comment 18

[Jonathan Kew [:jfkthame]]

Attachment: patch for 1.9.2 branch

This is the patch for 1.9.2 (it's the same code, just moved around a bit). Requesting approval to land on branch so that we don't get the incorrect font selection in FF3.6 on the latest OS X update.

Comment 19

[LegNeato]

Comment on attachment 490842 [details] [diff] [review]
patch for 1.9.2 branch

a=LegNeato for 1.9.2.13.

Should we take this on 1.9.1 as well?

Comment 20

[philippe (part-time)]

(In reply to comment #19)
> Comment on attachment 490842 [details] [diff] [review]
> patch for 1.9.2 branch
 
> Should we take this on 1.9.1 as well?

1.9.1 is not affected

Comment 21

[Jonathan Kew [:jfkthame]]

Pushed to 1.9.2:
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/28d98b480e3d