Re: [font-load-events] comments on new promises-based draft

On Mon, Sep 9, 2013 at 1:30 AM, John Daggett <jdaggett@mozilla.com> wrote:
> Overall, looks good, I think the new draft does a nice job of using promises and solves the problem of how to load fonts within worker threads.
>
> A few problems/issues:
>
> 1. duplication of CSSFontFaceRule struct
>
> The current definition of the FontFace interface unnecessarily duplicates that of the CSSFontFaceRule interface.  Better to define an interface shared by both:
>
>   [NoInterfaceObject]
>   interface FontFaceInfoStuff {
>     attribute DOMString family;
>     attribute DOMString src;
>     attribute DOMString style;
>     attribute DOMString weight;
>     attribute DOMString stretch;
>     attribute DOMString unicodeRange;
>     attribute DOMString variant;
>     attribute DOMString featureSettings;
>   }
>
>   CSSFontFaceRule implements FontFaceInfoStuff;
>   FontFace implements FontFaceInfoStuff;
>
> Note: I added 'src' here intentionally.
>
> The CSS3 Fonts draft will need to be updated but since this is strictly an editorial change and has no impact on the behavior, I think we can make this change without requiring another LC period.

I'm fine with this, but why have 'src'?  I don't use 'src' for the
descriptor data in FontFace - it's a separate argument instead, so I
can more easily be consistent with the BinaryData case.

> 2. side effects of 'new FontFace'
>    a. adds FontFace the FontFaceSet object?
>    b. adds a new CSSFontFaceRule when on main/Document thread?

The spec doesn't define any additional side-effects for "new
FontFace()", so nothing additional happens.  Would you like an
explicit note that nothing additional occurs?

> The spec really needs to state more clearly both what happens with a new @font-face rule is created and what happens when a new FontFace object is created.  It implies that the two are linked ("CSS-connected") but the exact association isn't clearly spelled out.

Ooh, you're right.  I handle the case where you add a new stylesheet
that contains @font-face rules, but not the case where you just add a
new @font-face to an existing stylesheet.  I'll take care of this.

> Creating independent FontFace objects doesn't really make sense.  They are part of a set of faces and font matching is done against a set of faces.  So a given FontFace needs to be part of a FontFaceSet, which implies you want to use a factory method instead of a constructor.  I realize the general trend is away from factory methods but given that @font-face fonts are loaded at use time and not when defined, I think a factory method makes more sense in this case.  The factory method would simply add all new fonts to the FontFaceSet.

FontFace objects can be individually constructed and then handed out
to workers, even if they're never explicitly used in the document.

FontFace objects don't automatically load their resources upon
construction - if they're url-based, you have to call .load() on them
(or put them in the font source and then have them get automatically
requested).  So there's no need for the awkwardness of a factory
function.

If you want to add a new font for the document to use, just do:

    document.fonts.add(new FontFace("foo", "url(bar)");

> I'm assuming you intended that for the main thread/Document case that creating a FontFace object would also create a CSSFontFaceRule, right?  If so, I think the DOMString or BinaryData parameter to the FontFace constructor needs to be reflected in the value of CSSFontFaceRule.src.  I think it's simply enough to say that if BinaryData is passed to the constructor that the 'src' descriptor is set to the data URL form of that data.

No, no such effects are documented, and no such effects are intended.
Part of the reason the source of FontFace objects is "hidden" (in an
internal attribute, rather than exposed to authors) is so that we can
GC the ArrayBuffer after we load the font from it, if there's nothing
else holding onto it.  No reason to keep around a file that might be a
MB in size just to reflect with it!  (And with a data url, we'd be
keeping around something about 1/3 larger!)

> 3. order of FontFace objects is important
>
> The order of two @font-face rules defines the load order:
>
>   @font-face { font-family: fontA; src: url(foo.woff); }
>   @font-face { font-family: fontA; src: url(bar.woff); }
>
> This implies that bar.woff is loaded first, followed by foo.woff if the character matched isn't supported by bar.woff (see step 5 in the font matching algorithm).
>
> So I don't think 'SetClass(FontFace)' for FontFaceSet works unless the order of creation is preserved.  This sort of leads to the next issue...

Order is already defined for FontFaceSet - all css-connected FontFaces
come first, in CSS order, then all the manually-constructed ones, in
insertion order.  This needs to interact more closely with Fonts'
loading algorithm, so that it uses the FontFaceSet order.

> 4. need ability to add/remove/change order of FontFace's in a FontFaceSet?
>
> Not sure this is really needed frankly, for the same reason that I'm not sure we need CSS OM API's to explicitly add/remove @font-face rules.

By virtue of me not overriding the add and delete methods, you can add
and remove faces from the FontFaceSet.  Reordering can be partially
done via delete()/add() calls, which is inconvenient, but as you say,
I don't think it's necessary.

> And, as I mentioned previously, those dang section symbols with the skanky, transitioned yellow background are distracting/ugly/heinous as all heck.  A pox on them all!  Use a 'contextmenu' attribute 'Copy link' instead.

? The section symbols don't have a yellow background unless you hover
them, because that's the behavior of *all* links.  Anyway, let's
discuss this part over drinks at the f2f. ^_^

~TJ

Received on Monday, 9 September 2013 17:05:25 UTC