New issue
Advanced search Search tips

Issue 811083 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Compat



Sign in to add a comment

Incorrect parsing of box-shadow: color inset {2 lengths}

Reported by goo...@gtalbot.org, Feb 11 2018

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0

Example URL:
http://www.gtalbot.org/BrowserBugsSection/CSS3Backgrounds/box-shadow-032.xht

Steps to reproduce the problem:
2 reduced self-explanatory tests that Chrome 65.0.3315.3 (beta release) fails:

http://www.gtalbot.org/BrowserBugsSection/CSS3Backgrounds/box-shadow-032.xht
which uses the code 
box-shadow: green inset 100px 100px 0px 0px;

http://www.gtalbot.org/BrowserBugsSection/CSS3Backgrounds/box-shadow-036.xht
which uses the code
box-shadow: 100px 100px 0px 0px inset green;

Firefox 60.0a1 buildID=20180210220139 passes those 2 tests.

What is the expected behavior?
A filled green square and no red

What went wrong?
A red square

Does it occur on multiple sites: N/A

Is it a problem with a plugin? No 

Did this work before? No 

Does this work in other browsers? Yes

Chrome version: <Copy from: 'about:version'>  Channel: beta
OS Version: 
Flash Version: irrelevant

The 'box-shadow' syntax is

<shadow> = inset? && <length>{2,4} && <color>?

https://www.w3.org/TR/css-backgrounds-3/#the-box-shadow

therefore the optional reserved keyword inset and the optional color could be anywhere, in any order, except between length values.

The buggy behavior (red square) remains if I use 2 (or 3) length values instead of 4. Eg:
box-shadow: green inset 100px 100px;
fails in Chrome 65.0.3315.3.

Tests pass if color is defined with the color property, outside the box-shadow declaration.
Eg
color: green;
box-shadow: inset 100px 100px;
creates a green shadow overlapping the red square in Chrome 65.

- - - - - - - 

Chrome 65.0.3315.3 also fails

http://test.csswg.org/suites/css-backgrounds-3_dev/nightly-unstable/html4/box-shadow-syntax-001.htm

for the exact same (parsing) reasons.
 

Comment 1 by cnardi@chromium.org, Feb 11 2018

Components: Blink>CSS
Labels: -OS-Linux Hotlist-Interop
Owner: cnardi@chromium.org
Status: Started (was: Unconfirmed)
Confirmed, should be a pretty simple fix.
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b638ee4ed93101d3a8dd7cf92fcd71b98922a2e6

commit b638ee4ed93101d3a8dd7cf92fcd71b98922a2e6
Author: Chris Nardi <cnardi@chromium.org>
Date: Tue Feb 13 06:24:05 2018

Ignore order when parsing inset and color for box-shadow

According to the spec [1], box-shadow should allow the inset keyword and
a color in any order. Our implementation allowed only 2 out of the 4
possible orders; fix our parsing to match the spec.

[1]: https://drafts.csswg.org/css-backgrounds/#typedef-shadow

Bug:  811083 
Change-Id: I54c3d15c177702624994c45044b446fd82fc9d98
Reviewed-on: https://chromium-review.googlesource.com/911994
Commit-Queue: Chris Nardi <cnardi@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536270}
[modify] https://crrev.com/b638ee4ed93101d3a8dd7cf92fcd71b98922a2e6/third_party/WebKit/LayoutTests/NeverFixTests
[modify] https://crrev.com/b638ee4ed93101d3a8dd7cf92fcd71b98922a2e6/third_party/WebKit/LayoutTests/external/wpt/css/css-backgrounds/box-shadow-syntax-001.xht
[modify] https://crrev.com/b638ee4ed93101d3a8dd7cf92fcd71b98922a2e6/third_party/WebKit/LayoutTests/external/wpt/css/css-backgrounds/reference/box-shadow-syntax-001.xht
[modify] https://crrev.com/b638ee4ed93101d3a8dd7cf92fcd71b98922a2e6/third_party/WebKit/LayoutTests/platform/linux/fast/box-shadow/inset-expected.png
[modify] https://crrev.com/b638ee4ed93101d3a8dd7cf92fcd71b98922a2e6/third_party/WebKit/LayoutTests/platform/linux/fast/box-shadow/inset-subpixel-expected.png
[modify] https://crrev.com/b638ee4ed93101d3a8dd7cf92fcd71b98922a2e6/third_party/WebKit/LayoutTests/platform/mac/fast/box-shadow/inset-expected.png
[modify] https://crrev.com/b638ee4ed93101d3a8dd7cf92fcd71b98922a2e6/third_party/WebKit/LayoutTests/platform/mac/fast/box-shadow/inset-subpixel-expected.png
[modify] https://crrev.com/b638ee4ed93101d3a8dd7cf92fcd71b98922a2e6/third_party/WebKit/LayoutTests/platform/win/fast/box-shadow/inset-expected.png
[modify] https://crrev.com/b638ee4ed93101d3a8dd7cf92fcd71b98922a2e6/third_party/WebKit/LayoutTests/platform/win/fast/box-shadow/inset-subpixel-expected.png
[modify] https://crrev.com/b638ee4ed93101d3a8dd7cf92fcd71b98922a2e6/third_party/WebKit/Source/core/css/CSSShadowValue.cpp
[modify] https://crrev.com/b638ee4ed93101d3a8dd7cf92fcd71b98922a2e6/third_party/WebKit/Source/core/css/properties/CSSParsingUtils.cpp

Comment 3 by cnardi@chromium.org, Feb 13 2018

Status: Fixed (was: Started)
Filed a follow-up spec bug for serialization, which should change the spec to match Firefox/Chrome/WebKit: https://github.com/w3c/csswg-drafts/issues/2305.

Sign in to add a comment