Closed Bug 480267 Opened 12 years ago Closed 12 years ago

[@font-face] need to make font name unique across all documents

Categories

(Core :: Graphics, defect, P3)

x86
macOS
defect

Tracking

()

RESOLVED FIXED

People

(Reporter: jtd, Assigned: jtd)

References

()

Details

(Keywords: fixed1.9.1)

Attachments

(1 file, 4 obsolete files)

Based on the test case for bug 468387, Mac rendering of downloadable fonts can be affected by previously loaded fonts.  When creating a platform font the code in gfxQuartzFontCache::MakePlatformFont currently uses the postscript name for the font in the font entry, which in turn is used as part of the key for the font cache.  So it really needs to be unique across documents and instantiations within the same document.

Steps:

1. Start browser
2. Click on URL

Result: synthetic italics/bolding don't always show correctly for regular weight fonts, due to the font being cached with different style characteristics. [see attachment 364060 [details]]

Fix is to use a generated unique name, as on Windows.  This will eliminate the chance of a font cache collision.  Note that the postscript name is not used as a key since the ATSFontRef/ATSUFontID is already initialized.
Rather than use the postscript name, create a unique name and use that instead when constructing the font entry for an activated downloaded font.
Attachment #364263 - Flags: review?(vladimir)
use guid's for the unique name, both mac/windows
Attachment #364263 - Attachment is obsolete: true
Attachment #364276 - Flags: review?(vladimir)
Attachment #364263 - Flags: review?(vladimir)
Attachment #364276 - Attachment is obsolete: true
Attachment #364279 - Flags: review?(vladimir)
Attachment #364276 - Flags: review?(vladimir)
Priority: -- → P3
(In reply to comment #3)
> Created an attachment (id=364279) [details]
> patch, v.0.2a, oops, forgot to clean out hacking code

This patch breaks the ability to use .otf fonts via @font-face under Windows.
Attachment #364279 - Flags: review?(vladimir)
Comment on attachment 364279 [details] [diff] [review]
patch, v.0.2a, oops, forgot to clean out hacking code

Ok, sounds like the guid format needs to be massaged into a valid Postscript name (i.e. not curly braces).
Lordy I do love Windows.  The GDI API AddFontMemResourceEx refuses to load fonts with names longer than 30 characters.  To workaround this use "uf" + (guid in base64 form) as the name.  With this change, both .ttf and .otf fonts load under Windows.
Attachment #364279 - Attachment is obsolete: true
(In reply to comment #6)
> Created an attachment (id=366260) [details]
> patch, v.0.3a, use base64 guid names due to windows limitation

Works great with all of my tests.
reftest compares a table that defines font faces for various weights/styles using a regular face against a simple table using only the regular face.
Attachment #366260 - Attachment is obsolete: true
Attachment #366480 - Flags: review?(vladimir)
Comment on attachment 366480 [details] [diff] [review]
patch, v.0.4, add reftest for synthetic bold/italic suppression

Looks fine, though I was slightly confused my MAX_B64_LEN being 32 (given that the earlier magic numbers mentioned were 31 and 30).  Should that be 31?
Attachment #366480 - Flags: review?(vladimir) → review+
The buffer needs to be long enough to hold the base64 encoded 16 bytes of the nsID.  Since this will be (4/3) * 16 bytes, I just bumped it up to 32 bytes and made sure to assert on the size of nsID.  So MAX_B64_LEN doesn't relate directly to the size restriction on Windows, although 30 would certainly work.
Pushed to moz-central
http://hg.mozilla.org/mozilla-central/rev/635b1c8783a6
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
I backed this out because it seemed to cause a failure in reftests on OS X: 

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/buildbot/moz2-slave/mozilla-central-macosx-unittest/build/layout/reftests/text/475092-pos.html

http://hg.mozilla.org/mozilla-central/rev/14bc4dbf10cb
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Relanded (reftest marked random, bug 482596)
http://hg.mozilla.org/mozilla-central/rev/db99e62ffe8f
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
Comment on attachment 366480 [details] [diff] [review]
patch, v.0.4, add reftest for synthetic bold/italic suppression

important fix needed to fix caching problems on both mac/windows with downloadable fonts.  reftest included.
Attachment #366480 - Flags: approval1.9.1?
Attachment #366480 - Flags: approval1.9.1? → approval1.9.1+
pushed to 1.9.1
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/6c3ef904b399

Because of bug 486242 (only on 1.9.1 branch), marked reftest as random
Keywords: fixed1.9.1
Depends on: 486242
No longer blocks: 561021
You need to log in before you can comment on or make changes to this bug.