fix: skip onchange callback if serialized value did not change

fixes jsdom/jsdom#3305
jsdom · Dec 12, 2021 · cbdc83a · cbdc83a
1 parent b527ed7
commit cbdc83a
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 1 deletion.
diff --git a/lib/CSSStyleDeclaration.js b/lib/CSSStyleDeclaration.js
@@ -18,6 +18,7 @@ var CSSStyleDeclaration = function CSSStyleDeclaration(onChangeCallback) {
   this._values = {};
   this._importants = {};
   this._length = 0;
+  this._setInProgress = false;
   this._onChange =
     onChangeCallback ||
     function() {
@@ -77,6 +78,7 @@ CSSStyleDeclaration.prototype = {
       this.removeProperty(name);
       return;
     }
+    var originalText = this.cssText;
     if (this._values[name]) {
       // Property already exist. Overwrite it.
       var index = Array.prototype.indexOf.call(this, name);
@@ -91,7 +93,9 @@ CSSStyleDeclaration.prototype = {
     }
     this._values[name] = value;
     this._importants[name] = priority;
-    this._onChange(this.cssText);
+    if (this.cssText !== originalText && !this._setInProgress) {
+      this._onChange(this.cssText);
+    }
   },
 
   /**
@@ -196,6 +200,7 @@ Object.defineProperties(CSSStyleDeclaration.prototype, {
         // malformed css, just return
         return;
       }
+      this._setInProgress = true;
       var rule_length = dummyRule.length;
       var name;
       for (i = 0; i < rule_length; ++i) {
@@ -206,6 +211,7 @@ Object.defineProperties(CSSStyleDeclaration.prototype, {
           dummyRule.getPropertyPriority(name)
         );
       }
+      this._setInProgress = false;
       this._onChange(this.cssText);
     },
     enumerable: true,

diff --git a/lib/CSSStyleDeclaration.test.js b/lib/CSSStyleDeclaration.test.js
@@ -353,10 +353,46 @@ describe('CSSStyleDeclaration', () => {
   });
 
   test('onchange callback should be called when the csstext changes', () => {
+    var called = 0;
     var style = new CSSStyleDeclaration(function(cssText) {
+      called++;
       expect(cssText).toEqual('opacity: 0;');
     });
+    style.cssText = 'opacity: 0;';
+    expect(called).toEqual(1);
+    style.cssText = 'opacity: 0;';
+    expect(called).toEqual(2);
+  });
+
+  test('onchange callback should be called only once when multiple properties were added', () => {
+    var called = 0;
+    var style = new CSSStyleDeclaration(function(cssText) {
+      called++;
+      expect(cssText).toEqual('width: 100px; height: 100px;');
+    });
+    style.cssText = 'width: 100px;height:100px;';
+    expect(called).toEqual(1);
+  });
+
+  test('onchange callback should not be called when property is set to the same value', () => {
+    var called = 0;
+    var style = new CSSStyleDeclaration(function() {
+      called++;
+    });
+
+    style.setProperty('opacity', 0);
+    expect(called).toEqual(1);
     style.setProperty('opacity', 0);
+    expect(called).toEqual(1);
+  });
+
+  test('onchange callback should not be called when removeProperty was called on non-existing property', () => {
+    var called = 0;
+    var style = new CSSStyleDeclaration(function() {
+      called++;
+    });
+    style.removeProperty('opacity');
+    expect(called).toEqual(0);
   });
 
   test('setting float should work the same as cssfloat', () => {