From 65b18d1cd90fa174358f21b74062a83431a29e64 Mon Sep 17 00:00:00 2001
From: Simon Kalt <ska@kialo.com>
Date: Wed, 26 Apr 2023 15:33:28 +0200
Subject: [PATCH] Properly update margin/padding properties when removing
 individual ones

---
 lib/CSSStyleDeclaration.test.js | 54 +++++++++++++++++++++++++++++++++
 lib/parsers.js                  | 18 +++++------
 2 files changed, 63 insertions(+), 9 deletions(-)

diff --git a/lib/CSSStyleDeclaration.test.js b/lib/CSSStyleDeclaration.test.js
index 9d12b45..3127770 100644
--- a/lib/CSSStyleDeclaration.test.js
+++ b/lib/CSSStyleDeclaration.test.js
@@ -363,6 +363,60 @@ describe('CSSStyleDeclaration', () => {
     }
   });
 
+  test('removing and setting individual margin properties updates the combined property accordingly', () => {
+    var style = new CSSStyleDeclaration();
+    style.margin = '1px 2px 3px 4px';
+
+    style.marginTop = '';
+    expect(style.margin).toEqual('');
+    expect(style.marginRight).toEqual('2px');
+    expect(style.marginBottom).toEqual('3px');
+    expect(style.marginLeft).toEqual('4px');
+
+    style.marginBottom = '';
+    expect(style.margin).toEqual('');
+    expect(style.marginRight).toEqual('2px');
+    expect(style.marginLeft).toEqual('4px');
+
+    style.marginBottom = '5px';
+    expect(style.margin).toEqual('');
+    expect(style.marginRight).toEqual('2px');
+    expect(style.marginBottom).toEqual('5px');
+    expect(style.marginLeft).toEqual('4px');
+
+    style.marginTop = '6px';
+    expect(style.cssText).toEqual('margin: 6px 2px 5px 4px;');
+  });
+
+  test.each(['padding', 'margin'])(
+    'removing an individual %s property should remove the combined property and replace it with the remaining individual ones',
+    (property) => {
+      var style = new CSSStyleDeclaration();
+      var parts = ['Top', 'Right', 'Bottom', 'Left'];
+      var partValues = ['1px', '2px', '3px', '4px'];
+
+      for (var j = 0; j < parts.length; j++) {
+        var partToRemove = parts[j];
+        style[property] = partValues.join(' ');
+        style[property + partToRemove] = '';
+
+        // Main property should have been removed
+        expect(style[property]).toEqual('');
+
+        // Expect other parts to still be there
+        for (var k = 0; k < parts.length; k++) {
+          var propertyCss = property + '-' + parts[k].toLowerCase() + ': ' + partValues[k] + ';';
+          if (k === j) {
+            expect(style[property + parts[k]]).toEqual('');
+            expect(style.cssText).not.toContain(propertyCss);
+          } else {
+            expect(style[property + parts[k]]).toEqual(partValues[k]);
+            expect(style.cssText).toContain(propertyCss);
+          }
+        }
+      }
+  });
+
   test('setting a value to 0 should return the string value', () => {
     var style = new CSSStyleDeclaration();
     style.setProperty('fill-opacity', 0);
diff --git a/lib/parsers.js b/lib/parsers.js
index 109fbe3..13bd751 100644
--- a/lib/parsers.js
+++ b/lib/parsers.js
@@ -694,19 +694,19 @@ exports.subImplicitSetter = function (prefix, part, isValid, parser) {
     }
     v = parser(v);
     this._setProperty(property, v);
-    var parts = [];
-    for (var i = 0; i < 4; i++) {
-      if (this._values[subparts[i]] == null || this._values[subparts[i]] === '') {
-        break;
-      }
-      parts.push(this._values[subparts[i]]);
-    }
-    if (parts.length === 4) {
-      for (i = 0; i < 4; i++) {
+
+    var parts = subparts.map(subpart => this._values[subpart]);
+    if (parts.every(p => p !== '' && p != null)) {
+      for (var i = 0; i < subparts.length; i++) {
         this.removeProperty(subparts[i]);
         this._values[subparts[i]] = parts[i];
       }
       this._setProperty(prefix, parts.join(' '));
+    } else {
+      this.removeProperty(prefix);
+      for (var i = 0; i < subparts.length; i++) {
+        this._setProperty(subparts[i], parts[i]);
+      }
     }
     return v;
   };