479152 - [@font-face] need to bump generation after loading local

Status: RESOLVED FIXED

(Core :: Graphics, defect, P2)

Description

[John Daggett (:jtd)]

Attachment: testcase, local() in various fallback situations

The code in gfxUserFontSet doesn't bump the generation when loading a local face:

http://hg.mozilla.org/mozilla-central/file/a04e9fa6471e/gfx/thebes/src/gfxUserFontSet.cpp#l289

If the local() reference appears at the beginning of the src descriptor list, then there's no need, the local face will be loaded at the time of lookup and there's no need to force a reflow.  But in the case of a src descriptor list that falls back to a local face, the generation *does* need to get bumped for the reflow to pick up the newly loaded font.

In the attached testcase, the entire page should appear in some flavor of sans-serif.

Comment 1

[Karl Tomlinson (:karlt)]

(In reply to comment #0)
> But in the case of a src descriptor list that falls back to a local face,
> the generation *does* need to get bumped for the reflow to pick up the newly
> loaded font.

... falls back to a local face from a failed asynchronous (non-local) load only, i think.
i.e. not when falling back from an earlier local face that is not present.

Something else to consider:

With 'src: url(foo), local("bar")', while waiting for foo to load, instead of using the next family, should we be using "bar"?

There would then never be a reason to increment the generation with faces loaded synchronously.

Comment 2

[John Daggett (:jtd)]

Right, we only need to bump the generation when a local face is used after a load of an external font resource (i.e. src url()) fails.

The idea of falling back to local faces if present is interesting, something that probably warrants a larger discussion around the issue of what user agents should display in place of unloaded fonts (WebKit's blank text, our current fallback behavior, etc.).  I think the details get a little tricky when dealing with a src descriptor containing multiple formats that may or may not be supported.

Comment 3

[John Daggett (:jtd)]

Attachment: patch, v.0.1, bump generation on local load with reftest

Comment 4

[Karl Tomlinson (:karlt)]

(In reply to comment #3)
> Created an attachment (id=363034) [details]
> patch, v.0.1, bump generation on local load with reftest

Note this is essentially backing out the fix for bug 469752.

Comment 5

[John Daggett (:jtd)]

Attachment: patch, v.0.2, only bump generation when local follows external

Only bump the generation after LoadNext is called from within OnDownloadComplete, since this is the situation under which load behavior (i.e. mapping of font family ==> font entry) will change.

Comment 6

[Karl Tomlinson (:karlt)]

Comment on attachment 363051 [details] [diff] [review]
patch, v.0.2, only bump generation when local follows external

Yes, that makes sense.

Comment 7

[John Daggett (:jtd)]

Pushed
http://hg.mozilla.org/mozilla-central/rev/97d071b5a166

Comment 8

[John Daggett (:jtd)]

Pushed to 1.9.1
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/4cf4d88480be

Comment 9

[Boris Zbarsky [:bzbarsky]]

The test for this bug went orange today, because it was taking more than the 10s reftest load timeout to get a 404 from example.com due to network congestion the moco network.

Since this can happen any time, I've disabled the test and filed bug 481729 on the issue.