Closed Bug 1500590 Opened 4 years ago Closed 3 years ago

Firefox fails WPT test css-grid/parsing/grid-template-areas-valid.html (due to preserving whitespace when parsing grid-template-areas tokens)

Categories

(Core :: CSS Parsing and Computation, enhancement, P3)

enhancement

Tracking

()

RESOLVED DUPLICATE of bug 1505635
Tracking Status
firefox64 --- affected

People

(Reporter: dholbert, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: parity-chrome, parity-safari)

Attachments

(1 file)

Attached file testcase 1
We're apparently the only browser with failures in this test:
http://w3c-test.org/css/css-grid/parsing/grid-template-areas-valid.html

As noted in the source code there,
> // Firefox preserves specific whitespace [...]
> // Other browsers consolidate to ' ' [...]


I'm attaching a testcase to demonstrate this. Chrome gives the same string on each line of the results, whereas Firefox shows different amounts of spaces and/or "\9" (tab) characters.
I don't see any requirement in the spec that white-space should be normalized.
https://drafts.csswg.org/css-grid/#grid-template-areas-property
"Computed value: 	the keyword none or a list of string values"
If anything, I read that as "return the strings that were specified"
which is what we do, IIRC.
Looks like the test-author (Eric Willigers) has a bugzilla account -- let's check in with him.

Hi Eric!  See comment 1 - can you comment on this expectation about whitespace serialization in your test?  Is there any spec text that justifies the test's expectation?
Flags: needinfo?(ericwilligers)
I was going with the majority as I found the spec unclear. I have no specific expertise for these properties.

Serialization is often under-specified.
https://drafts.csswg.org/cssom/#serializing-css-values
"Note: The rules described here outlines the general principles of serialization. For legacy reasons, some properties serialize in a different manner, which is intentionally undefined here due to lack of resources. Please consult your local reverse-engineer for details."

I wrote test_valid_value to accept an array of tolerated serializations. mrego has pushed back against such usage, e.g.
https://github.com/web-platform-tests/wpt/pull/11781#discussion_r215135461
"Then IMHO we shouldn't check both things and some browsers will see a failure and eventually fix the issue (or re-discuss the topic if they don't agree)."

Where browsers' serializations differ, I have noted these with comments in the tests, and/or have raised browser bugs and linked to them in the tests' pull requests. Where the spec is not obvious, I have treated it as a judgement call whether to accept a variety of serializations, or mandate a specific serialization. Reviewers can (and do) challenge my judgement calls. I have avoided the Chromium->WPT import process for these tests, to maximize cross-browser engagement.
Flags: needinfo?(ericwilligers)
(In reply to Eric Willigers from comment #3)
> I wrote test_valid_value to accept an array of tolerated serializations.
> mrego has pushed back against such usage, e.g.
> https://github.com/web-platform-tests/wpt/pull/11781#discussion_r215135461

I agree with @mrego in that particular case, i.e. redundant keywords MUST be
omitted when serializing values.  Including them is not compliant with CSSOM.

I filed bug 1500708 on our 'grid-auto-flow' serialization not being
the shortest form and I'll correct the grid-auto-flow-valid.html WPT there.
Please file bugs if you know about more cases like that!

==

Serializing a <string> on the other hand is different.  The CSSOM spec says:
https://drafts.csswg.org/cssom/#serializing-css-values
"<string>
  The string _serialized as a string_."

and white-space normalization is NOT allowed there:
https://drafts.csswg.org/cssom/#serialize-a-string

So, this is a bug in Chrome.
I've filed https://bugs.chromium.org/p/chromium/issues/detail?id=897444
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INVALID
https://github.com/web-platform-tests/wpt/pull/13745 updates the WPT to show Firefox passing.
It appears the CSSWG has now decided to change the spec:
https://github.com/w3c/csswg-drafts/issues/3261#issuecomment-446672986
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Blocks: 1514265

We already have a bug open on implementing that spec resolution...

Status: REOPENED → RESOLVED
Closed: 4 years ago3 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.