Repository navigation
[css-backgrounds] Change canonical serialization for box-shadow #2305
Description
The spec declares the syntax for a shadow in box-shadow to be:
<shadow> = inset? && <length>{2,4} && <color>?
However, Firefox, WebKit, and Chrome all serialize this as <color>? && <length>{2,4} && inset?. This matches the serialization for drop-shadow() (see https://drafts.fxtf.org/filter-effects/#funcdef-filter-drop-shadow) and text-shadow (see https://drafts.csswg.org/css-text-decor-3/#text-shadow-property), which both are declared as <color>? && <length>{2,3}.
For consistency, <shadow> should be defined as:
<shadow> = <color>? && <length>{2,4} && inset?
in order for serialization to match 3 out of the 4 major browsers and text-shadow/drop-shadow().
Activity
[css-backgrounds-3] Match grammar of <<shadow>> to that of text-shado…
fantasai commentedon Feb 14, 2018
Pushed out tentative edits, waiting for WG approval.
Test box-shadow serialization
Test box-shadow serialization
css-meeting-bot commentedon Feb 28, 2018
The Working Group just discussed
[css-backgrounds] Change canonical serialization for box-shadow, and agreed to the following resolutions:RESOLVED: switch the order to match current implementationsThe full IRC log of that discussion
<dael> Topic: [css-backgrounds] Change canonical serialization for box-shadow<dael> github: https://github.com//issues/2305
<dael> TabAtkins: Going by standard rule that default serialization should match grammar that implies color after link but impl oes color before links. We should change grammer to match impl.
<dael> Rossen_: Looks like edits were pushed out. So only thing we need to do is get consensus.
<dael> Rossen_: Any other opinions as to why we shouldn't do this?
<dbaron> This is part of why I am not a fan of that general rule being imposed without doing compatibility testing first... although I think we may have rolled back the rule a bit?
<bradk> +1
<AmeliaBR> +1 to matching reality & also other properties
<dael> Rossen_: Objections to switch the order to match current impl?
<dael> RESOLVED: switch the order to match current implementations
Correct serialization of box-shadow and text-shadow
csnardi commentedon Mar 7, 2018
@fantasai I created two PRs for tests that should verify this change; they should be linked to this issue.
TalbotG commentedon Mar 13, 2018
@csnardi If you need more tests, you know that I have 15 tests targeting box-shadow in
http://www.gtalbot.org/BrowserBugsSection/CSS3Backgrounds/
http://www.gtalbot.org/BrowserBugsSection/CSS3Backgrounds/box-shadow-021.xht
...
http://www.gtalbot.org/BrowserBugsSection/CSS3Backgrounds/box-shadow-036.xht
csnardi commentedon Mar 13, 2018
@TalbotG Thanks, I didn't know that! I'll take a look at them a little bit later and add in any parsing cases I missed to the first pull request linked above.
csnardi commentedon Apr 6, 2018
@fantasai Friendly reminder if you have some time for reviewing web-platform-tests/wpt#9800 and web-platform-tests/wpt#9519 which should close out this issue.
Correct serialization of box-shadow and text-shadow
2 remaining items
Bug 1438276 [wpt PR 9519] - Test box-shadow serialization, a=testonly
csnardi commentedon Apr 23, 2018
I think the needs testcase label can be removed now.
Bug 1443103 [wpt PR 9800] - Correct serialization of box-shadow and t…
Bug 1443103 [wpt PR 9800] - Correct serialization of box-shadow and t…
Bug 1438276 [wpt PR 9519] - Test box-shadow serialization, a=testonly
Bug 1443103 [wpt PR 9800] - Correct serialization of box-shadow and t…
Bug 1438276 [wpt PR 9519] - Test box-shadow serialization, a=testonly
Bug 1438276 [wpt PR 9519] - Test box-shadow serialization, a=testonly
Bug 1443103 [wpt PR 9800] - Correct serialization of box-shadow and t…
Bug 1438276 [wpt PR 9519] - Test box-shadow serialization, a=testonly