1086207 - font load events and promise resolution do not occur in correct order

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P3)

Description

[John Daggett (:jtd)]

Attachment: testcase, FontFace.status not updated after font load

The Font Loading API allows for fonts to be created and added to the set of fonts used by a document:

  var f = new FontFace(name, fontURL);
  document.fonts.add(f);

Fonts can be explicitly loaded:

  f.load();
  assert(f.status != "unloaded");

This works but when the load happens via layout lazily loading the font, the status of the face is never updated correctly.

  document.ready.then(function() {
    assert(f.status != "unloaded");
  });

This second assertion fails with current code.

Comment 1

[John Daggett (:jtd)]

Note: layout.css.font-loading-api.enabled must be set to true to run the testcase.

Comment 2

[Cameron McCormack (:heycam)]

It does seem to be set to loaded, if I inspect its status in the Web Console, so maybe it is happening later than it should, or document.fonts.ready is resolving earlier than it should.

Comment 3

[John Daggett (:jtd)]

(In reply to Cameron McCormack (:heycam) (away October 6 - November 5) from comment #2)
> It does seem to be set to loaded, if I inspect its status in the Web
> Console, so maybe it is happening later than it should, or
> document.fonts.ready is resolving earlier than it should.

Checking the load status on a timeout does show it as loaded. But I think the underlying problem here is the weird mStatus flag in FontFace. I think that probably should *not* exist. 

Part of this may be a spec problem. Looking over the spec now, I'm not sure the relationship of 'loading' on both the FontFace and FontFaceSet to pending font loads makes sense. Effectively this requires a sync reflow whenever FontFace.status or FontFaceSet.status is checked. I think it's trying to keep the 'status' state in sync with the ready promise. I don't think this is needed.

There's no clear detail about whether reflows need to be done before FontFace.status is returned but it does say that pending loads need to be cleared before FontFaceSet.status is returned. And since the defn of FontFaceSet.status == "loaded" is "no FontFace.status is loading", that seems to require what the code says it requires.

However, either way I think FontFace.status should *always* return the state of the underlying gfxUserFontEntry.

Comment 4

[John Daggett (:jtd)]

While not entirely clear, the spec indicates that status changes to FontFace objects should take place as a condition for the ready() promise being fulfilled. Also, looking over the spec indicates that there's no need to wait for pending layout operations to complete in the case of FontFace.status (but there is for FontFaceSet.status).

Comment 5

[John Daggett (:jtd)]

Attachment: extended testcase, check status after events and promises for a single font load

Wrote an extended testcase, looks like we have some serious problems to deal with here. Very little is correct... :(

Testcase: creates a font face object with a data URL, adds it to the font set, then set up handlers to log font and font set status on both events and when promises are resolved.

Note: the ready promise on FontFaceSet in Chrome is a *function* that returns a promise rather than a promise attribute.

Chrome output (as expected):
add new font - font unloaded, set loaded
onloading event - font loading, set loading
after layout - font loading, set loading
font loaded - font loaded, set loading
onloadingdone event - font loaded, set loaded
document.fonts ready - font loaded, set loaded

Gecko trunk output (*sigh*):
onloadingdone event - font unloaded, set loaded (195x !!!)
after layout - font unloaded, set loaded
<< too much recursion >>
document.fonts ready - font unloaded, set loaded
onloading event - font loaded, set loaded
onloadingdone event - font loaded, set loaded
font loaded - font loaded, set loaded

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Do we need to fix this before shipping the font loading API?

Also, was there some other question you wanted my feedback on?

Comment 7

[John Daggett (:jtd)]

(In reply to David Baron [:dbaron] (UTC-4) (needinfo? for questions) (away/busy Oct. 24-31) from comment #6)
> Do we need to fix this before shipping the font loading API?

Yes, we need to fix this before enabling the pref. There are synchronization issues here and that will affect display if not fixed (e.g. if an loadingdone event fires before the font is actually loaded, use of a font within canvas will not display in the desired way).

> Also, was there some other question you wanted my feedback on?

I originally cc'ed you because I thought there was a spec issue (comment 3) but after reading through it a little more, I think the algorithms in the spec are correct but some of the definitions are confusing and not quite correct. They probably could be explained better but they are sufficient at this point I think.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to John Daggett (:jtd) from comment #7)
> Yes, we need to fix this before enabling the pref. There are synchronization
> issues here and that will affect display if not fixed (e.g. if an
> loadingdone event fires before the font is actually loaded, use of a font
> within canvas will not display in the desired way).

OK, I filed bug 1088437 to track enabling the API, and made this bug block it.

> I originally cc'ed you because I thought there was a spec issue (comment 3)
> but after reading through it a little more, I think the algorithms in the
> spec are correct but some of the definitions are confusing and not quite
> correct. They probably could be explained better but they are sufficient at
> this point I think.

Probably best for you to send feedback directly to www-style rather than through me, and then let me know if there are issues there that you need me bug people in the WG about if you don't get an adequate response.

Comment 9

[John Daggett (:jtd)]

(In reply to David Baron [:dbaron] (UTC-4) (needinfo? for questions) (away/busy Oct. 24-31) from comment #8)
> Probably best for you to send feedback directly to www-style rather than
> through me, and then let me know if there are issues there that you need me
> bug people in the WG about if you don't get an adequate response.

Yup, that's what I did. I think the definition of FontFaceSet.status should be revised to be more in line with what the actual algorithm says about when 'status' changes occur:

http://lists.w3.org/Archives/Public/www-style/2014Oct/0373.html

Comment 10

[Cameron McCormack (:heycam)]

There are a few different issues here so I'm going to break them off into separate bugs.

Comment 11

[Cameron McCormack (:heycam)]

Fixed by the work in dependent bugs.