RESOLVED WORKSFORME 136502
[CSS Font Loading] Add ready attribute to document.fonts
https://bugs.webkit.org/show_bug.cgi?id=136502
Summary [CSS Font Loading] Add ready attribute to document.fonts
Bear Travis
Reported 2014-09-03 14:40:43 PDT
document.fonts.ready should result in a promise that resolves when the document finishes loading fonts.
Attachments
Initial Patch (14.50 KB, patch)
2014-11-24 15:38 PST, Bear Travis
no flags
Updated patch (18.87 KB, patch)
2014-11-25 11:30 PST, Bear Travis
no flags
Updated patch (21.94 KB, patch)
2014-12-11 10:13 PST, Bear Travis
no flags
Updated patch (24.66 KB, patch)
2014-12-15 16:01 PST, Bear Travis
no flags
Fixing ready test (24.67 KB, patch)
2014-12-16 10:26 PST, Bear Travis
no flags
Bear Travis
Comment 1 2014-11-24 15:38:22 PST
Created attachment 242179 [details] Initial Patch
Bear Travis
Comment 2 2014-11-25 11:30:56 PST
Created attachment 242206 [details] Updated patch
Sam Weinig
Comment 3 2014-12-03 15:41:04 PST
Comment on attachment 242206 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=242206&action=review > Source/WebCore/css/FontLoader.h:120 > + OwnPtr<DeferredWrapper> m_ready; We have so far been able to keep DeferredWrapper in the bindings layer only, and I would prefer to keep it that way. Instead, what I recommend is having two std::functions (one for success, one for failure) that capture the DeferredWrapper. See JSSubtleCrypto::generateKey() for an example of how we do this.
Bear Travis
Comment 4 2014-12-11 10:13:13 PST
Created attachment 243130 [details] Updated patch
Sam Weinig
Comment 5 2014-12-11 12:36:18 PST
Comment on attachment 243130 [details] Updated patch Looks good. I would like to see a test that calls document.fonts.ready multiple times. I am not clear whether ready should return the same promise each time it is called, but it should be tested either way (and we should verify with the spec author what the intent is).
Bear Travis
Comment 6 2014-12-11 14:02:27 PST
I can add a test containing multiple ready reads. I'm not clear on when/if the ready attribute must return the same value on read, and have a question out to the style list. Storing the promise without sharing DeferredWrapper with FontLoader seemed a bit tricky to me, as I couldn't find an example of code that added a member variable to the JS layer like JSFontLoaderCustom.h would need to. Would that be the right way, and is it possible?
Bear Travis
Comment 7 2014-12-15 16:01:31 PST
Created attachment 243326 [details] Updated patch document.fonts.ready should return the same promise until it has been fulfilled and a font begins loading, at which point document.fonts.ready should return a new, unfulfilled promise. See http://dev.w3.org/csswg/css-font-loading/#FontFaceSet-events
Bear Travis
Comment 8 2014-12-16 10:26:42 PST
Created attachment 243366 [details] Fixing ready test
Brent Fulgham
Comment 9 2016-03-10 11:25:21 PST
Sam, it looks like he added the test you asked for. Was there anything else you wanted to see here? Bear: Could you please upload a rebaselined patch we can apply to the tree?
Myles C. Maxfield
Comment 10 2016-03-10 13:53:27 PST
(In reply to comment #9) > Sam, it looks like he added the test you asked for. Was there anything else > you wanted to see here? > > Bear: Could you please upload a rebaselined patch we can apply to the tree? This is done already. (since r196747) Also, IIRC Bear doesn't work on WebKit anymore.
Note You need to log in before you can comment on or make changes to this bug.