Closed Bug 523717 Opened 13 years ago Closed 13 years ago

incorrect layout after downloadable font with smaller underline offset loads

Categories

(Core :: Graphics, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dbaron, Assigned: dbaron)

Details

Attachments

(1 file, 2 obsolete files)

Attached patch patch (obsolete) — Splinter Review
I was trying to reproduce the problem in bug 522223 on Linux, but I couldn't.  However, while doing this, I noticed (bug 522223 comment 17) that the layout was incorrect after the user font loaded, since it changed after I did a zoom-in and then zoom-out.

This is because we're not clearing the cached underline offset.

The attached patch fixes the problem for me on Linux; I haven't yet written an automated test, since that's hard (for me anyway...I'd need to re-remember how to do font generation and generate test fonts with different underline offsets), though I might try later.

Presumably the Core Text backend needs the same fix.

I'd note that the clearing of mUnderlineOffset *was* added for Windows in http://hg.mozilla.org/mozilla-central/rev/28eea1633fe1 (inside InitFontList) and Mac ATSUI in http://hg.mozilla.org/mozilla-central/rev/c84b63d503df .

(Should we move the Windows one out of InitFontList to be consistent with the other platforms?)
Flags: in-testsuite?
Attachment #407630 - Flags: review?(jdaggett)
Attached patch patch (obsolete) — Splinter Review
Here's a patch with a test that fails prior to the patch (it's the final != test that fails, actually).

Tomorrow I'll try to remember to figure out if the font I finally generated that made this work actually has a low underline or a high one.
Attachment #407630 - Attachment is obsolete: true
Attachment #407935 - Flags: review?(jdaggett)
Attachment #407630 - Flags: review?(jdaggett)
Attached patch patchSplinter Review
Yeah, the font that actually caused a failure without the patch is one with a higher underline offset, not lower.
Attachment #407935 - Attachment is obsolete: true
Attachment #407940 - Flags: review?(jdaggett)
Attachment #407935 - Flags: review?(jdaggett)
Comment on attachment 407940 [details] [diff] [review]
patch

Looks good.
Attachment #407940 - Flags: review?(jdaggett) → review+
http://hg.mozilla.org/mozilla-central/rev/b06d6479f8a4
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
And I filed bug 534132 on generating the test font better so it works on Windows and Mac as well; I gave up trying after some fighting with it.
You need to log in before you can comment on or make changes to this bug.