Repository: jsdom/cssstyle
Pull request #165 (2023-11-10 01:20:04)

Handle empty string and null for margin and padding subparts
Per the linked issue in [jsdom](https://github.com/jsdom/jsdom/issues/2504), the following bug exists, where `dom` is a JSDom instance:

```
const elem = dom.window.document.createElement('a');

elem.style.marginTop = '10px';
console.log(elem.style.marginTop); // '10px'

elem.style.marginTop = '';
console.log(elem.style.marginTop); // Expected '', but '10px' received.
```

This PR fixes this bug by specifying that `null` and `empty string` are valid values for subparts of the properties `margin` and `padding`. I've also added tests for the behavior of setting these properties to `undefined` (value should remain as is).

Verified that this behavior (using the repro above) matches the following browser's behavior:

Chrome 119.0.6045.123
Edge 120.0.2186.2
Firefox 120.0b8
Safari 17.1 (19616.2.9.11.7)

Fixes jsdom/jsdom#2504 


Patch:
diff --git a/lib/parsers.js b/lib/parsers.js
index d866eed..109fbe3 100644
--- a/lib/parsers.js
+++ b/lib/parsers.js
@@ -683,6 +683,9 @@ exports.subImplicitSetter = function (prefix, part, isValid, parser) {
     if (typeof v === 'number') {
       v = v.toString();
     }
+    if (v === null) {
+      v = '';
+    }
     if (typeof v !== 'string') {
       return undefined;
     }
diff --git a/lib/properties/margin.js b/lib/properties/margin.js
index 37e25ba..3a42cd7 100644
--- a/lib/properties/margin.js
+++ b/lib/properties/margin.js
@@ -9,6 +9,7 @@ var isValid = function (v) {
   }
   var type = parsers.valueType(v);
   return (
+    type === TYPES.NULL_OR_EMPTY_STR ||
     type === TYPES.LENGTH ||
     type === TYPES.PERCENT ||
     (type === TYPES.INTEGER && (v === '0' || v === 0))
@@ -40,6 +41,9 @@ module.exports.definition = {
     if (typeof v === 'number') {
       v = String(v);
     }
+    if (v === null) {
+      v = '';
+    }
     if (typeof v !== 'string') {
       return;
     }
diff --git a/lib/properties/padding.js b/lib/properties/padding.js
index 1285154..3848913 100644
--- a/lib/properties/padding.js
+++ b/lib/properties/padding.js
@@ -6,6 +6,7 @@ var TYPES = parsers.TYPES;
 var isValid = function (v) {
   var type = parsers.valueType(v);
   return (
+    type === TYPES.NULL_OR_EMPTY_STR ||
     type === TYPES.LENGTH ||
     type === TYPES.PERCENT ||
     (type === TYPES.INTEGER && (v === '0' || v === 0))
@@ -33,6 +34,9 @@ module.exports.definition = {
     if (typeof v === 'number') {
       v = String(v);
     }
+    if (v === null) {
+      v = '';
+    }
     if (typeof v !== 'string') {
       return;
     }


Test patch:
diff --git a/lib/CSSStyleDeclaration.test.js b/lib/CSSStyleDeclaration.test.js
index 32a0c83..e7464e0 100644
--- a/lib/CSSStyleDeclaration.test.js
+++ b/lib/CSSStyleDeclaration.test.js
@@ -402,6 +402,43 @@ describe('CSSStyleDeclaration', () => {
     expect(style.marginTop).toEqual('0.5ex');
   });
 
+  test('setting empty string and null to a padding or margin works', () => {
+    var style = new CSSStyleDeclaration();
+    var parts = ['Top', 'Right', 'Bottom', 'Left'];
+    function testParts(base, nullValue) {
+      var props = [base].concat(parts.map((part) => base + part));
+      for (let prop of props) {
+        expect(style[prop]).toEqual('');
+        style[prop] = '10px';
+        expect(style[prop]).toEqual('10px');
+        style[prop] = nullValue;
+        expect(style[prop]).toEqual('');
+      }
+    }
+
+    testParts('margin', '');
+    testParts('margin', null);
+    testParts('padding', '');
+    testParts('padding', null);
+  });
+
+  test('setting undefined to a padding or margin does nothing', () => {
+    var style = new CSSStyleDeclaration();
+    var parts = ['Top', 'Right', 'Bottom', 'Left'];
+    function testParts(base) {
+      var props = [base].concat(parts.map((part) => base + part));
+      for (let prop of props) {
+        style[prop] = '10px';
+        expect(style[prop]).toEqual('10px');
+        style[prop] = undefined;
+        expect(style[prop]).toEqual('10px');
+      }
+    }
+
+    testParts('margin');
+    testParts('padding');
+  });
+
   test('setting null to background works', () => {
     var style = new CSSStyleDeclaration();
     style.background = 'red';