Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Values set by mask and background shorthands should not serialize as …
…"initial"

https://bugs.webkit.org/show_bug.cgi?id=251838
rdar://problem/105114588

Reviewed by Tim Nguyen.

Needed to expand the function that serializes longhands to cover this case.
Also fixed serialization of the background shorthand to more closely match
the specification and a mistake in the parser that allowed invalid background
shorthand values involving sizes without positions.

* LayoutTests/editing/deleting/paste-with-transparent-background-color-expected.txt:
* LayoutTests/editing/deleting/paste-with-transparent-background-color-live-range-expected.txt:
Updated for progression where we no longer add "background-image: none" as a side effect.

* LayoutTests/fast/backgrounds/background-shorthand-after-set-backgroundSize-expected.txt:
* LayoutTests/fast/backgrounds/background-shorthand-after-set-backgroundSize.html:
* LayoutTests/fast/backgrounds/background-shorthand-with-backgroundSize-style-expected.txt:
* LayoutTests/fast/backgrounds/background-shorthand-with-backgroundSize-style.html:
* LayoutTests/fast/css/remove-shorthand-expected.txt:
* LayoutTests/fast/dom/background-shorthand-csstext-expected.txt:
* LayoutTests/fast/dom/background-shorthand-csstext.html:
Updated to assume background-color will be serialized first, as specified.

* LayoutTests/imported/w3c/web-platform-tests/css/css-backgrounds/parsing/background-invalid-expected.txt:
Updated to expect PASS for the test of parsing a size without a position.

* LayoutTests/imported/w3c/web-platform-tests/css/css-backgrounds/parsing/background-valid-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-backgrounds/parsing/background-valid.html:
Added tests of the longhand values. Removed result that allowed WebKit's old
incorrect serialization.

* LayoutTests/imported/w3c/web-platform-tests/css/css-masking/parsing/mask-valid.sub-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-masking/parsing/mask-valid.sub.html:
* LayoutTests/platform/glib/imported/w3c/web-platform-tests/css/css-masking/parsing/mask-valid.sub-expected.txt:
Added tests of the longhand values.

* Source/WebCore/css/CSSProperties.json: Moved background-color to the start
of the background shorthand so serialization matches the specification.

* Source/WebCore/css/StyleProperties.cpp:
(WebCore::serializeLonghandValue): Added special case for the lists that are
used for the mask and background longhands, because the parser puts
placeholders in there when parsing the shorthands.

* Source/WebCore/css/parser/CSSPropertyParser.cpp:
(WebCore::CSSPropertyParser::consumeBackgroundShorthand): Only parse size
immediately after a position. The old code required that it be after a
position, but not immediately after. Removed comment saying this makes
"invalid longhands". They are now valid, and the serializeLonghandValue
function above makes them serialize correctly.

Canonical link: https://commits.webkit.org/260157@main
  • Loading branch information
darinadler committed Feb 11, 2023
1 parent dd5e8f4 commit 5d2cf87
Show file tree
Hide file tree
Showing 18 changed files with 156 additions and 39 deletions.
Expand Up @@ -13,7 +13,7 @@ After cut and paste:
| "hello "
| <span>
| class="test"
| style="background-image: none; background-color: transparent;"
| style="background-color: transparent;"
| "world"
| " WebKit<#selection-caret>"
| <br>
Expand Up @@ -13,7 +13,7 @@ After cut and paste:
| "hello "
| <span>
| class="test"
| style="background-image: none; background-color: transparent;"
| style="background-color: transparent;"
| "world"
| " WebKit<#selection-caret>"
| <br>
Expand Up @@ -3,7 +3,7 @@ Tests a flag to make background shorthand property not override background-size
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS e.style.background is 'url("dummy://test.png") center center / cover no-repeat border-box red'
PASS e.style.background is 'red url("dummy://test.png") center center / cover no-repeat border-box'
PASS e.style.backgroundSize is 'cover'

PASS successfullyParsed is true
Expand Down
Expand Up @@ -12,7 +12,7 @@

e.style.backgroundSize = "cover";
e.style.background = "center red url(dummy://test.png) no-repeat border-box";
shouldBe("e.style.background", "'url(\"dummy://test.png\") center center / cover no-repeat border-box red'")
shouldBe("e.style.background", "'red url(\"dummy://test.png\") center center / cover no-repeat border-box'")
shouldBe("e.style.backgroundSize", "'cover'");
debug("")

Expand Down
Expand Up @@ -3,56 +3,56 @@ Tests background shortand property with background-size
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS e.style.background is 'url("dummy://test.png") center center / cover no-repeat border-box red'
PASS e.style.background is 'red url("dummy://test.png") center center / cover no-repeat border-box'
PASS e.style.backgroundSize is 'cover'
PASS checkStyle() is true
PASS computedStyle.getPropertyValue("background") is 'rgb(255, 0, 0) url("dummy://test.png") no-repeat scroll 50% 50% / cover border-box border-box'
PASS computedStyle.getPropertyValue("background-size") is 'cover'
PASS checkComputedStyleValue() is true

PASS e.style.background is 'url("dummy://test.png") 20px center / contain no-repeat padding-box red'
PASS e.style.background is 'red url("dummy://test.png") 20px center / contain no-repeat padding-box'
PASS e.style.backgroundSize is 'contain'
PASS checkStyle() is true
PASS computedStyle.getPropertyValue("background") is 'rgb(255, 0, 0) url("dummy://test.png") no-repeat scroll 20px 50% / contain padding-box padding-box'
PASS computedStyle.getPropertyValue("background-size") is 'contain'
PASS checkComputedStyleValue() is true

PASS e.style.background is 'url("dummy://test.png") 50px 60px / 50% 75% no-repeat red'
PASS e.style.background is 'red url("dummy://test.png") 50px 60px / 50% 75% no-repeat'
PASS e.style.backgroundSize is '50% 75%'
PASS checkStyle() is true
PASS computedStyle.getPropertyValue("background") is 'rgb(255, 0, 0) url("dummy://test.png") no-repeat scroll 50px 60px / 50% 75% padding-box border-box'
PASS computedStyle.getPropertyValue("background-size") is '50% 75%'
PASS checkComputedStyleValue() is true

PASS e.style.background is 'url("dummy://test.png") left top / 100px 200px repeat border-box content-box red'
PASS e.style.background is 'red url("dummy://test.png") left top / 100px 200px repeat border-box content-box'
PASS e.style.backgroundSize is '100px 200px'
PASS checkStyle() is true
PASS computedStyle.getPropertyValue("background") is 'rgb(255, 0, 0) url("dummy://test.png") repeat scroll 0% 0% / 100px 200px border-box content-box'
PASS computedStyle.getPropertyValue("background-size") is '100px 200px'
PASS checkComputedStyleValue() is true

PASS e.style.background is 'url("dummy://test.png") 50% repeat content-box padding-box red'
PASS e.style.background is 'red url("dummy://test.png") 50% repeat content-box padding-box'
PASS e.style.backgroundSize is 'auto'
PASS checkStyle() is true
PASS computedStyle.getPropertyValue("background") is 'rgb(255, 0, 0) url("dummy://test.png") repeat scroll 50% 50% / auto content-box padding-box'
PASS computedStyle.getPropertyValue("background-size") is 'auto'
PASS checkComputedStyleValue() is true

PASS e.style.background is 'url("dummy://test.png") 50px 60px / 50% auto no-repeat fixed red'
PASS e.style.background is 'red url("dummy://test.png") 50px 60px / 50% auto no-repeat fixed'
PASS e.style.backgroundSize is '50% auto'
PASS checkStyle() is true
PASS computedStyle.getPropertyValue("background") is 'rgb(255, 0, 0) url("dummy://test.png") no-repeat fixed 50px 60px / 50% auto padding-box border-box'
PASS computedStyle.getPropertyValue("background-size") is '50% auto'
PASS checkComputedStyleValue() is true

PASS e.style.background is 'url("dummy://test.png") left top / 100px auto repeat red'
PASS e.style.background is 'red url("dummy://test.png") left top / 100px auto repeat'
PASS e.style.backgroundSize is '100px auto'
PASS checkStyle() is true
PASS computedStyle.getPropertyValue("background") is 'rgb(255, 0, 0) url("dummy://test.png") repeat scroll 0% 0% / 100px auto padding-box border-box'
PASS computedStyle.getPropertyValue("background-size") is '100px auto'
PASS checkComputedStyleValue() is true

PASS e.style.background is 'url("dummy://test.png") 50% repeat fixed content-box red'
PASS e.style.background is 'red url("dummy://test.png") 50% repeat fixed content-box'
PASS e.style.backgroundSize is 'auto'
PASS checkStyle() is true
PASS computedStyle.getPropertyValue("background") is 'rgb(255, 0, 0) url("dummy://test.png") repeat fixed 50% 50% / auto content-box content-box'
Expand All @@ -66,7 +66,7 @@ PASS computedStyle.getPropertyValue("background") is 'rgba(0, 0, 0, 0) none repe
PASS computedStyle.getPropertyValue("background-size") is '50% auto'
PASS checkComputedStyleValue() is true

PASS e.style.background is 'fixed red'
PASS e.style.background is 'red fixed'
PASS e.style.backgroundSize is 'auto'
PASS checkStyle() is true
PASS computedStyle.getPropertyValue("background") is 'rgb(255, 0, 0) none repeat fixed 0% 0% / auto padding-box border-box'
Expand Down
Expand Up @@ -23,7 +23,7 @@
}

e.style.background = "center / cover red url(dummy://test.png) no-repeat border-box";
shouldBe("e.style.background", "'url(\"dummy://test.png\") center center / cover no-repeat border-box red'");
shouldBe("e.style.background", "'red url(\"dummy://test.png\") center center / cover no-repeat border-box'");
shouldBe("e.style.backgroundSize", "'cover'");
shouldBe("checkStyle()", "true");
shouldBe('computedStyle.getPropertyValue("background")', "'rgb(255, 0, 0) url(\"dummy://test.png\") no-repeat scroll 50% 50% / cover border-box border-box'");
Expand All @@ -32,7 +32,7 @@
debug("")

e.style.background = "red 20px / contain url(dummy://test.png) no-repeat padding-box";
shouldBe("e.style.background", "'url(\"dummy://test.png\") 20px center / contain no-repeat padding-box red'");
shouldBe("e.style.background", "'red url(\"dummy://test.png\") 20px center / contain no-repeat padding-box'");
shouldBe("e.style.backgroundSize", "'contain'");
shouldBe("checkStyle()", "true");
shouldBe('computedStyle.getPropertyValue("background")', "'rgb(255, 0, 0) url(\"dummy://test.png\") no-repeat scroll 20px 50% / contain padding-box padding-box'");
Expand All @@ -41,7 +41,7 @@
debug("")

e.style.background = "red url(dummy://test.png) 50px 60px / 50% 75% no-repeat";
shouldBe("e.style.background", "'url(\"dummy://test.png\") 50px 60px / 50% 75% no-repeat red'");
shouldBe("e.style.background", "'red url(\"dummy://test.png\") 50px 60px / 50% 75% no-repeat'");
shouldBe("e.style.backgroundSize", "'50% 75%'");
shouldBe("checkStyle()", "true");
shouldBe('computedStyle.getPropertyValue("background")', "'rgb(255, 0, 0) url(\"dummy://test.png\") no-repeat scroll 50px 60px / 50% 75% padding-box border-box'");
Expand All @@ -50,7 +50,7 @@
debug("")

e.style.background = "red url(dummy://test.png) repeat top left / 100px 200px border-box content-box";
shouldBe("e.style.background", "'url(\"dummy://test.png\") left top / 100px 200px repeat border-box content-box red'");
shouldBe("e.style.background", "'red url(\"dummy://test.png\") left top / 100px 200px repeat border-box content-box'");
shouldBe("e.style.backgroundSize", "'100px 200px'");
shouldBe("checkStyle()", "true");
shouldBe('computedStyle.getPropertyValue("background")', "'rgb(255, 0, 0) url(\"dummy://test.png\") repeat scroll 0% 0% / 100px 200px border-box content-box'");
Expand All @@ -59,7 +59,7 @@
debug("")

e.style.background = "red url(dummy://test.png) repeat 50% / auto auto content-box padding-box";
shouldBe("e.style.background", "'url(\"dummy://test.png\") 50% repeat content-box padding-box red'");
shouldBe("e.style.background", "'red url(\"dummy://test.png\") 50% repeat content-box padding-box'");
shouldBe("e.style.backgroundSize", "'auto'");
shouldBe("checkStyle()", "true");
shouldBe('computedStyle.getPropertyValue("background")', "'rgb(255, 0, 0) url(\"dummy://test.png\") repeat scroll 50% 50% / auto content-box padding-box'");
Expand All @@ -68,7 +68,7 @@
debug("")

e.style.background = "url(dummy://test.png) red 50px 60px / 50% no-repeat fixed";
shouldBe("e.style.background", "'url(\"dummy://test.png\") 50px 60px / 50% auto no-repeat fixed red'");
shouldBe("e.style.background", "'red url(\"dummy://test.png\") 50px 60px / 50% auto no-repeat fixed'");
shouldBe("e.style.backgroundSize", "'50% auto'");
shouldBe("checkStyle()", "true");
shouldBe('computedStyle.getPropertyValue("background")', "'rgb(255, 0, 0) url(\"dummy://test.png\") no-repeat fixed 50px 60px / 50% auto padding-box border-box'");
Expand All @@ -77,7 +77,7 @@
debug("")

e.style.background = "red repeat scroll padding-box border-box top left / 100px url(dummy://test.png)";
shouldBe("e.style.background", "'url(\"dummy://test.png\") left top / 100px auto repeat red'");
shouldBe("e.style.background", "'red url(\"dummy://test.png\") left top / 100px auto repeat'");
shouldBe("e.style.backgroundSize", "'100px auto'");
shouldBe("checkStyle()", "true");
shouldBe('computedStyle.getPropertyValue("background")', "'rgb(255, 0, 0) url(\"dummy://test.png\") repeat scroll 0% 0% / 100px auto padding-box border-box'");
Expand All @@ -86,7 +86,7 @@
debug("")

e.style.background = "50% / auto fixed url(dummy://test.png) repeat content-box red";
shouldBe("e.style.background", "'url(\"dummy://test.png\") 50% repeat fixed content-box red'");
shouldBe("e.style.background", "'red url(\"dummy://test.png\") 50% repeat fixed content-box'");
shouldBe("e.style.backgroundSize", "'auto'");
shouldBe("checkStyle()", "true");
shouldBe('computedStyle.getPropertyValue("background")', "'rgb(255, 0, 0) url(\"dummy://test.png\") repeat fixed 50% 50% / auto content-box content-box'");
Expand All @@ -104,7 +104,7 @@
debug("")

e.style.background = "red fixed";
shouldBe("e.style.background", "'fixed red'");
shouldBe("e.style.background", "'red fixed'");
shouldBe("e.style.backgroundSize", "'auto'");
shouldBe("checkStyle()", "true");
shouldBe('computedStyle.getPropertyValue("background")', "'rgb(255, 0, 0) none repeat fixed 0% 0% / auto padding-box border-box'");
Expand Down
2 changes: 1 addition & 1 deletion LayoutTests/fast/css/remove-shorthand-expected.txt
Expand Up @@ -7,7 +7,7 @@ removes "background"
and adds "".
Removing background-position
removes "background"
and adds "background-image, background-size, background-repeat, background-attachment, background-origin, background-clip, background-color".
and adds "background-color, background-image, background-size, background-repeat, background-attachment, background-origin, background-clip".
Removing border
removes "border"
and adds "".
Expand Down
Expand Up @@ -2,5 +2,5 @@ This page tests whether or not the background shorthand properly omits initial v

PASS: document.body.style.background should be green and is.
PASS: document.getElementById('div1').style.background should be and is.
PASS: document.getElementById('div2').style.background should be 50% 50% blue and is.
PASS: document.getElementById('div3').style.background should be repeat rgb(255, 255, 255) and is.
PASS: document.getElementById('div2').style.background should be blue 50% 50% and is.
PASS: document.getElementById('div3').style.background should be rgb(255, 255, 255) repeat and is.
4 changes: 2 additions & 2 deletions LayoutTests/fast/dom/background-shorthand-csstext.html
Expand Up @@ -33,8 +33,8 @@

shouldBe("document.body.style.background", "green");
shouldBe("document.getElementById('div1').style.background", "");
shouldBe("document.getElementById('div2').style.background", "50% 50% blue");
shouldBe("document.getElementById('div3').style.background", "repeat rgb(255, 255, 255)");
shouldBe("document.getElementById('div2').style.background", "blue 50% 50%");
shouldBe("document.getElementById('div3').style.background", "rgb(255, 255, 255) repeat");
}
</script>
</head>
Expand Down
@@ -1,4 +1,4 @@

PASS e.style['background'] = "red, green" should not set the property value
FAIL e.style['background'] = "black 0 url(https://example.invalid/) / cover" should not set the property value assert_equals: expected "" but got "url(\"https://example.invalid/\") 0px center / cover black"
PASS e.style['background'] = "black 0 url(https://example.invalid/) / cover" should not set the property value

@@ -1,3 +1,21 @@

PASS e.style['background'] = "url(\"https://example.com/\") 1px 2px / 3px 4px space round local padding-box content-box, rgb(5, 6, 7) url(\"https://example.com/\") 1px 2px / 3px 4px space round local padding-box content-box" should set the property value
PASS e.style['background'] = "none" should set background-attachment
PASS e.style['background'] = "none" should set background-clip
PASS e.style['background'] = "none" should set background-color
PASS e.style['background'] = "none" should set background-image
PASS e.style['background'] = "none" should set background-origin
PASS e.style['background'] = "none" should set background-position
PASS e.style['background'] = "none" should set background-repeat
PASS e.style['background'] = "none" should set background-size
PASS e.style['background'] = "none" should not set unrelated longhands
PASS e.style['background'] = "url(\"https://example.com/\") 1px 2px / 3px 4px space round local padding-box content-box, rgb(5, 6, 7) url(\"https://example.com/\") 1px 2px / 3px 4px space round local padding-box content-box" should set background-attachment
PASS e.style['background'] = "url(\"https://example.com/\") 1px 2px / 3px 4px space round local padding-box content-box, rgb(5, 6, 7) url(\"https://example.com/\") 1px 2px / 3px 4px space round local padding-box content-box" should set background-clip
PASS e.style['background'] = "url(\"https://example.com/\") 1px 2px / 3px 4px space round local padding-box content-box, rgb(5, 6, 7) url(\"https://example.com/\") 1px 2px / 3px 4px space round local padding-box content-box" should set background-color
PASS e.style['background'] = "url(\"https://example.com/\") 1px 2px / 3px 4px space round local padding-box content-box, rgb(5, 6, 7) url(\"https://example.com/\") 1px 2px / 3px 4px space round local padding-box content-box" should set background-image
PASS e.style['background'] = "url(\"https://example.com/\") 1px 2px / 3px 4px space round local padding-box content-box, rgb(5, 6, 7) url(\"https://example.com/\") 1px 2px / 3px 4px space round local padding-box content-box" should set background-origin
PASS e.style['background'] = "url(\"https://example.com/\") 1px 2px / 3px 4px space round local padding-box content-box, rgb(5, 6, 7) url(\"https://example.com/\") 1px 2px / 3px 4px space round local padding-box content-box" should set background-position
PASS e.style['background'] = "url(\"https://example.com/\") 1px 2px / 3px 4px space round local padding-box content-box, rgb(5, 6, 7) url(\"https://example.com/\") 1px 2px / 3px 4px space round local padding-box content-box" should set background-repeat
PASS e.style['background'] = "url(\"https://example.com/\") 1px 2px / 3px 4px space round local padding-box content-box, rgb(5, 6, 7) url(\"https://example.com/\") 1px 2px / 3px 4px space round local padding-box content-box" should set background-size
PASS e.style['background'] = "url(\"https://example.com/\") 1px 2px / 3px 4px space round local padding-box content-box, rgb(5, 6, 7) url(\"https://example.com/\") 1px 2px / 3px 4px space round local padding-box content-box" should not set unrelated longhands

Expand Up @@ -9,20 +9,41 @@
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/css/support/parsing-testcommon.js"></script>
<script src="/css/support/shorthand-testcommon.js"></script>
</head>
<body>
<script>
// Background serialization varies across browsers. https://github.com/w3c/csswg-drafts/issues/418

// Background serialization varies across browsers. https://github.com/w3c/csswg-drafts/issues/418
test_valid_value("background",
'url("https://example.com/") 1px 2px / 3px 4px space round local padding-box content-box, rgb(5, 6, 7) url("https://example.com/") 1px 2px / 3px 4px space round local padding-box content-box', [
'url("https://example.com/") 1px 2px / 3px 4px space round local padding-box content-box, rgb(5, 6, 7) url("https://example.com/") 1px 2px / 3px 4px space round local padding-box content-box', // spec
'url("https://example.com/") 1px 2px / 3px 4px space round local padding-box content-box, rgb(5, 6, 7) url("https://example.com/") 1px 2px / 3px 4px space round local padding-box content-box', // spec, WebKit
'url("https://example.com/") local space round 1px 2px / 3px 4px padding-box content-box, url("https://example.com/") local space round 1px 2px / 3px 4px padding-box content-box rgb(5, 6, 7)', // Edge
'url("https://example.com/") space round local 1px 2px / 3px 4px padding-box content-box, rgb(5, 6, 7) url("https://example.com/") space round local 1px 2px / 3px 4px padding-box content-box', // Firefox
'url("https://example.com/") 1px 2px / 3px 4px space round local padding-box content-box, url("https://example.com/") 1px 2px / 3px 4px space round local padding-box content-box rgb(5, 6, 7)', // Blink
'url(https://example.com/) 1px 2px / 3px 4px space round local padding-box content-box, url(https://example.com/) 1px 2px / 3px 4px space round local padding-box content-box rgb(5, 6, 7)' // WebKit omits quotes - https://bugs.webkit.org/show_bug.cgi?id=28869
'url("https://example.com/") 1px 2px / 3px 4px space round local padding-box content-box, url("https://example.com/") 1px 2px / 3px 4px space round local padding-box content-box rgb(5, 6, 7)' // Blink
]);

test_shorthand_value('background', 'none', {
'background-image': 'none',
'background-position': '0%',
'background-size': 'auto',
'background-repeat': 'repeat',
'background-attachment': 'scroll',
'background-origin': 'padding-box',
'background-clip': 'border-box',
'background-color': 'transparent',
})
test_shorthand_value('background', 'url("https://example.com/") 1px 2px / 3px 4px space round local padding-box content-box, rgb(5, 6, 7) url("https://example.com/") 1px 2px / 3px 4px space round local padding-box content-box', {
'background-image': 'url("https://example.com/"), url("https://example.com/")',
'background-position': '1px 2px, 1px 2px',
'background-size': '3px 4px, 3px 4px',
'background-repeat': 'space round, space round',
'background-attachment': 'local, local',
'background-origin': 'padding-box, padding-box',
'background-clip': 'content-box, content-box',
'background-color': 'rgb(5, 6, 7)',
})

</script>
</body>
</html>

0 comments on commit 5d2cf87

Please sign in to comment.