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

Status: RESOLVED FIXED

(Core :: Graphics, defect, P3)

Description

[John Daggett (:jtd)]

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.

Comment 1

[John Daggett (:jtd)]

Attachment: patch, v.0.1, make downloaded font names unique across loads

Rather than use the postscript name, create a unique name and use that instead when constructing the font entry for an activated downloaded font.

Comment 2

[John Daggett (:jtd)]

Attachment: patch, v.0.2, use guid as the unique name

use guid's for the unique name, both mac/windows

Comment 3

[John Daggett (:jtd)]

Attachment: patch, v.0.2a, oops, forgot to clean out hacking code

Comment 4

[Bill Gianopoulos [:WG9s]]

(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.

Comment 5

[John Daggett (:jtd)]

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).

Comment 6

[John Daggett (:jtd)]

Attachment: patch, v.0.3a, use base64 guid names due to windows limitation

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.

Comment 7

[Bill Gianopoulos [:WG9s]]

(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.

Comment 8

[John Daggett (:jtd)]

Attachment: patch, v.0.4, add reftest for synthetic bold/italic suppression

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.

Comment 9

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

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?

Comment 10

[John Daggett (:jtd)]

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.

Comment 11

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Oh, gotcha.. yeah, that's fine.

Comment 12

[John Daggett (:jtd)]

Pushed to moz-central
http://hg.mozilla.org/mozilla-central/rev/635b1c8783a6

Comment 13

[Joe Drew (not getting mail)]

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

Comment 14

[John Daggett (:jtd)]

Relanded (reftest marked random, bug 482596)
http://hg.mozilla.org/mozilla-central/rev/db99e62ffe8f

Comment 15

[John Daggett (:jtd)]

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.

Comment 16

[John Daggett (:jtd)]

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