Last Comment Bug 668758 - @font-face rules can interfere with generic-font prefs
: @font-face rules can interfere with generic-font prefs
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Layout: Text (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: ---
Assigned To: Jonathan Kew (:jfkthame)
:
Mentors:
Depends on: 671385 672951
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-01 02:57 PDT by Jonathan Kew (:jfkthame)
Modified: 2011-07-20 14:38 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
example of the unexpected effect of @font-face on prefs fonts (607 bytes, text/html)
2011-07-01 02:59 PDT, Jonathan Kew (:jfkthame)
no flags Details
fixed example so it demonstrates the issue on Windows as well (809 bytes, text/html)
2011-07-01 03:49 PDT, Jonathan Kew (:jfkthame)
no flags Details
patch, don't look up prefs font families in the user font set (10.01 KB, patch)
2011-07-01 06:56 PDT, Jonathan Kew (:jfkthame)
no flags Details | Diff | Review
reftest (3.29 KB, patch)
2011-07-01 06:56 PDT, Jonathan Kew (:jfkthame)
jd.bugzilla: review+
Details | Diff | Review
patch, don't look up prefs font families in the user font set (12.36 KB, patch)
2011-07-01 15:16 PDT, Jonathan Kew (:jfkthame)
jd.bugzilla: review+
Details | Diff | Review

Description Jonathan Kew (:jfkthame) 2011-07-01 02:57:17 PDT
CSS generics such as 'serif' are mapped to (language-dependent) font family names on the platform via the font preferences such as font.default.serif.x-western.

However, if a @font-face rule happens to use the same family name as was used in the font prefs, the downloaded font will be used instead of the original platform font.

Is this expected behavior? It seems wrong to me; I'd expect the mapping of generics to platform fonts via the preferences to be unaffected by @font-face usage on the page.
Comment 1 Jonathan Kew (:jfkthame) 2011-07-01 02:59:22 PDT
Created attachment 543384 [details]
example of the unexpected effect of @font-face on prefs fonts

This demonstrates the unexpected interaction on OS X, and should work similarly on Windows AFAICS.
Comment 2 Jonathan Kew (:jfkthame) 2011-07-01 03:49:43 PDT
Created attachment 543387 [details]
fixed example so it demonstrates the issue on Windows as well
Comment 3 Jonathan Kew (:jfkthame) 2011-07-01 06:56:13 PDT
Created attachment 543419 [details] [diff] [review]
patch, don't look up prefs font families in the user font set

Assuming the current behavior is not actually wanted, here's an attempt to fix it by passing a flag in gfxFontGroup::ForEachFontInternal to tell it whether it should be checking the user font set or not.
Comment 4 Jonathan Kew (:jfkthame) 2011-07-01 06:56:54 PDT
Created attachment 543420 [details] [diff] [review]
reftest
Comment 5 Jonathan Kew (:jfkthame) 2011-07-01 15:16:09 PDT
Created attachment 543525 [details] [diff] [review]
patch, don't look up prefs font families in the user font set

Updated to fix build failure on Linux.
Comment 6 John Daggett (:jtd) 2011-07-12 09:38:33 PDT
Comment on attachment 543525 [details] [diff] [review]
patch, don't look up prefs font families in the user font set

Looks good but we really need to clean up the generic family code at some point, it's somewhat silly that we're parsing "serif" at this level.  dbaron and I talked about this briefly in the past, I think we should be passing in a font list in an array with generics as special tokens, since the style system has already resolved these.  But that's going to take a lot of effort to untangle existing code across platforms.
Comment 7 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-07-12 09:43:10 PDT
...which is bug 280443 (at least mostly).
Comment 9 Jonathan Kew (:jfkthame) 2011-07-13 03:02:21 PDT
Argh, this caused a build failure in gfxFT2Fonts on Android.
Backed out:
http://hg.mozilla.org/mozilla-central/rev/854daee4377c
http://hg.mozilla.org/mozilla-central/rev/84bc1d07bacc
http://hg.mozilla.org/mozilla-central/rev/931f06b80727
Comment 10 Jonathan Kew (:jfkthame) 2011-07-13 07:06:54 PDT
Re-landed, with the android bustage fixed:
http://hg.mozilla.org/mozilla-central/rev/812aa60517a7
http://hg.mozilla.org/mozilla-central/rev/64ee37ef692e

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