1833491 - WPT css-grid/parsing/grid-template-shorthand-areas-valid.html fails in Firefox

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Daniel Holbert [:dholbert]]

wpt.fyi:
https://wpt.fyi/results/css/css-grid/parsing/grid-template-shorthand-areas-valid.html

Direct link to test:
http://wpt.live/css/css-grid/parsing/grid-template-shorthand-areas-valid.html

The test does something like the following:

let elem = document.documentElement;
elem.style.gridTemplate = "none / 1px" ;
elem.style.gridTemplateAreas = "";
elem.style.gridTemplateAreas = '"a"';

...and then it inspects elem.style.gridTemplate.

The lines that adjust grid-template-areas seem to also reset the serialization of the grid-template shorthand property itself.

Comment 1

[Daniel Holbert [:dholbert]]

So this seems to be a style-system bug with serialization of these interrelated properties.

Note, the grid-template property is defined here:
https://drafts.csswg.org/css-grid/#explicit-grid-shorthand

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: reduced testcase 1

Comment 3

[Daniel Holbert [:dholbert]]

[EDIT: ignore this comment -- as noted in comment 4, I had a typo. I'll repost the testcase and update comment 0 to fix a typo.]

So as I understand it, this WPT test is expecting that both of the buttons in my attached reduced testcase 1 should produce the same result.

(Right now, we alert a different answer from each button.)

Chrome actually matches our behavior on my attached testcase 1, though... and they match our behavior if I run code like that quoted in comment 0 in my web console. And yet, they pass the WPT test.

So there's something suspicious going on here... Possibly a test bug, and possibly a Chrome bug that's making them pass despite the test having a bug.~

Comment 4

[Daniel Holbert [:dholbert]]

Aha, I was missing quotes around "a" in order to trigger the thing that the test is actually testing. When I add those quotes, I do see a difference between Firefox and Chrome. I'll update comment 0 to reflect that, and post an updated version of the testcase.

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: reduced testcase 1

Comment 6

[Daniel Holbert [:dholbert]]

I think this might be a test bug? The grid-template shorthand is supposed to set the grid-template-areas property to none. So once it's been set to some other valid value, it makes sense that it wouldn't be possible to serialize a value from it.

This seems similar to e.g. the situation with font shorthand and the font-size-adjust longhand, which (per MDN) it resets to its initial value (none) despite not directly having the ability to set a custom value for it. It looks like we have the same behavior-difference with Chrome for that example, actually; I'll attach a testcase to demonstrate.

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: reduced testcase 2 (now testing analogous situation with `font` shorthand)

Comment 8

[Daniel Holbert [:dholbert]]

Attachment: screenshot comparing Firefox (top) vs Chrome (bottom) on testcase 2

Here's a screenshot showing the difference for testcase 2.

Comment 9

[Daniel Holbert [:dholbert]]

emilio, does this ring any bells for you? I'm pretty sure we're correct to decline-to-serialize-the-shorthand in this situation, right? (Is there a known Chrome bug on their behavior, assuming they're the buggy ones here?)

Comment 10

[Oriol Brufau [:Oriol]]

Blink is clearly wrong, probably https://bugs.chromium.org/p/chromium/issues/detail?id=1322084 or similar.

Comment 11

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1833491 - Fix grid-template test. r=dholbert

The test is broken. grid-template can't serialize to something which
doesn't really round-trip, so firefox is correct and Blink is wrong.

Comment 12

[Emilio Cobos Álvarez [:emilio]]

Yeah, that test makes no sense.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/792ba4596f52
Fix grid-template test. r=dholbert

Comment 14

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

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

Comment 15

[Narcis Beleuzu [:NarcisB]]

Backed out for wpt failures on grid-template-shorthand-areas-valid.html

Backout link: https://hg.mozilla.org/integration/autoland/rev/10fc0d6f32c1f0b2c11c23f38c36fc79dfd8fc96
Log link: https://treeherder.mozilla.org/logviewer?job_id=416101293&repo=autoland&lineNumber=6069

Comment 16

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

Upstream PR was closed without merging

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/91f286900f71
Fix grid-template test. r=dholbert

Comment 18

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/91f286900f71

Comment 19

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

Upstream PR merged by jgraham