Land: fixes for DOMAttrModified mutation events (Felix Gnass)

lib/jsdom/level2/events.js

@@ -4,6 +4,7 @@
  *
  */
 var core = require("./core").dom.level2.core,
+    utils = require("../utils"),
     sys = require("sys");
 
 var events = {};
@@ -314,29 +315,8 @@ function mutationEventsEnabled(el) {
          getDocument(el).implementation.hasFeature('MutationEvents');
 }
 
-function advise(clazz, method, advice) {
-    var proto = clazz.prototype,
-        impl = proto[method];
-
-  proto[method] = function() {
-    var args = Array.prototype.slice.call(arguments);
-    var ret = impl.apply(this, arguments);
-    args.unshift(ret);
-    return advice.apply(this, args) || ret;
-  };
-}
-
-function adviseBefore(clazz, method, advice) {
-  var proto = clazz.prototype,
-      impl = proto[method];
-
-  proto[method] = function() {
-    advice.apply(this, arguments);
-    return impl.apply(this, arguments);
-  };
-}
-
-function dispatchInsertionEvent(ret, newChild, refChild) {
+utils.intercept(core.Node, 'insertBefore', function(_super, args, newChild, refChild) {
+  var ret = _super.apply(this, args);
   if (mutationEventsEnabled(this)) {
     var doc = getDocument(this),
         ev = doc.createEvent("MutationEvents");
@@ -354,9 +334,10 @@ function dispatchInsertionEvent(ret, newChild, refChild) {
       });
     }
   }
-}
+  return ret;
+});
 
-function dispatchRemovalEvent(oldChild) {
+utils.intercept(core.Node, 'removeChild', function (_super, args, oldChild) {
   if (mutationEventsEnabled(this)) {
     var doc = getDocument(this),
         ev = doc.createEvent("MutationEvents");
@@ -373,31 +354,34 @@ function dispatchRemovalEvent(oldChild) {
       }
     });
   }
-}
+  return _super.apply(this, args);
+});
 
-function dispatchAttrEvent(change, arg) {
-  return function(ret) {
+function dispatchAttrEvent(change) {
+  return function(_super, args, node) {
     var target = this._parentNode,
-        node = arguments[arg];
+        prev = _super.apply(this, args);
 
     if (mutationEventsEnabled(target)) {
       var doc = target._ownerDocument,
           attrChange = events.MutationEvent.prototype[change],
-          prevVal = arg == 0 ? node.value : null,
-          newVal = arg == 1 ? node.value : null,
-          ev = doc.createEvent("MutationEvents");
+          attrName = prev && prev.name || node.name,
+          prevVal = prev && prev.value || null,
+          newVal = change == 'ADDITION' ? node.value : null,
+          ev;
 
-      ev.initMutationEvent("DOMAttrModified", true, false, target, prevVal, newVal, node.name, attrChange);
-      target.dispatchEvent(ev);
+      if (!newVal || newVal != prevVal) {
+        ev = doc.createEvent("MutationEvents");
+        ev.initMutationEvent("DOMAttrModified", true, false, target, prevVal, newVal, attrName, attrChange);
+        target.dispatchEvent(ev);
+      }
     }
+    return prev;
   }
 }
 
-advise(core.Node, 'insertBefore', dispatchInsertionEvent);
-adviseBefore(core.Node, 'removeChild', dispatchRemovalEvent);
-
-advise(core.AttrNodeMap, 'removeNamedItem', dispatchAttrEvent('REMOVAL', 0));
-advise(core.AttrNodeMap, 'setNamedItem', dispatchAttrEvent('ADDITION', 1));
+utils.intercept(core.AttrNodeMap, 'removeNamedItem', dispatchAttrEvent('REMOVAL'));
+utils.intercept(core.AttrNodeMap, 'setNamedItem', dispatchAttrEvent('ADDITION'));
 
 core.CharacterData.prototype.__defineGetter__("_nodeValue", function() {
   return this.__nodeValue;

lib/jsdom/level2/style.js

@@ -1,5 +1,6 @@
 var core = require("./core").dom.level2.core,
     html = require("./html").dom.level2.html,
+    utils = require("../utils"),
     cssom = require("cssom"),
     assert = require('assert');
 
@@ -128,8 +129,6 @@ function evaluateStyleAttribute(data) {
   // {state: 'name'}, so instead we just build a dummy sheet.
   var styleSheet = cssom.parse('dummy{' + data + '}');
   var style = this.style;
-  //console.log('evaluating style on ' + this.tagName + ': ' + data + '  ->')
-  //console.log(styleSheet);
   while (style.length) {
     style.removeProperty(style[0]);
   }
@@ -167,13 +166,12 @@ StyleAttr.prototype.__proto__ = core.Attr.prototype;
  * Overwrite core.AttrNodeMap#setNamedItem to create a StyleAttr instance
  * instead of a core.Attr if the name equals 'style'.
  */
-var _setNamedItem = core.AttrNodeMap.prototype.setNamedItem;
-core.AttrNodeMap.prototype.setNamedItem = function(arg) {
-  if (arg.name == 'style') {
-    arg = new StyleAttr(this._parentNode, arg.nodeValue);
+utils.intercept(core.AttrNodeMap, 'setNamedItem', function(_super, args, attr) {
+  if (attr.name == 'style') {
+    attr = new StyleAttr(this._parentNode, attr.nodeValue);
   }
-  return _setNamedItem.call(this, arg);
-};
+  return _super.call(this, attr);
+});
 
 /**
  * Lazily create a CSSStyleDeclaration.

lib/jsdom/utils.js

@@ -0,0 +1,27 @@
+/**
+ * Intercepts a method by replacing the prototype's implementation
+ * with a wrapper that invokes the given interceptor instead.
+ * 
+ *     utils.intercept(core.Element, 'inserBefore',
+ *       function(_super, args, newChild, refChild) {
+ *         console.log('insertBefore', newChild, refChild);
+ *         return _super.apply(this, args);
+ *       }
+ *     );
+ */
+exports.intercept = function(clazz, method, interceptor) {
+  var proto = clazz.prototype,
+      _super = proto[method],
+      unwrapArgs = interceptor.length > 2;
+
+  proto[method] = function() {
+    if (unwrapArgs) {
+      var args = Array.prototype.slice.call(arguments);
+      args.unshift(_super, arguments);
+      return interceptor.apply(this, args);
+    }
+    else {
+      return interceptor.call(this, _super, arguments);
+    }
+  };
+};

test/jsdom/index.js

@@ -599,7 +599,27 @@ exports.tests = {
     h2.firstChild.nodeValue = 'bar';
     test.equal(h2.innerHTML, text, 'ChactaterData changes should be captured');
 
-    test.done()
+    var event;
+    h2.setAttribute('class', 'foo');
+    document.addEventListener('DOMAttrModified', function(ev) {
+      event = ev;
+    });
+    h2.setAttribute('class', 'bar');
+    test.ok(!!event, 'Changing an attribute should trigger DOMAttrModified');
+    test.equal(event.attrName, 'class', 'attrName should be class');
+    test.equal(event.prevValue, 'foo', 'prevValue should be foo');
+    test.equal(event.newValue, 'bar', 'newValue should be bar');
+
+    event = false;
+    h2.setAttribute('class', 'bar');
+    test.ok(!event, 'Setting the same value again should not trigger an event');
+
+    h2.removeAttribute('class');
+    test.ok(!!event, 'Removing an attribute should trigger DOMAttrModified');
+    test.equal(event.attrName, 'class', 'attrName should be class');
+    test.equal(event.prevValue, 'bar', 'prevValue should be bar');
+
+    test.done();
   },
 
   remove_listener_in_handler: function(test) {