Skip to content

Commit

Permalink
Make background shorthand default to border-box when border-area spec…
Browse files Browse the repository at this point in the history
…ified

https://bugs.webkit.org/show_bug.cgi?id=283308
rdar://140142957

Reviewed by Tim Nguyen.

If the origin is not specified, but clip is given as border-area,
set origin to border-box.

See w3c/csswg-drafts#11167

* LayoutTests/imported/w3c/web-platform-tests/css/css-backgrounds/parsing/background-shorthand-serialization-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-backgrounds/parsing/background-shorthand-serialization.html:
* 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:
* Source/WebCore/css/ShorthandSerializer.cpp:
(WebCore::ShorthandSerializer::serializeLayered const):
* Source/WebCore/css/parser/CSSPropertyParser.cpp:
(WebCore::CSSPropertyParser::consumeBackgroundShorthand):

Canonical link: https://commits.webkit.org/287521@main
  • Loading branch information
fantasai authored and mnutt committed Dec 8, 2024
1 parent 2535af1 commit aaf0660
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,6 @@ PASS multiple backgrounds
FAIL background-size with non-initial background-position assert_equals: expected "url(\"/favicon.ico\") 0% 0% / 10rem" but got "url(\"/favicon.ico\") 0% 0% / 10rem auto"
FAIL multiple backgrounds with varying values assert_equals: expected "url(\"/favicon.ico\") left top no-repeat, url(\"/favicon.ico\") center center / 100% 100% no-repeat, white url(\"/favicon.ico\")" but got "url(\"/favicon.ico\") left top no-repeat, url(\"/favicon.ico\") center center / 100% 100% no-repeat, url(\"/favicon.ico\") white"
PASS all initial values
PASS border-area border-box
PASS border-area padding-box

Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<html>
<head>
<meta charset="utf-8">
<title>CSS Background Shorthand Serialization Test: background shorthand should only serialize non-initial values</title>
<title>CSS Background Shorthand Specified Value Serialization Test: background shorthand should only serialize non-initial values</title>
<link rel="help" href="https://drafts.csswg.org/css-backgrounds/#background">
<meta name="assert" content="background shorthand should only serialize non-initial values">
<script src="/resources/testharness.js"></script>
Expand Down Expand Up @@ -43,4 +43,16 @@
element.style = `background: padding-box border-box;`;
assert_equals(element.style.background , 'none');
}, "all initial values");


test((t) => {
element.style = `background: border-area border-box;`;
assert_equals(element.style.background , 'border-area');
}, "border-area border-box");


test((t) => {
element.style = `background: border-area padding-box;`;
assert_equals(element.style.background , 'padding-box border-area');
}, "border-area padding-box");
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,22 @@ PASS e.style['background'] = "url(\"https://example.com/\") 1px 2px / 3px 4px sp
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
PASS e.style['background'] = "border-box content-box" should set background-attachment
PASS e.style['background'] = "border-box content-box" should set background-clip
PASS e.style['background'] = "border-box content-box" should set background-color
PASS e.style['background'] = "border-box content-box" should set background-image
PASS e.style['background'] = "border-box content-box" should set background-origin
PASS e.style['background'] = "border-box content-box" should set background-position
PASS e.style['background'] = "border-box content-box" should set background-repeat
PASS e.style['background'] = "border-box content-box" should set background-size
PASS e.style['background'] = "border-box content-box" should not set unrelated longhands
PASS e.style['background'] = "border-area" should set background-attachment
PASS e.style['background'] = "border-area" should set background-clip
PASS e.style['background'] = "border-area" should set background-color
PASS e.style['background'] = "border-area" should set background-image
PASS e.style['background'] = "border-area" should set background-origin
PASS e.style['background'] = "border-area" should set background-position
PASS e.style['background'] = "border-area" should set background-repeat
PASS e.style['background'] = "border-area" should set background-size
PASS e.style['background'] = "border-area" should not set unrelated longhands

Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,26 @@
'background-clip': 'content-box, content-box',
'background-color': 'rgb(5, 6, 7)',
})
test_shorthand_value('background', 'border-box content-box', {
'background-image': 'none',
'background-position': '0% 0%',
'background-size': 'auto',
'background-repeat': 'repeat',
'background-attachment': 'scroll',
'background-origin': 'border-box',
'background-clip': 'content-box',
'background-color': 'transparent',
})
test_shorthand_value('background', 'border-area', {
'background-image': 'none',
'background-position': '0% 0%',
'background-size': 'auto',
'background-repeat': 'repeat',
'background-attachment': 'scroll',
'background-origin': 'border-box',
'background-clip': 'border-area',
'background-color': 'transparent',
})

</script>
</body>
Expand Down
12 changes: 8 additions & 4 deletions Source/WebCore/css/ShorthandSerializer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -673,11 +673,15 @@ String ShorthandSerializer::serializeLayered() const
if (!layerValues.skip(j - 1) && !layerValues.skip(j))
layerValues.skip(j) = true;
} else if (!layerValues.skip(j - 1) || !layerValues.skip(j)) {
// If the two are different, both need to be serialized, except in the special case of no-clip.
if (layerValues.valueID(j) != CSSValueNoClip) {
layerValues.skip(j - 1) = false;
layerValues.skip(j) = false;
// If the two are different, both need to be serialized, unless clip is invalid as origin
if (layerValues.valueID(j) == CSSValueNoClip)
continue;
if (layerValues.valueID(j) == CSSValueBorderArea) {
layerValues.skip(j - 1) = layerValues.valueID(j - 1) == CSSValueBorderBox;
continue;
}
layerValues.skip(j - 1) = false;
layerValues.skip(j) = false;
}
}

Expand Down
7 changes: 7 additions & 0 deletions Source/WebCore/css/parser/CSSPropertyParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2015,6 +2015,7 @@ bool CSSPropertyParser::consumeBackgroundShorthand(const StylePropertyShorthand&
do {
bool parsedLonghand[10] = { false };
bool lastParsedWasPosition = false;
bool clipIsBorderArea = false;
RefPtr<CSSValue> originValue;
do {
bool foundProperty = false;
Expand Down Expand Up @@ -2057,6 +2058,8 @@ bool CSSPropertyParser::consumeBackgroundShorthand(const StylePropertyShorthand&
if (value) {
if (property == CSSPropertyBackgroundOrigin || property == CSSPropertyMaskOrigin)
originValue = value;
else if (property == CSSPropertyBackgroundClip)
clipIsBorderArea = value->valueID() == CSSValueBorderArea;
parsedLonghand[i] = true;
foundProperty = true;
longhands[i].append(value.releaseNonNull());
Expand All @@ -2082,6 +2085,10 @@ bool CSSPropertyParser::consumeBackgroundShorthand(const StylePropertyShorthand&
longhands[i].append(originValue.releaseNonNull());
continue;
}
if (clipIsBorderArea && (property == CSSPropertyBackgroundOrigin) && !parsedLonghand[i]) {
longhands[i].append(CSSPrimitiveValue::create(CSSValueBorderBox));
continue;
}
if (!parsedLonghand[i])
longhands[i].append(Ref { CSSPrimitiveValue::implicitInitialValue() });
}
Expand Down

0 comments on commit aaf0660

Please sign in to comment.