1434698 - font-feature-settings with integers can be serialized without an integer

Status: RESOLVED INVALID

(Core :: CSS Parsing and Computation, defect)

Description

[Chris Nardi]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.31 Safari/537.36

Steps to reproduce:

See https://w3c-test.org/css/css-fonts/font-feature-settings-serialization-001.html.


Actual results:

'font-feature-settings: "vert" 1' serializes to '"vert"'.


Expected results:

The spec (https://drafts.csswg.org/cssom/#css-rules) doesn't specify any specific rules for serializing font-feature-settings, so I would assume that the integer should be serialized always to an integer, i.e. the above case should serialize to '"vert" 1'. This would match the behavior of WebKit/Chrome/Edge (WebKit/Chrome both have changes landed in trunk to use double-quotes instead of single-quotes to match Firefox/Edge).

Comment 1

[Daniel Holbert [:dholbert]]

It looks like we're omitting the 1 because it's the default (so the values '"vert" 1' and '"vert"' are functionally equivalent).

Chrome isn't actually any better here -- they just happen to have chosen a different "standard" serialization. They serialize all three of three equivalent styles to the same thing:
 (1) font-feature-settings: "vert";
 (2) font-feature-settings: "vert" on;
 (3) font-feature-settings: "vert" 1;
They serialize all three as the third option.  Is there any spec support?

We do the same as them (serializing all three to a single thing), though we settled on the shorter of the options.

Comment 2

[Daniel Holbert [:dholbert]]

>Is there any spec support?
(...for that choice of Chrome's)

Sounds like we need spec clarification here, anyway. Chris, perhaps you could bring this up with the CSSWG?  (Looks like you're a chromium contributor, too -- do you know why Chromium chose to serialize all three different "spellings" of this value to that third option, when there's a shorter syntax available?)

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Per https://github.com/w3c/csswg-drafts/issues/1564 I suspect Firefox's serialization is the right one.

Comment 4

[Daniel Holbert [:dholbert]]

Yeah - this seems like it's same as https://github.com/w3c/csswg-drafts/issues/2229 except applied to a different CSS property.

Comment 5

[Daniel Holbert [:dholbert]]

(which itself is a special case of the issue that Emilio linked to)

So I think this is technically "invalid" as a Firefox bug (i.e. Firefox's behavior is correct per spec, to the extent that this is specced or intended-to-be-specced in the issue that Emilio linked to).

Chris, let us know if we're misunderstanding, though.

Comment 6

[Chris Nardi]

Yeah, it looks like you're correct in that the shortest serialization rule should apply there. That's probably just an oversight in the original WebKit implementation, which survives in Chrome today.

https://jsfiddle.net/f956aoLm/ demonstrates another serialization difference between Chrome/Safari/Edge and Firefox, in that font-feature-settings: "vert" 0; is serialized to '"vert" off'. Chrome/Safari/Edge serializes this to '"vert" 0'". I'm not sure which is correct per the spec.

Comment 7

[Daniel Holbert [:dholbert]]

(In reply to Chris Nardi from comment #6)
> Yeah, it looks like you're correct in that the shortest serialization rule
> should apply there. That's probably just an oversight in the original WebKit
> implementation, which survives in Chrome today.

OK, thanks for confirming.

> https://jsfiddle.net/f956aoLm/ demonstrates another serialization difference
> between Chrome/Safari/Edge and Firefox, in that font-feature-settings: "vert" 0;
> is serialized to '"vert" off'. Chrome/Safari/Edge serializes this
> to '"vert" 0'". I'm not sure which is correct per the spec.

Thanks! I think Firefox is wrong there. '"vert" 0' is clearly shorter than '"vert" off' (and would also be more consistent with our behavior for on/1 serialization, too).

I'll spin off a new bug for that.

Comment 8

[Daniel Holbert [:dholbert]]

I spun off bug 1434724 for the "off" vs "0" issue. Thanks!