Skip to content

Commit 3db53cb

Browse files
asamuzaKasamuzaK
authored
Fix background shorthand handlers
Fixes #3169. Fixes #3663. Fixes #3944.
1 parent 678e840 commit 3db53cb

5 files changed

Lines changed: 144 additions & 75 deletions

File tree

‎lib/jsdom/living/css/helpers/shorthand-properties.js‎

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ const propertyDefinitions = require("../../../../generated/css-property-definiti
44
const { hasVarFunc, isGlobalKeyword, isValidPropertyValue, splitValue } = require("./css-values");
55
const background = require("../properties/background");
66
const backgroundColor = require("../properties/backgroundColor");
7-
const backgroundSize = require("../properties/backgroundSize");
87
const border = require("../properties/border");
98
const borderWidth = require("../properties/borderWidth");
109
const borderStyle = require("../properties/borderStyle");
@@ -213,31 +212,54 @@ function getPositionValue(positionValues, position) {
213212
*/
214213
function replaceBackgroundShorthand(property, properties) {
215214
const { value: propertyValue } = properties.get(property);
216-
const parsedValue = background.shorthandFor.get(property).parse(propertyValue);
217-
const values = splitValue(parsedValue, {
215+
const values = splitValue(propertyValue, {
218216
delimiter: ","
219217
});
220218
const { value: shorthandValue } = properties.get(background.property);
221219
const bgValues = background.parse(shorthandValue);
222220
const bgLength = bgValues.length;
223221
if (property === backgroundColor.property) {
224-
bgValues[bgLength - 1][property] = parsedValue[0];
222+
bgValues[bgLength - 1][property] = values[0];
225223
} else {
226224
for (let i = 0; i < bgLength; i++) {
227-
bgValues[i][property] = values[i];
225+
bgValues[i][property] = values[i] !== undefined ? values[i] : values[0];
228226
}
229227
}
230228
const backgrounds = [];
231229
for (const bgValue of bgValues) {
232230
const bg = [];
233-
for (const [longhand, value] of Object.entries(bgValue)) {
234-
if (!value || value === background.initialValues.get(longhand)) {
231+
let hasPosition = false;
232+
const originValue = bgValue["background-origin"];
233+
const clipValue = bgValue["background-clip"];
234+
const isDefaultBox =
235+
originValue === background.initialValues.get("background-origin") &&
236+
clipValue === background.initialValues.get("background-clip");
237+
for (const longhand of background.initialValues.keys()) {
238+
const value = bgValue[longhand];
239+
if (!value) {
235240
continue;
236241
}
237-
if (longhand === backgroundSize.property) {
238-
bg.push(`/ ${value}`);
239-
} else {
240-
bg.push(value);
242+
if (longhand === "background-origin") {
243+
if (!isDefaultBox) {
244+
bg.push(originValue);
245+
}
246+
} else if (longhand === "background-clip") {
247+
if (!isDefaultBox && originValue !== clipValue) {
248+
bg.push(clipValue);
249+
}
250+
} else if (value !== background.initialValues.get(longhand)) {
251+
if (longhand === "background-position") {
252+
hasPosition = true;
253+
bg.push(value);
254+
} else if (longhand === "background-size") {
255+
if (hasPosition) {
256+
bg.push(`/ ${value}`);
257+
} else {
258+
bg.push(background.initialValues.get("background-position"), `/ ${value}`);
259+
}
260+
} else {
261+
bg.push(value);
262+
}
241263
}
242264
}
243265
backgrounds.push(bg.join(" "));

‎lib/jsdom/living/css/properties/background.js‎

Lines changed: 98 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -67,25 +67,42 @@ const descriptor = {
6767
const backgrounds = [];
6868
for (const bgValue of bgValues) {
6969
const bg = [];
70-
for (const [longhand, value] of Object.entries(bgValue)) {
70+
let hasPosition = false;
71+
const originValue = bgValue[backgroundOrigin.property];
72+
const clipValue = bgValue[backgroundClip.property];
73+
const isDefaultBox =
74+
originValue === initialValues.get(backgroundOrigin.property) &&
75+
clipValue === initialValues.get(backgroundClip.property);
76+
for (const [longhand] of shorthandFor) {
77+
const value = bgValue[longhand];
7178
if (value) {
7279
const arr = bgMap.get(longhand);
7380
arr.push(value);
74-
bgMap.set(longhand, arr);
75-
if (value !== initialValues.get(longhand)) {
76-
if (longhand === backgroundSize.property) {
77-
bg.push(`/ ${value}`);
78-
} else {
79-
bg.push(value);
81+
if (longhand === backgroundOrigin.property) {
82+
if (!isDefaultBox) {
83+
bg.push(originValue);
8084
}
81-
} else if (longhand === backgroundImage.property) {
82-
if (v === "none") {
83-
bg.push(value);
85+
} else if (longhand === backgroundClip.property) {
86+
if (!isDefaultBox && originValue !== clipValue) {
87+
bg.push(clipValue);
8488
}
85-
} else if (longhand === backgroundColor.property) {
86-
if (v === "transparent") {
89+
} else if (value !== initialValues.get(longhand)) {
90+
if (longhand === backgroundPosition.property) {
91+
hasPosition = true;
92+
bg.push(value);
93+
} else if (longhand === backgroundSize.property) {
94+
if (hasPosition) {
95+
bg.push(`/ ${value}`);
96+
} else {
97+
bg.push(initialValues.get(backgroundPosition.property), `/ ${value}`);
98+
}
99+
} else {
87100
bg.push(value);
88101
}
102+
} else if (longhand === backgroundImage.property && v === "none") {
103+
bg.push(value);
104+
} else if (longhand === backgroundColor.property && v === "transparent") {
105+
bg.push(value);
89106
}
90107
}
91108
}
@@ -115,6 +132,9 @@ const descriptor = {
115132
let l = 0;
116133
for (const [longhand] of shorthandFor) {
117134
const val = this.getPropertyValue(longhand);
135+
if (!val || parsers.hasVarFunc(val)) {
136+
return "";
137+
}
118138
if (longhand === backgroundImage.property) {
119139
if (val === "none" && v === "none" && this.getPropertyValue(backgroundColor.property) === "transparent") {
120140
return val;
@@ -123,20 +143,19 @@ const descriptor = {
123143
const imgValues = parsers.splitValue(val, {
124144
delimiter: ","
125145
});
126-
l = imgValues.length;
146+
l = Math.max(l, imgValues.length);
127147
bgMap.set(longhand, imgValues);
128148
}
129149
} else if (longhand === backgroundColor.property) {
130150
if (val !== initialValues.get(longhand) || v.includes(val)) {
131151
bgMap.set(longhand, [val]);
132152
}
133153
} else if (val !== initialValues.get(longhand)) {
134-
bgMap.set(
135-
longhand,
136-
parsers.splitValue(val, {
137-
delimiter: ","
138-
})
139-
);
154+
const values = parsers.splitValue(val, {
155+
delimiter: ","
156+
});
157+
l = Math.max(l, values.length);
158+
bgMap.set(longhand, values);
140159
}
141160
}
142161
if (l === 0) {
@@ -149,53 +168,73 @@ const descriptor = {
149168
}
150169
const bgValues = [];
151170
for (let i = 0; i < l; i++) {
152-
bgValues[i] = [];
153-
}
154-
for (const [longhand, values] of bgMap) {
155-
for (let i = 0; i < l; i++) {
156-
switch (longhand) {
157-
case backgroundColor.property: {
158-
if (i === l - 1) {
159-
const value = values[0];
160-
if (parsers.hasVarFunc(value)) {
161-
return "";
162-
}
163-
if (value && value !== initialValues.get(longhand)) {
164-
const bgValue = bgValues[i];
165-
bgValue.push(value);
166-
}
167-
}
168-
break;
171+
const bg = [];
172+
let hasPosition = false;
173+
let originValue, clipValue;
174+
const originValues = bgMap.get(backgroundOrigin.property);
175+
const clipValues = bgMap.get(backgroundClip.property);
176+
if (originValues) {
177+
if (originValues[i] !== undefined) {
178+
originValue = originValues[i];
179+
} else {
180+
originValue = initialValues.get(backgroundOrigin.property);
181+
}
182+
} else {
183+
originValue = initialValues.get(backgroundOrigin.property);
184+
}
185+
if (clipValues) {
186+
if (clipValues[i] !== undefined) {
187+
clipValue = clipValues[i];
188+
} else {
189+
clipValue = initialValues.get(backgroundClip.property);
190+
}
191+
} else {
192+
clipValue = initialValues.get(backgroundClip.property);
193+
}
194+
const isDefaultBox =
195+
originValue === initialValues.get(backgroundOrigin.property) &&
196+
clipValue === initialValues.get(backgroundClip.property);
197+
for (const [longhand] of shorthandFor) {
198+
let value;
199+
if (bgMap.has(longhand)) {
200+
const values = bgMap.get(longhand);
201+
value = values[i] !== undefined ? values[i] : initialValues.get(longhand);
202+
} else {
203+
value = initialValues.get(longhand);
204+
}
205+
if (parsers.hasVarFunc(value)) {
206+
return "";
207+
}
208+
if (longhand === backgroundOrigin.property) {
209+
if (!isDefaultBox) {
210+
bg.push(originValue);
169211
}
170-
case backgroundSize.property: {
171-
const value = values[i];
172-
if (parsers.hasVarFunc(value)) {
173-
return "";
174-
}
175-
if (value && value !== initialValues.get(longhand)) {
176-
const bgValue = bgValues[i];
177-
bgValue.push(`/ ${value}`);
178-
}
179-
break;
212+
} else if (longhand === backgroundClip.property) {
213+
if (!isDefaultBox && originValue !== clipValue) {
214+
bg.push(clipValue);
180215
}
181-
default: {
182-
const value = values[i];
183-
if (parsers.hasVarFunc(value)) {
184-
return "";
185-
}
186-
if (value && value !== initialValues.get(longhand)) {
187-
const bgValue = bgValues[i];
188-
bgValue.push(value);
216+
} else if (longhand === backgroundColor.property) {
217+
if (i === l - 1 && (value !== initialValues.get(longhand) || bgMap.has(longhand))) {
218+
bg.push(value);
219+
}
220+
} else if (value !== initialValues.get(longhand)) {
221+
if (longhand === backgroundPosition.property) {
222+
hasPosition = true;
223+
bg.push(value);
224+
} else if (longhand === backgroundSize.property) {
225+
if (hasPosition) {
226+
bg.push(`/ ${value}`);
227+
} else {
228+
bg.push(initialValues.get(backgroundPosition.property), `/ ${value}`);
189229
}
230+
} else {
231+
bg.push(value);
190232
}
191233
}
192234
}
235+
bgValues.push(bg.join(" "));
193236
}
194-
const backgrounds = [];
195-
for (const bgValue of bgValues) {
196-
backgrounds.push(bgValue.join(" "));
197-
}
198-
return backgrounds.join(", ");
237+
return bgValues.join(", ");
199238
},
200239
enumerable: true,
201240
configurable: true

‎test/web-platform-tests/to-run.yaml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,6 @@ parsing/background-position-y-valid.html:
118118
"e.style['background-position-y'] = \"calc(10px - 0.5em), top -20%, bottom 10px\" should set the property value": [fail, Unknown]
119119
parsing/background-repeat-computed.html: [fail, CSS not implemented]
120120
parsing/background-shorthand-serialization.html:
121-
"background-size with non-initial background-position": [fail, Unknown]
122121
"multiple backgrounds with varying values": [fail, Unknown]
123122
"all initial values": [fail, Unknown]
124123
parsing/background-size-computed.html: [fail, CSS not implemented]

‎test/web-platform-tests/to-upstream-expectations.yaml‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
css/css-backgrounds/background-border-cssText.html: [fail, https://github.com/jsdom/jsdom/issues/4095]
2-
css/css-backgrounds/background-shorthand.html: [fail, https://github.com/jsdom/jsdom/issues/3663 https://github.com/jsdom/jsdom/issues/3169]
32
css/css-borders/border-color.html: [fail, https://github.com/jsdom/jsdom/issues/3972]
43
css/css-borders/border-shorthands.html:
54
"Computed value of border shorthand": [fail, Regression test for https://github.com/jsdom/jsdom/issues/3971]
@@ -19,7 +18,9 @@ css/cssom/getComputedStyle-shadow-host.html: [fail, https://github.com/jsdom/jsd
1918
css/cssom/getComputedStyle-shadow-inherit.html:
2019
"toggle shadowRoot.host visibility": [fail, https://github.com/jsdom/jsdom/issues/3941]
2120
css/cssom/shadow-dom-style-sheet.html: [fail, https://github.com/jsdom/jsdom/issues/3645]
22-
css/cssom/style-background-shorthand-overwrite.html: [fail, Need cssstyle fix; https://github.com/jsdom/jsdom/issues/3944]
21+
css/cssom/style-background-shorthand-overwrite.html:
22+
"overwrite, computed": [fail, Need to fix getComputedStyle()]
23+
"without overwrite, computed": [fail, Need to fix getComputedStyle()]
2324
css/cssom/style-set-custom-property-priority.html: [fail, Need to fix getComputedStyle; related https://github.com/jsdom/cssstyle/issues/225]
2425
css/mediaqueries/nested-media-queries.html: [fail, conditionText serialization doesn't include spaces]
2526
custom-elements/ElementInternals-accessibility.html:

‎test/web-platform-tests/to-upstream/css/cssom/style-background-shorthand-overwrite.html‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,20 @@
1515
test(() => {
1616
const div = document.getElementById("with-overwrite");
1717
assert_equals(div.style.backgroundPosition, "center top", "style");
18+
}, "overwrite, specified");
19+
20+
test(() => {
21+
const div = document.getElementById("with-overwrite");
1822
assert_equals(window.getComputedStyle(div).backgroundPosition, "50% 0%", "computed");
19-
}, "overwrite");
23+
}, "overwrite, computed");
2024

2125
test(() => {
2226
const div = document.getElementById("without-overwrite");
2327
assert_equals(div.style.backgroundPosition, "center top", "style");
28+
}, "without overwrite, specified");
29+
30+
test(() => {
31+
const div = document.getElementById("without-overwrite");
2432
assert_equals(window.getComputedStyle(div).backgroundPosition, "50% 0%", "computed");
25-
}, "without overwrite");
33+
}, "without overwrite, computed");
2634
</script>

0 commit comments

Comments
 (0)