1823475 - WPT css/css-masking/parsing/clip-path-valid.html fails in Firefox due to us serializing specified style with position keywords and the test incorrectly expecting percentages

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Daniel Holbert [:dholbert]]

This WPT test (in the interop-2023 set) has some failures in Firefox:
https://wpt.fyi/results/css/css-masking/parsing/clip-path-valid.html
http://wpt.live/css/css-masking/parsing/clip-path-valid.html

The failures are for things like:

e.style['clip-path'] = "circle()" should set the property value
assert_equals: serialization should be canonical
expected "circle(at 50% 50%)" but got "circle(at center center)"

We're using keywords (center) in the computed style, while the test is expecting 50%.

I think this is a valid bug (i.e. the test's expectations are correct and we're wrong), per spec.

Spec references:

• https://drafts.csswg.org/css-shapes-1/#funcdef-basic-shape-circle says the circle's position is a <position> (which defaults to center if omitted)
• https://drafts.csswg.org/css-values-4/#typedef-position defines <position> and includes a note to say what happens in the computed value:

Note: Computed values are always serialized as two offsets (without keywords) because the computed value does not preserve syntactic distinctions.

(From context, the spec is using "offsets" here to mean <length-percentage>, as opposed to a keyword.)

Comment 1

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #0)

The failures are for things like:
[...]
We're using keywords (center) in the computed style, while the test is expecting 50%.

There's one other failure in that test that fails for an ordering reason -- and in that case, I think we're correct and the test is wrong.

The test failure looks like:

e.style['clip-path'] = "border-box circle(7% at 8% 9%)" should set the property value
assert_equals: serialization should be canonical
expected "border-box circle(7% at 8% 9%)" but got "circle(7% at 8% 9%) border-box"

We're serializing the computed style with border-box at the end, but the test is expecting it at the start.
It's correct for it to go at the end (after the shape); it's the <geometry-box> component in the property's "Values" definition here:
<clip-source> | [ <basic-shape> || <geometry-box> ] | none
from https://drafts.fxtf.org/css-masking-1/#the-clip-path

We're supposed to use that order for serialization in the computed style, per https://drafts.csswg.org/cssom/#serialize-a-css-value ("reorder the component values to use the canonical order of component values as given in the property definition table").

Comment 2

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #0)

We're using keywords (center) in the computed style, while the test is expecting 50%.

Oh wait, sorry -- this test is actually checking the specified style, not the computed style. And the spec says that the specified style should preserve keywords ("Keywords are serialized as keywords"), per https://drafts.csswg.org/css-values-4/#position-serialization

So this is a test bug.

Comment 3

[Daniel Holbert [:dholbert]]

In particular, the test reads the specified style like so:

        div.style[property] = value;
        var readValue = div.style.getPropertyValue(property);
[...]
            assert_equals(readValue, serializedValue, "serialization should be canonical");

So yeah, this is specified-style, and Firefox seems to be matching the spec, and the test is wrong.

Comment 4

[Jonathan Kew [:jfkthame]]

(In reply to Daniel Holbert [:dholbert] from comment #0)

This WPT test (in the interop-2023 set) has some failures in Firefox:
https://wpt.fyi/results/css/css-masking/parsing/clip-path-valid.html
http://wpt.live/css/css-masking/parsing/clip-path-valid.html

The failures are for things like:

e.style['clip-path'] = "circle()" should set the property value
assert_equals: serialization should be canonical
expected "circle(at 50% 50%)" but got "circle(at center center)"

We're using keywords (center) in the computed style, while the test is expecting 50%.

Shouldn't the position component be omitted from the serialization altogether in this case, as it's the default? Per https://drafts.csswg.org/cssom/#serialize-a-css-value: "If component values can be omitted or replaced with a shorter representation without changing the meaning of the value, omit/replace them."

(Unless the vague "for legacy reasons..." note is applicable...)

Comment 5

[Daniel Holbert [:dholbert]]

(In reply to Jonathan Kew [:jfkthame] from comment #4)

Shouldn't the position component be omitted from the serialization altogether in this case, as it's the default? Per https://drafts.csswg.org/cssom/#serialize-a-css-value: "If component values can be omitted or replaced with a shorter representation without changing the meaning of the value, omit/replace them."

Yeah, that seems reasonable too. We seem to match that principle for radial-gradient -- here's a testcase:

data:text/html,<!DOCTYPE html>
<body style="background: radial-gradient(circle at center, red, blue)">
<script>alert(document.body.style.backgroundImage)</script>

Firefox and WebKit[1] report radial-gradient(circle, red, blue)
Chrome reports radial-gradient(circle at center center, red, blue)

Notably:

• The radial-gradient grammar uses the exact same at <position> syntax that clip-path does, so it's reasonable to expect that they would serialize consistently.
• Firefox and WebKit are serializing radial-gradient with the <position> omitted, as the spec asks for (per jfkthame's comment 4)
• Chrome is serializing radial-gradient with the <position> explicitly spelled out with keywords -- center center -- the same way that Firefox is serializing clip-path (and differently from how Chrome serializes clip-path).

[1] (Note: my local WebKit is epiphany which doesn't like the newlines or script in my above data-URI, so I tested it there by removing the newlines and inspecting the serialization using devtools console)

Comment 6

[Daniel Holbert [:dholbert]]

I filed https://bugs.chromium.org/p/chromium/issues/detail?id=1427277 on the Chrome-specific radial-gradient serialization issue in comment 5, and I added a note to mention clip-path and the fact that we might want to fix up this WPT test's expectation, for consistency & spec-compliance.

Comment 7

[Boris Chiou [:boris]]

Attachment: Bug 1823475 - Drop DefaultPosition and omit "at <position>" if it is not specified.

at <position> is optional value and we should omit it if author doesn't
specify it, for all basic-shape functions, and ray().

Note that there is a related interpolation issue [1] if one of the end values
doesn't specify at <position>. We didn't address this issue in this patch.

[1] https://github.com/w3c/csswg-drafts/issues/9068

Comment 8

[Boris Chiou [:boris]]

Attachment: Bug 1823475 - Convert position into length-percentage for basic shapes.

Also update shape-outside-valid-position.html because horizontal should
be first per spec:
Using grammar order means that <position> values always give horizontal
components first, then vertical.

https://drafts.csswg.org/css-shapes-1/#basic-shape-serialization

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/150d4fac3536
Drop DefaultPosition and omit "at <position>" if it is not specified. r=devtools-reviewers,emilio

Comment 10

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/150d4fac3536

Comment 11

[Boris Chiou [:boris]]

Attachment: Bug 1823475 - Skip the serialization when using border-box on clip-path.

Per spec, if no reference box is specified, the border-box will be used as
reference box.

Therefore, if the author doesn't specify the reference-box, or specifies
border-box, we shouldn't serialize it.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a3e9e98d521c
Convert position into length-percentage for basic shapes. r=emilio

Comment 13

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/42325 for changes under testing/web-platform/tests

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b3cf916ed82f
Skip the serialization when using border-box on clip-path. r=dholbert

Comment 15

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/a3e9e98d521c
https://hg.mozilla.org/mozilla-central/rev/b3cf916ed82f

Comment 16

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 17

[Boris Chiou [:boris]]

All patches have been landed. Per wpt.fyi, all tests in clip-path-valid.html are passed.