Repository: jsdom/cssstyle
Pull request #147 (2021-12-12 13:34:38)

fix: skip onchange callback if serialized value did not change
fixes jsdom/jsdom#3305

According to the spec, [setting a css declaration](https://drafts.csswg.org/cssom/#set-a-css-declaration) "must return true if the serialization of declarations was changed as result of the steps. It may return false otherwise". This information is used when [calling `setProperty`](https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-setproperty), "if updated is true, update style attribute for the CSS declaration block."


At the same time for [`cssText` property](https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-csstext), there are no conditions for setting the new value



Patch:
diff --git a/lib/CSSStyleDeclaration.js b/lib/CSSStyleDeclaration.js
index d74e739..870b553 100644
--- a/lib/CSSStyleDeclaration.js
+++ b/lib/CSSStyleDeclaration.js
@@ -19,6 +19,7 @@ var CSSStyleDeclaration = function CSSStyleDeclaration(onChangeCallback) {
   this._importants = {};
   this._length = 0;
   this._onChange = onChangeCallback;
+  this._setInProgress = false;
 };
 CSSStyleDeclaration.prototype = {
   constructor: CSSStyleDeclaration,
@@ -73,6 +74,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);
@@ -87,7 +89,7 @@ CSSStyleDeclaration.prototype = {
     }
     this._values[name] = value;
     this._importants[name] = priority;
-    if (this._onChange) {
+    if (this._onChange && this.cssText !== originalText && !this._setInProgress) {
       this._onChange(this.cssText);
     }
   },
@@ -196,6 +198,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 +209,7 @@ Object.defineProperties(CSSStyleDeclaration.prototype, {
           dummyRule.getPropertyPriority(name)
         );
       }
+      this._setInProgress = false;
       if (this._onChange) {
         this._onChange(this.cssText);
       }


Test patch:
diff --git a/lib/CSSStyleDeclaration.test.js b/lib/CSSStyleDeclaration.test.js
index bed5047..6828fa4 100644
--- a/lib/CSSStyleDeclaration.test.js
+++ b/lib/CSSStyleDeclaration.test.js
@@ -461,10 +461,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', () => {