search

web-platform-tests / wpt

Test suites for Web platform specs — including WHATWG, W3C, and others
https://web-platform-tests.org/
Other
4.86k stars 3.04k forks source link

cssom/shorthand-values.html is wrong #13523

Open Loirooriol opened 5 years ago

Loirooriol commented 5 years ago

https://github.com/web-platform-tests/wpt/blob/780c03bd757ee85581fece3e5e2f4434de7a1014/css/cssom/shorthand-values.html#L31

It says that

border: 1px;
border-top: 1px !important;

should serialize as

border-right-width: 1px;
border-bottom-width: 1px;
border-left-width: 1px;
border-top-width: 1px !important;

But that's not right because the border shorthand also sets border-*-style and border-*-color longhands, so according to https://drafts.csswg.org/cssom/#serialize-a-css-declaration-block I expect something like

border-right: 1px;
border-bottom: 1px;
border-left: 1px;
border-top: 1px !important;

This is what Firefox and Blink do, and what WebKit will do after https://bugs.webkit.org/show_bug.cgi?id=190496

However, the border shorthand also sets border-image: https://drafts.csswg.org/css-backgrounds-3/#propdef-border

The border shorthand also resets border-image to its initial value.

There is no interoperability about this:

I think the Firefox behavior is the good one (using initial instead of the real initial value can cause problems like https://bugs.chromium.org/p/chromium/issues/detail?id=386459), but it's not completely clear.

You can see the results in https://wpt.fyi/results/css/cssom/shorthand-values.html?label=experimental

@emilio can you confirm the expected output?

emilio commented 5 years ago

Yeah, I agree that that entry is wrong. A browser that passes that subtest is clearly bogus.

Maybe the test should read something like: border-width: 1px; border-top-width: 1px !important;?

Blink includes border-image: initial in the cssText serialization

I think this behavior is wrong. Resetting to the initial value is not the same as resetting to the initial keyword.

All in all I agree with your analysis.

Loirooriol commented 5 years ago

Maybe the test should read something like: border-width: 1px; border-top-width: 1px !important;?

Adding this subtest seems a good idea, but given the issues with the existing one, I would also keep it (fixing the expected output).

Loirooriol commented 5 years ago

Thinking more about this, Firefox seems right about setting border-image to none 100% / 1 / 0 stretch instead of initial, but not all these values should appear in the serialization according to 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.

The syntax of border-image is

<‘border-image-source’> || <‘border-image-slice’> [ / <‘border-image-width’> | / <‘border-image-width’>? / <‘border-image-outset’> ]? || <‘border-image-repeat’>

So there are three possible minimal serializations:

Which one should be chosen? Edge uses the first one. And I can't really say Firefox is wrong due to

If either of the above syntactic translations would be less backwards-compatible, do not perform them

Seems fairly undefined behavior, maybe the subtest shouldn't be kept after all.