w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.49k stars 660 forks source link

[css-fonts-4] font-display says it's valid in @font-feature-values but it isn't an at-rule #2973

Open rsheeter opened 6 years ago

rsheeter commented 6 years ago

https://drafts.csswg.org/css-fonts-4/#font-feature-values-syntax appears to say only at-rules are allowed in @font-feature-values.

https://drafts.csswg.org/css-fonts-4/#font-display-font-feature-values seems to say font-display - not an at-rule - is valid in @font-feature-values.

Apologies if I'm horribly misunderstanding but ... how does this work? Is @font-feature-values Roboto { font-display: swap; } valid? If not, how do you use these together?

litherum commented 6 years ago

Yep. I'm not sure why we support putting font-display in @font-feature-values when you could instead put it in your @font-face.

rsheeter commented 6 years ago

For Google Fonts we'd actually like it to be valid outside @font-face, we're just a bit confused about the must be an at-rule bit seemingly disallowing font-display.

On Tue, Jul 31, 2018, 3:31 PM Myles C. Maxfield notifications@github.com wrote:

Yep. I'm not sure why we support putting font-display in @font-feature-values when you could instead put it in your @font-face.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/w3c/csswg-drafts/issues/2973#issuecomment-409389399, or mute the thread https://github.com/notifications/unsubscribe-auth/AGKrgLSKTh2iNYTXwZzSkAwWA-XSdnTxks5uMNrcgaJpZM4Vpcm2 .

tabatkins commented 6 years ago

It's allowed in @font-feature-values so you can apply a font-display even if you don't have control over the @font-face rules themselves - as mentioned, this is useful for the Google Fonts case.

The only actual requirement is that at-rules can't contain both style rules and declarations; since @font-feature-values doesn't contain either yet, it's fine to make it contain the font-display declaration. It just needs to be defined to do so.

litherum commented 6 years ago

if you don't have control over the @font-face rules themselves - as mentioned, this is useful for the Google Fonts

I don't understand; Google Fonts has control over the @font-face rules.

svgeesus commented 6 years ago

Yep. I'm not sure why we support putting font-display in @font-feature-values when you could instead put it in your @font-face.

We seem to allow it in both places, and I can see reasons for allowing it in both, but @rsheeter is correct that the spec is not currently self consistent regarding the content model of @font-feature-values.

I don't understand; Google Fonts has control over the @font-face rules.

Yes, but a content author who is using Google-hosted fonts (and Google-hosted @-import-ed stylesheets) doesn't. But they can add @font-feature-values to their own stylesheets.

rsheeter commented 6 years ago

To give a little context for Google Fonts, we don't know what font-display our users want so we'd have to add a param to let the user specify it. Adding a param breaks cache sharing between sites that would otherwise share. Our stats suggest that cross-site caching is currently quite effective and we'd like to keep it that way. If we were to end up with numerous parameters to tune the @font-face in various ways the cache fragmentation could be significant.

drott commented 6 years ago

Adding a param breaks cache sharing between sites that would otherwise share. Our stats suggest that cross-site caching is currently quite effective and we'd like to keep it that way. If we were to end up with numerous parameters to tune the @font-face in various ways the cache fragmentation could be significant.

@rsheeter, could you clarify the level of caching you are referring to? I am right in assuming you mean a cross-site Google Fonts server side cache for the CSS that is delivered to clients? Or browser side caching of @font-faces?

litherum commented 6 years ago

font-display has nothing to do with font features. I don't think this particular mechanism is a good fit.

lilles commented 6 years ago

Regarding syntax, at least Gecko has shipped @font-feature-values accepting <rule-list>, so they will only recognize @swash in the latter rule.

  @font-feature-values family {
    font-display: block;
    @swash { swishy: 1; flowing: 2; }
  }
  @font-feature-values family {
    dummy { }
    @swash { swishy: 1; flowing: 2; }
  }
KenjiBaheux commented 6 years ago

@litherum I see your point about font-display not being a "font feature" but the hook to the family seemed quite fitting. Any alternative suggestion?

lilles commented 6 years ago

Could a new rule for @font-face default values be an alternative?

@font-face-defaults <family-name># {
  <declaration-list>
}
lilles commented 5 years ago

Any feedback on this, or a counter-proposal?

Could a new rule for @font-face default values be an alternative?

@font-face-defaults <family-name># {
  <declaration-list>
}
KenjiBaheux commented 5 years ago

This looks reasonable to me.

litherum commented 5 years ago

Would the @font-face-defaults rule apply to all @font-family blocks that share the font name?

It’s a little unfortunate to make an entirely new at-rule just for a single descriptor. What else might we eventually want to put in that rule?

I wonder if instead of having a proliferation of different kinds of at-rules, we could instead do something like WebIDL’s “partial interfaces” or Swift’s “extensions” to allow you to re-open a previously closed block.

tabatkins commented 5 years ago

...oooh, that's a good idea. Hm.

How about this?

@partial font-face {
 ...
}

The @partial rule looks identical to a normal at-rule, just with the normal at-rule's name pushed forward as a plain ident and @partial being the name instead. It otherwise has the same syntax as the extended at-rule; additional prelude comes after the ident, etc. (So, for example, counter styles would be extended with @partial counter-style foo {...}.)

At-rules have to define that they're extensible; it's expected that every "closed" at-rule will be. They must define precisely how to match themselves against the "main" at-rule: for example, @partial counter-style is matched according to the counter style name; @partial font-face is matched according to the font-family descriptor and the various styling descriptors that are relevant for matching, so it extends all compatible @font-face rules.

All the @partial rules are collected and merged in order of appearance, as usual. They extend whichever version of the "main" rule that won the cascade (for all existing ones, this is just the last one). Relative ordering of @partial vs "main" rules is unimportant; the winning partial declarations are all ordered after the "main" declarations.

Worked out example:

@partial counter-style foo {
  prefix: "pre";
  suffix: "post";
}

@counter-style foo {
  system: cyclic;
  symbols: A B C;
}

@partial counter-style foo {
  suffix: "after";
  negative: "neg";
}

@counter-style foo {
  system: alphabetic;
  symbols: A B C;
  prefix: "before";
}

This is equivalent to:

@counter-style foo {
  system: alphabetic;
  symbols: A B C;
  prefix: "pre";
  suffix: "after";
  negative: "neg";
}

For font-face, the following would extend all faces of the "foo" family with a font-display value:

@partial font-face {
  font-family: "foo";
  font-display: optional;
}

If you wanted to only extend the bold faces, you could do:

@partial font-face {
  font-family: "foo";
  font-weight: bold;
  font-display: optional;
}

Thoughts?

litherum commented 5 years ago

I presume that if you don't have any font-family, font-weight, font-style, or font-stretch descriptors in the @partial font-face, then the contents of the @partial font-face would be applied to every @font-face (thereby letting you set a default font-display for every web font in the document). Presumably, adding any of those descriptors would filter the set of @font-face blocks the @partial applies to. (And the filtering would be done on range intersection, presumably, because you could have font-weight: 300 800;.) And any other descriptors like font-feature-settings wouldn't filter, but would instead be applied to the @font-face the same way that font-display would be applied.

I think you're right that, if we want something like @partial, at-rules will have to opt-in, and when they opt-in, they will have to specify matching rules like what I just described above for @font-face.

I'm not sure that the cascading behavior you described above is the best behavior, but I'll discuss it with people on our team who know more about that.

tabatkins commented 5 years ago

I presume that if you don't have any...

Yes, that's exactly what the last example is about.

I'm not sure that the cascading behavior you described above is the best behavior, but I'll discuss it with people on our team who know more about that.

The other possibility is that @partial is closed and last-wins too, but then you might want to write @partial partial ... ^_^

css-meeting-bot commented 5 years ago

The CSS Working Group just discussed font-display says it's valid in @font-feature-values but it isn't an at-rule.

The full IRC log of that discussion <dael> Topic: font-display says it's valid in @font-feature-values but it isn't an at-rule
<dael> github: https://github.com/w3c/csswg-drafts/issues/2973#issuecomment-442167124
<dael> TabAtkins: A while back got a requirt to override font descriptor for fonts they don't control. Specifically google fonts. Google fonts control that and due to caching don't want arbitrary things in there. Idea is add font-display to font feature values which kinda adds additional information and we can expose that
<chris> q+ to note this is very interesting but seems more CSS Syntax than CSS Fonts 4. Would also like to see the proposal more fleshed out.
<dael> TabAtkins: Right now font-feature-values doesn't take descriptors. Needs minor spec edits for that.
<dael> TabAtkins: myles objected because seems ad hoc. We want to define additional fontface, but they cascade atomically. Rather then trying to come up with special fix for this one problem we thought why not try and fix the general problem and have a way to explicitly extend these @rules
<myles> i didn't "object"
<myles> just said it would be a shame if we did it
<dael> TabAtkins: IN thread had proposal for @partial rule that looks like an @rule where partial is the first ident. Whatever is in there is matched with the appropriate @rule and override what's in it with the partial. @partialFontFace and then put the descriptor. Font family or a weight or what have you. CHanges the font display value of font faces loaded from another source
<dael> TabAtkins: Only a handful of atomic cascade now, but this lets you handle that override case
<dael> TabAtkins: Proposal in thread if we want it. Probably a new WD, doesn't fit in Fonts spec. I'm happy to edit and persue if people are interested, but want interest before more time
<dael> leaverou: How to determine which @rule it overrides?
<dbaron> q+
<dael> myles: The way this would have to work is each @rule has to opt in. By default none get a change and we opt in @rules. When it's opted in it would have to spec how the partial rules match with the base @rule. For Fonts there are 3 descriptors that would match, different for other @ rules
<dael> TabAtkins: Font family has to match and the partial has to be = or less restrictive match on style, stretch, and weight descriptors.
<chris> q?
<dael> TabAtkins: If you only want the bolds you can specifiy that, or you can allow for everything
<dael> TabAtkins: Defined by each @rule when it claims I'm allowed to be extended. Rules are ad hoc for what the @rule does
<astearns> ack chris
<Zakim> chris, you wanted to note this is very interesting but seems more CSS Syntax than CSS Fonts 4. Would also like to see the proposal more fleshed out.
<myles> q+
<dael> chris: I agree with this belonging in sep spec. Also have questions on how matching works. I understand how it works for each @rule differently, but I'd like a definition that says see each @rule for matching and there's a common term the @rules can refer to.
<fantasai> The contents of @supports already cascades
<dael> chris: Seems generally useful, but I'd like more examples and where it won't work, like I assume not on @supports
<astearns> ack dbaron
<dael> chris: I think it's great, it's really interesting
<dael> dbaron: This feels like it's a confusing mech to me. Applies to some but not other @rules. Applies to those that don't cascade and turns them into cascading @rules
<chris> kind of like inherit applies to non-inheritable propertes, which is also a bit weird :)
<dael> dbaron: I think it's confusing enough that some do that and some don't. What TabAtkins said about @font-face where it interacts differently also sounds weird and confusing
<fantasai> +1 to dbaron
<leaverou> +1 to dbaron as well
<dael> dbaron: I feel it might be better handled as extensions to each @rule that needs this mech. Feels like avariant, not a new @partial rule
<dael> TabAtkins: THat's what it is, I'm just unifying it under one name to be recognizable. THe stuff after the @partial is what that @rule wants to do its extension grammar with
<dael> chris: I think as unified as poss on extension mech is better than duplicating half the @rules
<astearns> ack myles
<dael> myles: That's the direction I was going to mention
<dael> myles: 2 pieces, one to open a closed blocka nd add stuff. Then there's the specific idea of the fonts problem where different people want to change different parts.
<dael> myles: Such a general solution would solve this use case. I don't think this use case is suffiecnt for this complicated thing. If there are other use cases the combination of them could motivate.
<dael> astearns: AS long as solutions are generalizable. I'd like to see other rules we'd want to open and see if enough similar. I'm a little scared with each rule can define how overwritten with partial rule
<dael> TabAtkins: All reasonable.
<dael> TabAtkins: The particular use case for fonts is strong. I'd like to address that. If we're not going for general until we find more things, I'd push to solvet his use case
<dael> myles: Rather then giving up I think somebody should do research to see if there are use cases. We're asking the question, not answering it
<dael> astearns: I think general solution merits a bit more work. WIll need non-font use cases so we can evaluate general solution outside this thing that needs to be solved. Agreed we need to solve this and we should take time to see if general solution is the way to do. More specific @rules is a rats nest we've added to for decades and normalizing would be good
<dael> fantasai: I don't think a generic rule is normalizing anything. Agree with dbaron that if we're going to do it...we might decide on a specific syntax, but the @rule should be named after its @rule and not be generic
<myles> @partial font-face @partial-font-face @font-face-partial
<myles> @font-partial-face ????
<dael> TabAtkins: I don't care about @parial fontface or @fontface.partial I jsut want to see partials as a thing
<myles> @key-partial-frames
<dael> TabAtkins: Let's look at use cases, see if we can find decent ones for other cases of extension. Maybe there's a good reason to extend something like keyframes.
<dael> astearns: Alright. Sound good to everyone?
<dael> astearns: Alright, thanks.
LeaVerou commented 5 years ago

(typed my comment too slowly to be recorded in the minutes, so I'm reposting here)

Besides concerns about use cases, I'm also a bit concerned that we are introducing a mechanism to override that is so different than the cascade and completely breaks with reordering. It's two different mental models for doing very similar things, which is a usability antipattern. I was wondering if we can somehow "match" the defined @font-face (or other) rule and override it in a more cascade-like manner.

superpoincare commented 5 years ago

Reposting https://github.com/google/fonts/issues/358#issuecomment-463322909 here:

Hi all,

I want to take this opportunity to point to a potential issue with @font-feature-values

I came across this tweet:

https://twitter.com/pjchrobak/status/1059229076987301888

It seems removing semicolon in font-display in @font-feature-values breaks Firefox CSS below that for older versions.

Keep in mind, that if you add @font-feature-values FontName { font-display: whatever; } without semicolon after value (what minifiers do), older versions of Firefox stop render whole CSS after this descriptor 🤯

I tested this on Browserstack with Firefox 56 in MacOS Mojave and the tweet seems right!

image

Firefox 57 onward fine but this raises questions about the syntax.

LeaVerou commented 5 years ago

Firefox 57 onward fine but this raises questions about the syntax.

How so? This sounds like a browser bug to me, and a fixed one at that.

superpoincare commented 5 years ago

Firefox has ESR versions for organisations and 60 is the latest but organisations sometimes take time to upgrade and 52 may also be in use.

svgeesus commented 5 years ago

Thanks for the concern, but I think changing the entire CSS descriptor syntax to accommodate a bug in Firefox 56- (in case people are still using Firefox 52 long term support version) is not warranted.

Thanks for pushing on the Google Fonts people to support font-display !

(Posted using Firefox 67a1)

rsheeter commented 5 years ago

In chatting with @litherum face to face we realized we never mentioned user impact of this ticket hanging open. So, better late than never: we delayed shipping font-display for Google Fonts for quite some time waiting for this to be resolved one way or another.

It would be helpful to clarify whether setting font-display outside @font-face in some fashion is ever going to be supported or to close this as Won't Fix if it just isn't going to happen.

svgeesus commented 5 years ago

Hi @rsheeter

So is this

Today, in the "Speed at Scale" talk at Google I/O we announced that font-display will soon be shipping to Google Fonts in the form of a query parameter.

putting it inside an at-rule?

superpoincare commented 5 years ago

@svgeesus

It's already live!

Check: https://fonts.googleapis.com/css?family=Open+Sans&display=swap

svgeesus commented 3 years ago

Sooo this long-reported bug is still open. As of today the spec says:

@font-feature-values <family-name># { <rule-list> }

<feature-value-block> = <font-feature-value-type> { <declaration-list> }

<font-feature-value-type> = @stylistic | @historical-forms | @styleset | @character-variant
  | @swash | @ornaments | @annotation

which means that the font-display descriptor for @font-feature-values can't actually be used thee. (The same descriptor can, of course, be used in @font-face.

To fix this, like @tabatkins said:

The only actual requirement is that at-rules can't contain both style rules and declarations; since @font-feature-values doesn't contain either yet, it's fine to make it contain the font-display declaration. It just needs to be defined to do so.

@font-feature-values <family-name># { <rule-list>  |  font-display }

<feature-value-block> = <font-feature-value-type> { <declaration-list> }

<font-feature-value-type> = @stylistic | @historical-forms | @styleset | @character-variant
  | @swash | @ornaments | @annotation

but also,

An @font-feature-values rule’s prelude contains a list of font family names, followed by a block containing multiple <feature-value-block>s, a special type of subsidiary at-rule. Each <feature-value-block> contains declarations mapping author-chosen human-friendly names (such as "flowing") to feature indexes for the associated feature.

to

An @font-feature-values rule’s prelude contains a list of font family names, followed by a block containing zero or more optional multiple <feature-value-block>s, (a special type of subsidiary at-rule) and an optional font-display descriptor

Each <feature-value-block> contains declarations mapping author-chosen human-friendly names (such as "flowing") to feature indexes for the associated feature.

Right? Plus some examples, like the one @lilles posted

@rsheeter @litherum sound good?

Edit: OK no. CSS Syntax 3 says

Similarly, the production represents a list of rules, and may only be used in grammars as the sole value in a block. It represents that the contents of the block must be parsed using the consume a list of rules algorithm.

svgeesus commented 3 years ago

It's already live!

Check: https://fonts.googleapis.com/css?family=Open+Sans&display=swap

Right, but that is putting it inside @font-face which was already clearly allowed.

svgeesus commented 3 years ago

@tabatkins what is the current status on relaxing mixing at-rules and declarations? We already decided to allow this right, to nest media queries inside rules? Or was it feature queries? Should I wait until Syntax 3 is fixed?

tabatkins commented 3 years ago

There's nothing wrong with it on the Syntax side; you can't mix declarations with style rules (except with special circumstances, like Nesting), but you can mix either with at-rules. (You'll want to switch the grammar to <declaration-list>, is all.)

The OM needs to make sure it exposes both .style and .childRules or whatever.

yisibl commented 2 weeks ago

I think allowing font-display still makes a lot of sense. Not all font services support customized font-display like Google fonts.

e.g. https://rsms.me/inter/inter.css

This should become a general capability of CSS.

@lilles Font-display is currently allowed in the specification, but Chrome has flagged it as Won't Fix, could you consider re-enabling it?

jfkthame commented 2 weeks ago

As Myles pointed out, font-display has nothing to do with font features. It doesn't belong in @font-feature-values, IMO, and we should remove that from the specification.

(I get that authors might want to override the font-display of resources for which they don't have direct control of the @font-face rule. But @font-feature-values is not the place to do it. Something more general like Tab's @partial would be preferable.)

drott commented 2 weeks ago

+1 to what @jfkthame said.

svgeesus commented 2 weeks ago

So the proposal is to remove the whole of 4.9.1. Controlling Font Display Per Font-Family via @font-feature-values ?

jfkthame commented 2 weeks ago

So the proposal is to remove the whole of 4.9.1. Controlling Font Display Per Font-Family via @font-feature-values ?

I'd be in favor of that, yes. (6.9.1. Basic syntax also has a mention of it that should be removed at the same time.)

(Has anyone actually shipped support for this?)