Last Comment Bug 764805 - user fonts with src:local don't handle style descriptors properly under GDI
: user fonts with src:local don't handle style descriptors properly under GDI
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Layout: Text (show other bugs)
: unspecified
: x86 Windows XP
: -- normal (vote)
: mozilla16
Assigned To: Jonathan Kew (:jfkthame)
:
Mentors:
Depends on:
Blocks: 724231
  Show dependency treegraph
 
Reported: 2012-06-14 07:00 PDT by Jonathan Kew (:jfkthame)
Modified: 2012-07-04 06:28 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch, fix handling of src:local style properties in the GDI backend (2.06 KB, patch)
2012-06-14 07:00 PDT, Jonathan Kew (:jfkthame)
jd.bugzilla: review+
Details | Diff | Review
reftest for src:local fonts with mismatched style properties (3.00 KB, patch)
2012-06-14 07:02 PDT, Jonathan Kew (:jfkthame)
no flags Details | Diff | Review
reftest for src:local fonts with mismatched style properties (3.00 KB, patch)
2012-06-14 07:49 PDT, Jonathan Kew (:jfkthame)
jd.bugzilla: review+
Details | Diff | Review
jdaggett's synth-italic testcase on XP (107.90 KB, image/png)
2012-06-26 05:07 PDT, Jonathan Kew (:jfkthame)
no flags Details

Description Jonathan Kew (:jfkthame) 2012-06-14 07:00:12 PDT
Created attachment 633122 [details] [diff] [review]
patch, fix handling of src:local style properties in the GDI backend

As noted in bug 754452 comment 27, the GDI back-end doesn't correctly handle the case where a regular face is loaded with src:local() and given an italic (or bold) descriptor. The associated style property will be set in the LOGFONT for the font entry, which results in GDI actually using the styled face (or applying its own synthetic styling) rather than using the intended regular face.

This patch fixes the issue by using the properties of the original platform font entry when creating the new user font entry, so that the resulting LOGFONT will match, and then updating the style properties to match the proxy, which has the values from the @font-face rule.
Comment 1 Jonathan Kew (:jfkthame) 2012-06-14 07:02:22 PDT
Created attachment 633123 [details] [diff] [review]
reftest for src:local fonts with mismatched style properties

Here's a reftest for the src:local styling issue; pushing to tryserver to see what happens across platforms.
Comment 2 Jonathan Kew (:jfkthame) 2012-06-14 07:49:21 PDT
Created attachment 633135 [details] [diff] [review]
reftest for src:local fonts with mismatched style properties

Oops, it won't work with typos in the reftest manifest! :( Trying again...
Comment 3 John Daggett (:jtd) 2012-06-25 19:27:46 PDT
This still doesn't handle all cases correctly:

http://people.mozilla.org/~jdaggett/tests/synthetic-italics-tests.html

The regular-as-italic case is incorrect when tested on XP.
Comment 4 Jonathan Kew (:jfkthame) 2012-06-26 05:07:53 PDT
Created attachment 636662 [details]
jdaggett's synth-italic testcase on XP

It looked correct when I tried it (see attached). And the reftest passed tryserver. Could you give more specifics of the failure you see?
Comment 5 John Daggett (:jtd) 2012-06-26 21:37:25 PDT
Ok, my bad, I was testing with a profile with some funky settings in it.  With a new profile looks fine.
Comment 6 John Daggett (:jtd) 2012-06-26 21:38:28 PDT
Comment on attachment 633135 [details] [diff] [review]
reftest for src:local fonts with mismatched style properties

Looks fine but we should have a bug for the Android failures.  Or simply include Droid Sans/Roboto in the set of fonts used.
Comment 7 Jonathan Kew (:jfkthame) 2012-06-28 03:02:13 PDT
I tried adding Droid Sans and Roboto, but the test still fails - the problem with both this and the existing local-1 testcase is that src:local(...) is completely broken on Android, so the user fonts don't get used at all. Filed bug 769194 to deal with this.

Pushed to inbound, with Droid Sans/Roboto added to the tests, but they still won't pass until 769194 is fixed.
https://hg.mozilla.org/integration/mozilla-inbound/rev/2ab61c8b46d8
https://hg.mozilla.org/integration/mozilla-inbound/rev/367d0809c8f9

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