1744309 - wpt failures in /css/css-contain/parsing/ [contain-computed.html, contain-valid.html] due to serializing `inline-size` in the wrong spot and failing to abbreviate to `content` and `strict` in computed style

Status: REOPENED

(Core :: Layout, defect)

Description

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

Syncing wpt PR 31868 found new untriaged test failures in CI

Tests Affected

New Tests That Don't Pass

/css/css-contain/parsing/contain-computed.html
Property contain value 'style layout paint': FAIL (Chrome: PASS, Safari: FAIL)
Property contain value 'size style layout paint': FAIL (Chrome: PASS, Safari: FAIL)
/css/css-contain/parsing/contain-valid.html
e.style['contain'] = "style" should set the property value: FAIL (Chrome: PASS, Safari: FAIL)
e.style['contain'] = "paint style" should set the property value: FAIL (Chrome: PASS, Safari: FAIL)
e.style['contain'] = "layout style paint" should set the property value: FAIL (Chrome: PASS, Safari: FAIL)
e.style['contain'] = "layout paint style size" should set the property value: FAIL (Chrome: PASS, Safari: FAIL)

CI Results

Gecko CI (Treeherder)
GitHub PR Head

Notes

These updates will be on mozilla-central once bug 1744205 lands.

Note: this bug is for tracking fixing the issues and is not
owned by the wpt sync bot.

This bug is linked to the relevant tests by an annotation in
https://github.com/web-platform-tests/wpt-metadata. These annotations
can be edited using the wpt interop dashboard
https://jgraham.github.io/wptdash/

If this bug is split into multiple bugs, please also update the
annotations, otherwise we are unable to track which wpt issues are
already triaged. Resolving as duplicate or closing this issue should
be cause the bot to automatically update or remove the annotation.

Comment 1

[Daniel Holbert [:dholbert]]

All of the failures involve contain:style, which we don't currently support, per bug 1463600.

Comment 2

[Daniel Holbert [:dholbert]]

Now we support contain:style but we still fail some subtests here. Reopening & co-opting this bug since we've already got it linked up from wpt.fyi's view of this test.

At this point our test failures seem to be cases where the test is expecting us to serialize to the shorthand keywords content and strict (first 2 failures), and due to the ordering of inline-size in our serialization (last 2 failures).
https://wpt.fyi/results/css/css-contain/parsing/contain-computed.html
http://wpt.live/css/css-contain/parsing/contain-computed.html

assert_equals: expected "content" but got "layout style paint"
assert_equals: expected "strict" but got "size layout style paint"
assert_equals: expected "inline-size layout style paint" but got "layout style paint inline-size"

https://wpt.fyi/results/css/css-contain/parsing/contain-valid.html
http://wpt.live/css/css-contain/parsing/contain-valid.html

assert_equals: serialization should be canonical expected "inline-size layout" but got "layout inline-size"

Comment 3

[Daniel Holbert [:dholbert]]

I suspect the latter two failures are bogus; the test seems to be expecting inline-size to be serialized at the start, but the spec doesn't make any such guarantee.

In fact, the spec lists that keyword as the last thing in its grammar:

New values: 	layout || style || paint || [ size | inline-size ] 

https://drafts.csswg.org/css-contain-3/#contain-property

...though really the order doesn't matter since || means "can occur in any order".

EDIT: it looks like the order does actually matter for serialization purposes per https://drafts.csswg.org/cssom/#serialize-a-css-value (see bullet point about canonical order and || there). css-contain-1 does have size at the start, and css-contain-3 changed that when adding inline-size, possibly by accident. I filed https://github.com/w3c/csswg-drafts/issues/8600 to clear that up.

Comment 4

[Daniel Holbert [:dholbert]]

...though also: given that we serialize size at the start of the list, we arguably should be serializing inline-size in that same position.

Comment 5

[Daniel Holbert [:dholbert]]

As for the first two failures, checking for the content and strict keywords: it looks like we tag those keywords with their own dedicated bit (bits 6 and 7) in our representation:
https://searchfox.org/mozilla-central/rev/e6a03adbf7930ae0cf131cc3274c80b2aae74e51/servo/components/style/values/specified/box.rs#1382-1385

I suspect those bits are non-functional aside from preserving those keywords (if they're what was specified) in the computed style. This also means that we don't automatically abbreviate to use those keywords in the computed style if they weren't present in the specified style, though.

If you compare these two testcases (check your web console):

data:text/html,<div id="foo" style="contain:strict"></div><script>console.log(window.getComputedStyle(foo).contain)</script>

data:text/html,<div id="foo" style="contain:size layout style paint"></div><script>console.log(window.getComputedStyle(foo).contain)</script>

...you can see that Firefox uses the longhand form (as-it-was-specified) in the latter case, whereas Chrome produces the shorter strict keyword there.

https://drafts.csswg.org/css-contain-1/#contain-property has a PROPOSED CORRECTION 1 block above the definition of the property which asserts that we're (a) not supposed to care about whether one of the short keywords were used, and (b) we're supposed to serialize back to a short keyword if it's equivalent (due to the shortest serialization principle). And it links out to this very WPT test as a test for that.

I'm not sure the shortest serialization principle is well-defined (see resolution in https://github.com/w3c/csswg-drafts/issues/1564#issuecomment-365680255 and also https://github.com/w3c/csswg-drafts/issues/1564#issuecomment-365683157 ); but also, I suspect this is simple to fix and nice for interop & brevity, so we might as well just fix it.

Comment 6

[Daniel Holbert [:dholbert]]

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

css-contain-1 does have size at the start, and css-contain-3 changed that when adding inline-size, possibly by accident. I filed https://github.com/w3c/csswg-drafts/issues/8600 to clear that up.

That's now been fixed; see https://github.com/w3c/csswg-drafts/issues/8600#issuecomment-1535285490 .

So the inline-size ordering issues are now clearly a Firefox bug, and we should serialize inline-size at the start.

Comment 7

[Thomas Wisniewski [:twisniewski]]

I think may now be resolved, as we're passing the tests with the exception of one in the "valid" WPT, due to not supporting "contain-intrinsic-size: auto none" (bug 1843746).

Comment 8

[Thomas Wisniewski [:twisniewski]]

We now only fail this case:

e.style['contain'] = "layout inline-size" should set the property value:
   assert_equals: serialization should be canonical expected "inline-size layout" but got "layout inline-size"

So this is probably going to be fixed along with bug 1672669.