Skip to content

Commit 1e8a7ff

Browse files
domenicclaude
domenic
and
claude
committed
Handle global keywords in CSS shorthand property handlers
Several shorthand property handlers (border, background, and their sub-shorthands) crashed or produced incorrect results when encountering CSS-wide keywords like "inherit" or "initial". The root cause is that `parse()` functions return strings for global keywords, but many callers assumed object/array returns. - In `processBorderProperties`, handle global keywords uniformly at the top of the loop before any property-specific branching, fixing `cssText = "border-top: inherit"` (and all border side/line shorthands) which previously threw a TypeError swallowed by the cssText setter's catch block. - In `matchesBorderShorthandValue`, return false for non-object parse results, fixing `border: inherit` followed by setting a longhand like `borderTopWidth`. - In `prepareBorderStringValue` and `prepareBorderObjectValue`, guard `replacePositionValue` calls against global keyword inputs, fixing `border: inherit` followed by setting a sub-shorthand like `borderTop`. - In `background.parse()`, return global keywords as strings (matching `border.parse()` behavior). Update the descriptor setter to propagate them to all longhands and the getter to verify longhand consistency before returning. - In `prepareProperties`, skip `replaceBackgroundShorthand` for global keyword shorthands, and fix a pre-existing Map mutation-during-iteration bug by updating `parsedProperties` instead of `properties`. - Mark `cssom-setProperty-shorthand.html` as passing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 0b79509 commit 1e8a7ff

5 files changed

Lines changed: 103 additions & 9 deletions

File tree

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

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,9 @@ function replaceBackgroundShorthand(property, properties) {
255255
*/
256256
function matchesBorderShorthandValue(property, value, shorthandValue) {
257257
const obj = border.parse(shorthandValue);
258+
if (typeof obj !== "object") {
259+
return false;
260+
}
258261
if (Object.hasOwn(obj, property)) {
259262
return value === obj[property];
260263
}
@@ -551,7 +554,11 @@ function prepareBorderStringValue({ property, value, priority, properties, parts
551554
shorthandItem.value = "";
552555
}
553556
if (lineItem.value) {
554-
lineItem.value = replacePositionValue(propertyValue, splitValue(lineItem.value), prop2);
557+
if (isGlobalKeyword(lineItem.value)) {
558+
lineItem.value = "";
559+
} else {
560+
lineItem.value = replacePositionValue(propertyValue, splitValue(lineItem.value), prop2);
561+
}
555562
}
556563
if (positionItem.value && !matchesBorderShorthandValue(lineProperty, propertyValue, positionItem.value)) {
557564
positionItem.value = "";
@@ -810,11 +817,23 @@ function prepareBorderObjectValue({ property, value, priority, properties, parts
810817
value[longhandProperty] :
811818
border.initialValues.get(`${prop1}-${line}`);
812819
if (line === WIDTH && lineWidthItem.value) {
813-
lineWidthItem.value = replacePositionValue(itemValue, splitValue(lineWidthItem.value), prop2);
820+
if (isGlobalKeyword(lineWidthItem.value)) {
821+
lineWidthItem.value = "";
822+
} else {
823+
lineWidthItem.value = replacePositionValue(itemValue, splitValue(lineWidthItem.value), prop2);
824+
}
814825
} else if (line === STYLE && lineStyleItem.value) {
815-
lineStyleItem.value = replacePositionValue(itemValue, splitValue(lineStyleItem.value), prop2);
826+
if (isGlobalKeyword(lineStyleItem.value)) {
827+
lineStyleItem.value = "";
828+
} else {
829+
lineStyleItem.value = replacePositionValue(itemValue, splitValue(lineStyleItem.value), prop2);
830+
}
816831
} else if (line === COLOR && lineColorItem.value) {
817-
lineColorItem.value = replacePositionValue(itemValue, splitValue(lineColorItem.value), prop2);
832+
if (isGlobalKeyword(lineColorItem.value)) {
833+
lineColorItem.value = "";
834+
} else {
835+
lineColorItem.value = replacePositionValue(itemValue, splitValue(lineColorItem.value), prop2);
836+
}
818837
}
819838
longhandItem.value = itemValue;
820839
longhandItem.priority = priority;
@@ -1216,6 +1235,10 @@ function processBorderProperties(borders) {
12161235
for (const [property, item] of borders) {
12171236
if (shorthandProperties.has(property)) {
12181237
const { value, priority } = item;
1238+
if (isGlobalKeyword(value)) {
1239+
longhandProperties.set(property, createPropertyItem(property, value, priority));
1240+
continue;
1241+
}
12191242
if (property === border.property) {
12201243
const lineItems = border.parse(value);
12211244
for (const [key, initialValue] of border.initialValues) {
@@ -1366,10 +1389,9 @@ function prepareProperties(properties) {
13661389
parsedProperties.set(property, createPropertyItem(property, value, priority));
13671390
if (hasPrecedingBackground) {
13681391
const { value: shorthandValue, priority: shorthandPriority } = properties.get(background.property);
1369-
if ((!shorthandPriority || priority) && !hasVarFunc(shorthandValue)) {
1392+
if ((!shorthandPriority || priority) && !hasVarFunc(shorthandValue) && !isGlobalKeyword(shorthandValue)) {
13701393
const replacedShorthandValue = replaceBackgroundShorthand(property, parsedProperties);
1371-
properties.delete(background.property);
1372-
properties.set(
1394+
parsedProperties.set(
13731395
background.property,
13741396
createPropertyItem(background.property, replacedShorthandValue, shorthandPriority)
13751397
);

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,13 @@ const descriptor = {
4444
this._setProperty(property, v);
4545
} else {
4646
const bgValues = parse(v);
47+
if (typeof bgValues === "string") {
48+
for (const [key] of shorthandFor) {
49+
this._setProperty(key, bgValues);
50+
}
51+
this._setProperty(property, bgValues);
52+
return;
53+
}
4754
if (!Array.isArray(bgValues)) {
4855
return;
4956
}
@@ -96,6 +103,14 @@ const descriptor = {
96103
if (parsers.hasVarFunc(v)) {
97104
return v;
98105
}
106+
if (parsers.isGlobalKeyword(v)) {
107+
for (const [longhand] of shorthandFor) {
108+
if (this.getPropertyValue(longhand) !== v) {
109+
return "";
110+
}
111+
}
112+
return v;
113+
}
99114
const bgMap = new Map();
100115
let l = 0;
101116
for (const [longhand] of shorthandFor) {
@@ -193,7 +208,7 @@ const descriptor = {
193208
* @returns {Array<object>|undefined} The parsed background values or undefined if invalid.
194209
*/
195210
function parse(v) {
196-
if (v === "") {
211+
if (v === "" || parsers.isGlobalKeyword(v)) {
197212
return v;
198213
} else if (parsers.hasCalcFunc(v)) {
199214
v = parsers.resolveCalc(v);

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,6 @@ css-style-reparse.html: [fail, Unknown]
420420
cssom-fontfacerule-constructors.html: [fail, Unknown]
421421
cssom-fontfacerule.html: [fail, No CSSFontFaceDescriptors implementation]
422422
cssom-getPropertyValue-common-checks.html: [fail, Depends on CSS]
423-
cssom-setProperty-shorthand.html: [fail, Unknown]
424423
cssstyledeclaration-all-shorthand.html: [fail, Unknown]
425424
cssstyledeclaration-cssfontrule.tentative.html: [fail, Unknown]
426425
cssstyledeclaration-csstext-all-shorthand.html: [fail, Unknown]

‎test/web-platform-tests/to-upstream/css/css-backgrounds/background-shorthand-keyword.html‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<!DOCTYPE html>
2+
<meta charset="utf-8">
3+
<title>background shorthand containing keywords</title>
4+
<link rel="help" href="https://drafts.csswg.org/css-backgrounds/#propdef-background">
5+
<script src="/resources/testharness.js"></script>
6+
<script src="/resources/testharnessreport.js"></script>
7+
8+
<script>
9+
"use strict";
10+
11+
test(() => {
12+
const div = document.createElement("div");
13+
div.style.cssText = "background: inherit";
14+
assert_equals(div.style.background, "inherit", "background");
15+
assert_equals(div.style.cssText, "background: inherit;", "cssText");
16+
}, "background: inherit set via cssText");
17+
18+
test(() => {
19+
const div = document.createElement("div");
20+
div.style.cssText = "background: inherit; background-color: red";
21+
assert_equals(div.style.backgroundColor, "red", "background-color");
22+
assert_equals(div.style.background, "", "background");
23+
}, "background: inherit then background-color override via cssText");
24+
</script>

‎test/web-platform-tests/to-upstream/css/css-borders/border-shorthand-keyword.html‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,38 @@
2525
assert_equals(div.style.borderBottom, "", "border-bottom");
2626
assert_equals(div.style.borderColor, "rgb(117, 117, 117)", "border-color");
2727
}, "border related properties with var()");
28+
29+
test(() => {
30+
const div = document.createElement("div");
31+
div.style.border = "inherit";
32+
div.style.borderTopWidth = "medium";
33+
assert_equals(div.style.border, "", "border shorthand should clear");
34+
assert_equals(div.style.borderTop, "", "border-top should clear");
35+
assert_equals(div.style.borderTopWidth, "medium", "border-top-width");
36+
}, "border shorthand clears when longhand is set to its initial value");
37+
38+
test(() => {
39+
const div = document.createElement("div");
40+
div.style.border = "inherit";
41+
div.style.borderTop = "1px solid red";
42+
assert_equals(div.style.border, "", "border");
43+
assert_equals(div.style.borderWidth, "", "border-width");
44+
assert_equals(div.style.borderStyle, "", "border-style");
45+
assert_equals(div.style.borderColor, "", "border-color");
46+
assert_equals(div.style.borderTop, "1px solid red", "border-top");
47+
}, "border shorthand clears when position shorthand overrides inherited border");
48+
49+
test(() => {
50+
const div = document.createElement("div");
51+
div.style.cssText = "border: inherit";
52+
assert_equals(div.style.border, "inherit", "border");
53+
assert_equals(div.style.cssText, "border: inherit;", "cssText");
54+
}, "border: inherit set via cssText");
55+
56+
test(() => {
57+
const div = document.createElement("div");
58+
div.style.cssText = "border-top: inherit";
59+
assert_equals(div.style.borderTop, "inherit", "border-top");
60+
assert_equals(div.style.cssText, "border-top: inherit;", "cssText");
61+
}, "border-top: inherit set via cssText");
2862
</script>

0 commit comments

Comments
 (0)