RESOLVED CONFIGURATION CHANGED 225790
document.fonts.ready is sometimes still resolved too quickly
https://bugs.webkit.org/show_bug.cgi?id=225790
Summary document.fonts.ready is sometimes still resolved too quickly
Cameron McCormack (:heycam)
Reported 2021-05-13 16:50:12 PDT
Here is a test: https://mcc.id.au/2021/font-load.html If I open this in a debug MiniBrowser, I get this output: FontFaceSet.status in onload: loaded FontFace.status in onload: unloaded FontFaceSet.status after ready: loading FontFace.status after ready: loading So the ready promise was initially resolved, and then at some point after the document.fonts.ready.then() call, it was replaced with a fresh, unresolved promise, so that once we're in the promise handler, the font has started loading. In a regular Safari, I occasionally get this output, but usually I get this (more expected) output: FontFaceSet.status in onload: loaded FontFace.status in onload: unloaded FontFaceSet.status after ready: loaded FontFace.status after ready: loaded Firefox and Chrome both delay the load event for font loads from style sheets, and so we get different output still. Firefox: FontFaceSet.status in onload: loaded FontFace.status in onload: loaded FontFaceSet.status after ready: loaded FontFace.status after ready: loaded Chrome: FontFaceSet.status in onload: loading FontFace.status in onload: loaded FontFaceSet.status after ready: loaded FontFace.status after ready: loaded (Chrome's makes a little less sense to me, with the FontFaceSet still loading but the only FontFace in it loaded.) The upshot of this is that if relying on font loads to have been triggered by content already in the document by the time of the load event, document.fonts.ready.then() is not sufficient in Safari to wait for them. I'm working around this in bug 225728 for some MathML tests. I'm not even sure though that WebKit's behavior here violates the spec. But it probably is a compat problem.
Attachments
Ryosuke Niwa
Comment 1 2021-05-13 17:05:43 PDT
Maybe the issue here is that CSSFontFace::m_status is initialized to Status::Loaded?
Ryosuke Niwa
Comment 2 2021-05-13 17:08:14 PDT
Oh, this is about document.fonts.
Ryosuke Niwa
Comment 3 2021-05-13 17:12:03 PDT
(In reply to Ryosuke Niwa from comment #1) > Maybe the issue here is that CSSFontFace::m_status is initialized to > Status::Loaded? I think this is still the issue. In particular, we rely on style resolution to find fonts in use. So if there was no style resolution by the time Document::implicitClose() is called, then we may fail to detect any fonts available in the document. The reason this is flaky in Safari is probably because Safari injects scripts to run the reader availability and other things so it's probably triggering a style resolution before load event is fired in some cases.
Ryosuke Niwa
Comment 4 2021-05-13 17:14:03 PDT
Hm... we do explicitly trigger a style resolution here: Ref<FontFaceSet> Document::fonts() { updateStyleIfNeeded(); return fontSelector().fontFaceSet(); }
Cameron McCormack (:heycam)
Comment 5 2021-05-13 17:16:41 PDT
Do we need to flush layout as well? Triggering font loads should be based on the specific characters used in the content.
Cameron McCormack (:heycam)
Comment 6 2021-05-13 17:17:40 PDT
Still, flushing here doesn't 100% solve the problem, since script could in theory do something like: let fonts = document.fonts; document.write("<p>some content that would trigger a font load</p>"); onload = function() { fonts.ready.then(...) };
Ryosuke Niwa
Comment 7 2021-05-13 17:28:31 PDT
(In reply to Cameron McCormack (:heycam) from comment #6) > Still, flushing here doesn't 100% solve the problem, since script could in > theory do something like: > > let fonts = document.fonts; > document.write("<p>some content that would trigger a font load</p>"); > onload = function() { > fonts.ready.then(...) > }; Right. It looks like the current spec seems to suggest that any @font-face rule which appears in a CSS will automatically adds a new entry to FontFaceSet of the document: https://drafts.csswg.org/css-font-loading/#font-face-css-connection This might be problematic given that this will end up loading all fonts regardless of whether they're used or not. It still makes it ambiguous as to when @font-face rule will be added to FontFaceSet and what "status" should say in the intervening period. Namely, if all @font-face rules are defined in an external CSS resource, then there is no CSS-connected FontFace object at the time of accessing document.fonts above, which means that document.fonts.status will say loaded. Only when the stylesheet gets loaded, then do we find @font-face rules and therefore adds more entries to the font source. Given that a new stylesheet content might be loaded at any moment while the document in question is loading, I don't think there is any reason to believe document.fonts.status will return a reliable information until load event is fired on the global object.
Cameron McCormack (:heycam)
Comment 8 2021-05-13 17:34:12 PDT
(In reply to Ryosuke Niwa from comment #7) > Right. It looks like the current spec seems to suggest that any @font-face > rule which appears in a CSS will automatically adds a new entry to > FontFaceSet of the document: > https://drafts.csswg.org/css-font-loading/#font-face-css-connection > > This might be problematic given that this will end up loading all fonts > regardless of whether they're used or not. I don't think that's what would happen. CSS connected FontFace objects should only start loading when the UA decides it needs them per https://drafts.csswg.org/css-font-loading/#font-face-set-css. > It still makes it ambiguous as to when @font-face rule will be added to > FontFaceSet and what "status" should say in the intervening period. Namely, > if all @font-face rules are defined in an external CSS resource, then there > is no CSS-connected FontFace object at the time of accessing document.fonts > above, which means that document.fonts.status will say loaded. Only when the > stylesheet gets loaded, then do we find @font-face rules and therefore adds > more entries to the font source. I agree it's underdefined when exactly the FontFace object should start loading. But I think, implicitly (like all other APIs), inspecting the contents of the FontFaceSet should reflect all style sheets currently loaded (which means inspecting the FontFaceSet should flush style sheets). The loading of the external style sheets in my test should block the load event, so by the time we inspect document.fonts I think it's guaranteed that the FontFace is in there (even if it's not clear what its loading state should be). > Given that a new stylesheet content might be loaded at any moment while the > document in question is loading, I don't think there is any reason to > believe document.fonts.status will return a reliable information until load > event is fired on the global object. Agreed.
Cameron McCormack (:heycam)
Comment 9 2021-05-13 17:41:52 PDT
(In reply to Ryosuke Niwa from comment #4) > Hm... we do explicitly trigger a style resolution here: > > Ref<FontFaceSet> Document::fonts() > { > updateStyleIfNeeded(); I tried replacing this with updateLayout(), but it still didn't help my testcase. I got this output instead: FontFaceSet.status in onload: loading FontFace.status in onload: loading FontFaceSet.status after ready: loading FontFace.status after ready: loading which is even more confusing, since if FontFaceSet.status in onload is loading, then we should already have done the ready state promise replacement, and for this test, that replacement should only happen once.
Cameron McCormack (:heycam)
Comment 10 2021-05-13 18:21:31 PDT
I might be missing something but I can't find any code that does the replacement of FontFaceSet::m_readyPromise when we switch from FontFaceSet state "loaded" to "loading" or any of the requirements to wait for resolving when "pending on the environment". This latter problem is what prevents the layout flush from making the testcase work.
Ryosuke Niwa
Comment 11 2021-05-13 20:00:28 PDT
(In reply to Cameron McCormack (:heycam) from comment #10) > I might be missing something but I can't find any code that does the > replacement of FontFaceSet::m_readyPromise when we switch from FontFaceSet > state "loaded" to "loading" or any of the requirements to wait for resolving > when "pending on the environment". This latter problem is what prevents the > layout flush from making the testcase work. That is indeed a problem although it's also strange that FontFaceSet.prototype.promise can end up returning different objects each time it is accessed.
Radar WebKit Bug Importer
Comment 12 2021-05-20 16:51:18 PDT
Ahmad Saleem
Comment 13 2022-07-24 09:28:04 PDT
Based on the test case from Comment 0, I am getting following results across all browsers: NOTE - All testing performed within Private / Incognito Mode with Multiple refresh to trigger unloaded issue. *** Safari 15.6 on macOS 12.5 **** FontFaceSet.status in onload: loaded FontFace.status in onload: loaded FontFaceSet.status after ready: loaded FontFace.status after ready: loaded *** Chrome Canary 106 **** FontFaceSet.status in onload: loading FontFace.status in onload: loaded FontFaceSet.status after ready: loaded FontFace.status after ready: loaded *** Firefox Nightly 104 **** FontFaceSet.status in onload: loading FontFace.status in onload: loaded FontFaceSet.status after ready: loaded FontFace.status after ready: loaded Just wanted to share updated results. I am not sure whether something change in web-spec or when Firefox decided to follow Chrome for FontFaceSet but this is as of today updated results. Thanks!
Ryosuke Niwa
Comment 14 2022-07-27 21:06:51 PDT
This appears to be config changed now that all of them say "loaded"
Note You need to log in before you can comment on or make changes to this bug.