Bug 136502

Summary:

[CSS Font Loading] Add ready attribute to document.fonts

Product:

WebKit

Reporter:

Bear Travis <betravis>

Component:

CSS

Assignee:

Bear Travis <betravis>

Status:

RESOLVED WORKSFORME

Severity:

Normal

CC:

bfulgham, fred.wang, mmaxfield, syoichi, thorton

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

URL:

http://dev.w3.org/csswg/css-font-loading/#font-face-set-ready

Bug Depends on:

Bug Blocks:

135390, 158884

Attachments:

Description	Flags
Initial Patch	none
Updated patch	none
Updated patch	none
Updated patch	none
Fixing ready test	none

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	Details Formatted Diff Diff
Updated patch (18.87 KB, patch) 2014-11-25 11:30 PST, Bear Travis	no flags	Details Formatted Diff Diff
Updated patch (21.94 KB, patch) 2014-12-11 10:13 PST, Bear Travis	no flags	Details Formatted Diff Diff
Updated patch (24.66 KB, patch) 2014-12-15 16:01 PST, Bear Travis	no flags	Details Formatted Diff Diff
Fixing ready test (24.67 KB, patch) 2014-12-16 10:26 PST, Bear Travis	no flags	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

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.