Skip to content

Commit 14747c5

Browse files
smfrmagomez
smfr
authored and
magomez
committed
background-repeat is incorrectly exposed through inline style
https://bugs.webkit.org/show_bug.cgi?id=243915 Reviewed by Antti Koivisto. Remove the hack that treated `background-repeat` and `mask-repeat` as shorthands for -x and -y longhands, since no such longhands are specified. This hack leaked out via `element.style.length`: setting a `background-repeat` value would internally set `background-repeat-x` and `background-repeat-y`, but then we'd consider those internal, and end up reporting `element.style.length` as zero. This broke the mechanism used by https://css3test.com/ to detect support for style properties. `background-repeat` has custom serialization behavior: `repeat no-repeat` is serialized as `repeat-x` for example. To support this, introduce a new CSSValue type CSSBackgroundRepeatValue. CSSPropertyBackgroundRepeatX/Y and CSSPropertyMaskRepeatX/Y are gone, and various bits of code treating CSSPropertyBackgroundRepeat and CSSPropertyMaskRepeat as shorthands is removed. FillLayer stores a FillRepeatXY struct with x and y members. * LayoutTests/fast/backgrounds/repeat/parsing-background-repeat-expected.txt: * LayoutTests/fast/backgrounds/repeat/parsing-background-repeat.html: Add some tests for `style.length`. Adjust for new serialization behavior. * LayoutTests/fast/css/background-layers-initial-size.html: Apply style to just the test div. * LayoutTests/fast/css/remove-shorthand-expected.txt: Ordering changed. * LayoutTests/fast/masking/parsing-mask-expected.txt: * LayoutTests/fast/masking/parsing-mask-repeat.html: Added. * LayoutTests/fast/masking/parsing-mask.html: Remove unnecessary font size style. `repeat no-repeat` serializes as `repeat-x` now. * LayoutTests/fast/masking/parsing-webkit-mask-expected.txt: * LayoutTests/fast/masking/parsing-webkit-mask.html: Remove unnecessary font size style. `repeat no-repeat` serializes as `repeat-x` now. * Source/WebCore/Sources.txt: * Source/WebCore/WebCore.xcodeproj/project.pbxproj: * Source/WebCore/animation/CSSPropertyAnimation.cpp: (WebCore::CSSPropertyAnimationWrapperMap::CSSPropertyAnimationWrapperMap): * Source/WebCore/css/CSSBackgroundRepeatValue.cpp: Added. (WebCore::CSSBackgroundRepeatValue::CSSBackgroundRepeatValue): (WebCore::CSSBackgroundRepeatValue::customCSSText const): (WebCore::CSSBackgroundRepeatValue::equals const): * Source/WebCore/css/CSSBackgroundRepeatValue.h: Added. * Source/WebCore/css/CSSComputedStyleDeclaration.cpp: (WebCore::fillRepeatToCSSValue): (WebCore::ComputedStyleExtractor::valueForPropertyInStyle): * Source/WebCore/css/CSSProperties.json: * Source/WebCore/css/CSSToStyleMap.cpp: (WebCore::CSSToStyleMap::mapFillRepeat): (WebCore::CSSToStyleMap::mapFillRepeatX): Deleted. (WebCore::CSSToStyleMap::mapFillRepeatY): Deleted. * Source/WebCore/css/CSSToStyleMap.h: * Source/WebCore/css/CSSValue.cpp: (WebCore::CSSValue::equals const): (WebCore::CSSValue::cssText const): (WebCore::CSSValue::destroy): * Source/WebCore/css/CSSValue.h: (WebCore::CSSValue::isBackgrouneRepeatValue const): * Source/WebCore/css/StyleProperties.cpp: (WebCore::StyleProperties::getPropertyValue const): (WebCore::StyleProperties::getLayeredShorthandValue const): (WebCore::StyleProperties::asTextInternal const): * Source/WebCore/css/parser/CSSParserFastPaths.cpp: (WebCore::CSSParserFastPaths::isValidKeywordPropertyAndValue): * Source/WebCore/css/parser/CSSPropertyParser.cpp: (WebCore::consumeRepeatStyleComponent): (WebCore::consumeRepeatStyle): (WebCore::consumeBackgroundComponent): (WebCore::CSSPropertyParser::parseSingleValue): (WebCore::CSSPropertyParser::consumeBackgroundShorthand): (WebCore::CSSPropertyParser::parseShorthand): * Source/WebCore/display/css/DisplayFillLayerImageGeometry.cpp: (WebCore::Display::geometryForLayer): * Source/WebCore/page/FrameView.cpp: (WebCore::FrameView::calculateExtendedBackgroundMode const): * Source/WebCore/rendering/RenderBoxModelObject.cpp: (WebCore::RenderBoxModelObject::calculateBackgroundImageGeometry const): * Source/WebCore/rendering/style/FillLayer.cpp: (WebCore::FillLayer::FillLayer): (WebCore::FillLayer::operator=): (WebCore::FillLayer::operator== const): (WebCore::FillLayer::fillUnsetProperties): (WebCore::FillLayer::hasRepeatXY const): (WebCore::operator<<): * Source/WebCore/rendering/style/FillLayer.h: (WebCore::FillRepeatXY::operator== const): (WebCore::FillLayer::repeat const): (WebCore::FillLayer::isRepeatSet const): (WebCore::FillLayer::setRepeat): (WebCore::FillLayer::clearRepeat): (WebCore::FillLayer::initialFillRepeat): (WebCore::FillLayer::repeatX const): Deleted. (WebCore::FillLayer::repeatY const): Deleted. (WebCore::FillLayer::isRepeatXSet const): Deleted. (WebCore::FillLayer::isRepeatYSet const): Deleted. (WebCore::FillLayer::setRepeatX): Deleted. (WebCore::FillLayer::setRepeatY): Deleted. (WebCore::FillLayer::clearRepeatX): Deleted. (WebCore::FillLayer::clearRepeatY): Deleted. (WebCore::FillLayer::initialFillRepeatX): Deleted. (WebCore::FillLayer::initialFillRepeatY): Deleted. * Source/WebCore/rendering/style/RenderStyle.cpp: (WebCore::RenderStyle::isIdempotentTextAutosizingCandidate const): * Source/WebCore/rendering/style/RenderStyle.h: (WebCore::RenderStyle::backgroundRepeat const): (WebCore::RenderStyle::maskRepeat const): (WebCore::RenderStyle::setBackgroundRepeat): (WebCore::RenderStyle::setMaskRepeat): (WebCore::RenderStyle::backgroundRepeatX const): Deleted. (WebCore::RenderStyle::backgroundRepeatY const): Deleted. (WebCore::RenderStyle::maskRepeatX const): Deleted. (WebCore::RenderStyle::maskRepeatY const): Deleted. (WebCore::RenderStyle::setBackgroundRepeatX): Deleted. (WebCore::RenderStyle::setBackgroundRepeatY): Deleted. (WebCore::RenderStyle::setMaskRepeatX): Deleted. (WebCore::RenderStyle::setMaskRepeatY): Deleted. Canonical link: https://commits.webkit.org/253440@main
1 parent 7d2a4ff commit 14747c5

32 files changed

Lines changed: 408 additions & 297 deletions

‎LayoutTests/fast/backgrounds/repeat/parsing-background-repeat-expected.txt‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,28 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
44

55

66
PASS test("background-repeat: repeat-x;") is "repeat-x"
7+
PASS testPropertyCount("background-repeat: repeat-x;") is 1
8+
PASS test("background-repeat: repeat-x, repeat-y;") is "repeat-x, repeat-y"
9+
PASS testPropertyCount("background-repeat: repeat-x;") is 1
10+
PASS test("background-image: url(test1.gif); background-repeat: repeat-x;") is "repeat-x"
11+
PASS testPropertyCount("background-image: url(test1.gif); background-repeat: repeat-x;") is 2
12+
PASS test("background-image: url(test1.gif), url(test1.gif); background-repeat: repeat-x;") is "repeat-x"
713
PASS test("background-repeat: repeat-y;") is "repeat-y"
814
PASS test("background-repeat: repeat;") is "repeat"
915
PASS test("background-repeat: no-repeat;") is "no-repeat"
1016
PASS test("background-repeat: round;") is "round"
1117
PASS test("background-repeat: space;") is "space"
12-
PASS test("background-repeat: repeat repeat;") is "repeat repeat"
18+
PASS test("background-repeat: repeat repeat;") is "repeat"
1319
PASS test("background-repeat: no-repeat space;") is "no-repeat space"
14-
PASS test("background-repeat: round round;") is "round round"
20+
PASS test("background-repeat: round round;") is "round"
1521
PASS test("background-repeat: space repeat;") is "space repeat"
1622
PASS test("background: purple url(resources/gradient.gif) repeat-x top left") is "repeat-x"
1723
PASS test("background: purple url(resources/gradient.gif) repeat-y 50% 50%") is "repeat-y"
1824
PASS test("background: purple url(resources/gradient.gif) repeat center") is "repeat"
1925
PASS test("background: purple url(resources/gradient.gif) no-repeat 12px") is "no-repeat"
2026
PASS test("background: purple url(resources/gradient.gif) round left 50px") is "round"
2127
PASS test("background: purple url(resources/gradient.gif) space 25px 25px") is "space"
28+
PASS testPropertyCount("background: purple url(resources/gradient.gif) space 25px 25px;") is 9
2229
PASS test("background-repeat: 45;") is ""
2330
PASS test("background-repeat: coconut;") is ""
2431
PASS successfullyParsed is true

‎LayoutTests/fast/backgrounds/repeat/parsing-background-repeat.html‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,37 @@
1818
return result;
1919
}
2020

21+
function testPropertyCount(value)
22+
{
23+
var div = document.createElement("div");
24+
div.setAttribute("style", value);
25+
document.body.appendChild(div);
26+
27+
var result = div.style.length;
28+
document.body.removeChild(div);
29+
return result;
30+
}
31+
2132
shouldBe('test("background-repeat: repeat-x;")', '"repeat-x"');
33+
shouldBe('testPropertyCount("background-repeat: repeat-x;")', '1');
34+
35+
shouldBe('test("background-repeat: repeat-x, repeat-y;")', '"repeat-x, repeat-y"');
36+
shouldBe('testPropertyCount("background-repeat: repeat-x;")', '1');
37+
38+
shouldBe('test("background-image: url(test1.gif); background-repeat: repeat-x;")', '"repeat-x"');
39+
shouldBe('testPropertyCount("background-image: url(test1.gif); background-repeat: repeat-x;")', '2');
40+
41+
shouldBe('test("background-image: url(test1.gif), url(test1.gif); background-repeat: repeat-x;")', '"repeat-x"');
42+
2243
shouldBe('test("background-repeat: repeat-y;")', '"repeat-y"');
2344
shouldBe('test("background-repeat: repeat;")', '"repeat"');
2445
shouldBe('test("background-repeat: no-repeat;")', '"no-repeat"');
2546
shouldBe('test("background-repeat: round;")', '"round"');
2647
shouldBe('test("background-repeat: space;")', '"space"');
2748

28-
shouldBe('test("background-repeat: repeat repeat;")', '"repeat repeat"');
49+
shouldBe('test("background-repeat: repeat repeat;")', '"repeat"');
2950
shouldBe('test("background-repeat: no-repeat space;")', '"no-repeat space"');
30-
shouldBe('test("background-repeat: round round;")', '"round round"');
51+
shouldBe('test("background-repeat: round round;")', '"round"');
3152
shouldBe('test("background-repeat: space repeat;")', '"space repeat"');
3253

3354
shouldBe('test("background: purple url(resources/gradient.gif) repeat-x top left")', '"repeat-x"');
@@ -37,6 +58,8 @@
3758
shouldBe('test("background: purple url(resources/gradient.gif) round left 50px")', '"round"');
3859
shouldBe('test("background: purple url(resources/gradient.gif) space 25px 25px")', '"space"');
3960

61+
shouldBe('testPropertyCount("background: purple url(resources/gradient.gif) space 25px 25px;")', '9');
62+
4063
shouldBe('test("background-repeat: 45;")', '""');
4164
shouldBe('test("background-repeat: coconut;")', '""');
4265
</script>

‎LayoutTests/fast/css/background-layers-initial-size.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
<head>
44
<script src="../../resources/js-test-pre.js"></script>
55
<style>
6-
div {
6+
#testDiv {
77
background: linear-gradient(45deg, rgba(0,0,0,.4) 50%, transparent 0) 100% 0 / 25px 25px no-repeat,
88
linear-gradient(-135deg, transparent 18px, yellowgreen 0);
99

‎LayoutTests/fast/css/remove-shorthand-expected.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ Test for http://bugs.webkit.org/show_bug.cgi?id=9284 Quirksmode (CSS1): Removing
33
Starting with a declaration containing all properties that are constituents of shortcuts, see what is removed when a shortcut property is removed. The shortcut’s constituents and only them should be removed.
44

55
Removing background
6-
removes "background-image, background-attachment, background-color, background-position, background-repeat"
6+
removes "background-image, background-repeat, background-attachment, background-color, background-position"
77
and adds "".
88
Removing background-position
99
removes "background-position"
@@ -54,7 +54,7 @@ Removing margin
5454
removes "margin"
5555
and adds "".
5656
Removing -webkit-mask
57-
removes "mask-image, -webkit-mask-position-x, -webkit-mask-position-y, mask-repeat-x, mask-repeat-y, -webkit-mask"
57+
removes "mask-image, -webkit-mask-position-x, -webkit-mask-position-y, mask-repeat, -webkit-mask"
5858
and adds "".
5959
Removing -webkit-mask-position
6060
removes "-webkit-mask-position-x, -webkit-mask-position-y, -webkit-mask"

‎LayoutTests/fast/inspector-support/style-expected.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ background-image: initial (original property was background)
44
background-position-x: initial (original property was background)
55
background-position-y: initial (original property was background)
66
background-size: initial (original property was background)
7+
background-repeat: initial (original property was background)
78
background-attachment: initial (original property was background)
89
background-origin: initial (original property was background)
910
background-clip: initial (original property was background)

‎LayoutTests/fast/masking/parsing-mask-expected.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ PASS innerStyle("mask", "space") is "space"
5656
PASS innerStyle("mask", "no-repeat") is "no-repeat"
5757
PASS innerStyle("mask", "repeat space") is "repeat space"
5858
PASS innerStyle("mask", "repeat round") is "repeat round"
59-
PASS innerStyle("mask", "repeat no-repeat") is "repeat no-repeat"
59+
PASS innerStyle("mask", "repeat no-repeat") is "repeat-x"
6060
PASS innerStyle("mask", "repeat space, repeat-x") is "repeat space, repeat-x"
6161
PASS innerStyle("mask", "repeat none") is "none repeat"
6262
PASS innerStyle("mask", "none repeat") is "none repeat"

‎LayoutTests/fast/masking/parsing-mask-repeat-expected.txt‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
This tests checks that all of the input values for mask-repeat parse correctly.
2+
3+
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
4+
5+
6+
PASS test("mask-repeat: repeat-x;") is "repeat-x"
7+
PASS testPropertyCount("mask-repeat: repeat-x;") is 1
8+
PASS test("mask-repeat: repeat-x, repeat-y;") is "repeat-x, repeat-y"
9+
PASS testPropertyCount("mask-repeat: repeat-x;") is 1
10+
PASS test("mask-image: url(test1.gif); mask-repeat: repeat-x;") is "repeat-x"
11+
PASS testPropertyCount("mask-image: url(test1.gif); mask-repeat: repeat-x;") is 2
12+
PASS test("mask-image: url(test1.gif), url(test1.gif); mask-repeat: repeat-x;") is "repeat-x"
13+
PASS test("mask-repeat: repeat-y;") is "repeat-y"
14+
PASS test("mask-repeat: repeat;") is "repeat"
15+
PASS test("mask-repeat: no-repeat;") is "no-repeat"
16+
PASS test("mask-repeat: round;") is "round"
17+
PASS test("mask-repeat: space;") is "space"
18+
PASS test("mask-repeat: repeat repeat;") is "repeat"
19+
PASS test("mask-repeat: no-repeat space;") is "no-repeat space"
20+
PASS test("mask-repeat: round round;") is "round"
21+
PASS test("mask-repeat: space repeat;") is "space repeat"
22+
PASS test("mask: url(resources/gradient.gif) repeat-x top left") is "repeat-x"
23+
PASS test("mask: url(resources/gradient.gif) repeat-y 50% 50%") is "repeat-y"
24+
PASS test("mask: url(resources/gradient.gif) repeat center") is "repeat"
25+
PASS test("mask: url(resources/gradient.gif) no-repeat 12px") is "no-repeat"
26+
PASS test("mask: url(resources/gradient.gif) round left 50px") is "round"
27+
PASS test("mask: url(resources/gradient.gif) space 25px 25px") is "space"
28+
PASS testPropertyCount("mask: url(resources/gradient.gif) space 25px 25px;") is 9
29+
PASS test("mask-repeat: 45;") is ""
30+
PASS test("mask-repeat: coconut;") is ""
31+
PASS successfullyParsed is true
32+
33+
TEST COMPLETE
34+

‎LayoutTests/fast/masking/parsing-mask-repeat.html‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<script src="../../resources/js-test-pre.js"></script>
5+
</head>
6+
<body>
7+
<script>
8+
description("This tests checks that all of the input values for mask-repeat parse correctly.");
9+
10+
function test(value)
11+
{
12+
var div = document.createElement("div");
13+
div.setAttribute("style", value);
14+
document.body.appendChild(div);
15+
16+
var result = div.style.getPropertyValue("mask-repeat");
17+
document.body.removeChild(div);
18+
return result;
19+
}
20+
21+
function testPropertyCount(value)
22+
{
23+
var div = document.createElement("div");
24+
div.setAttribute("style", value);
25+
document.body.appendChild(div);
26+
27+
var result = div.style.length;
28+
document.body.removeChild(div);
29+
return result;
30+
}
31+
32+
shouldBe('test("mask-repeat: repeat-x;")', '"repeat-x"');
33+
shouldBe('testPropertyCount("mask-repeat: repeat-x;")', '1');
34+
35+
shouldBe('test("mask-repeat: repeat-x, repeat-y;")', '"repeat-x, repeat-y"');
36+
shouldBe('testPropertyCount("mask-repeat: repeat-x;")', '1');
37+
38+
shouldBe('test("mask-image: url(test1.gif); mask-repeat: repeat-x;")', '"repeat-x"');
39+
shouldBe('testPropertyCount("mask-image: url(test1.gif); mask-repeat: repeat-x;")', '2');
40+
41+
shouldBe('test("mask-image: url(test1.gif), url(test1.gif); mask-repeat: repeat-x;")', '"repeat-x"');
42+
43+
shouldBe('test("mask-repeat: repeat-y;")', '"repeat-y"');
44+
shouldBe('test("mask-repeat: repeat;")', '"repeat"');
45+
shouldBe('test("mask-repeat: no-repeat;")', '"no-repeat"');
46+
shouldBe('test("mask-repeat: round;")', '"round"');
47+
shouldBe('test("mask-repeat: space;")', '"space"');
48+
49+
shouldBe('test("mask-repeat: repeat repeat;")', '"repeat"');
50+
shouldBe('test("mask-repeat: no-repeat space;")', '"no-repeat space"');
51+
shouldBe('test("mask-repeat: round round;")', '"round"');
52+
shouldBe('test("mask-repeat: space repeat;")', '"space repeat"');
53+
54+
shouldBe('test("mask: url(resources/gradient.gif) repeat-x top left")', '"repeat-x"');
55+
shouldBe('test("mask: url(resources/gradient.gif) repeat-y 50% 50%")', '"repeat-y"');
56+
shouldBe('test("mask: url(resources/gradient.gif) repeat center")', '"repeat"');
57+
shouldBe('test("mask: url(resources/gradient.gif) no-repeat 12px")', '"no-repeat"');
58+
shouldBe('test("mask: url(resources/gradient.gif) round left 50px")', '"round"');
59+
shouldBe('test("mask: url(resources/gradient.gif) space 25px 25px")', '"space"');
60+
61+
shouldBe('testPropertyCount("mask: url(resources/gradient.gif) space 25px 25px;")', '9');
62+
63+
shouldBe('test("mask-repeat: 45;")', '""');
64+
shouldBe('test("mask-repeat: coconut;")', '""');
65+
</script>
66+
<script src="../../resources/js-test-post.js"></script>
67+
</body>
68+
</html>

‎LayoutTests/fast/masking/parsing-mask.html‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
11
<!DOCTYPE html>
22
<html>
3-
<style>
4-
* { font-size: 16px; }
5-
div { font-size: 8px; }
6-
</style>
73
<body>
84
<script src="../../resources/js-test-pre.js"></script>
95
<script>
@@ -101,7 +97,7 @@
10197
testInner("mask", "no-repeat", "no-repeat");
10298
testInner("mask", "repeat space", "repeat space");
10399
testInner("mask", "repeat round", "repeat round");
104-
testInner("mask", "repeat no-repeat", "repeat no-repeat");
100+
testInner("mask", "repeat no-repeat", "repeat-x");
105101
testInner("mask", "repeat space, repeat-x", "repeat space, repeat-x");
106102
testInner("mask", "repeat none", "none repeat");
107103
testInner("mask", "none repeat", "none repeat");

‎LayoutTests/fast/masking/parsing-webkit-mask-expected.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ PASS innerStyle("-webkit-mask", "space") is "space"
5858
PASS innerStyle("-webkit-mask", "no-repeat") is "no-repeat"
5959
PASS innerStyle("-webkit-mask", "repeat space") is "repeat space"
6060
PASS innerStyle("-webkit-mask", "repeat round") is "repeat round"
61-
PASS innerStyle("-webkit-mask", "repeat no-repeat") is "repeat no-repeat"
61+
PASS innerStyle("-webkit-mask", "repeat no-repeat") is "repeat-x"
6262
PASS innerStyle("-webkit-mask", "repeat space, repeat-x") is "repeat space, repeat-x"
6363
PASS innerStyle("-webkit-mask", "repeat none") is "none repeat"
6464
PASS innerStyle("-webkit-mask", "none repeat") is "none repeat"

0 commit comments

Comments
 (0)