Merge pull request #85 from orangemug/fix/issue-73-disabled-events

Fix for issue #73 disabled events (work in progress)

lib/delegate.js

@@ -30,6 +30,9 @@ function Delegate(root) {
 
   /** @type function() */
   this.handle = Delegate.prototype.handle.bind(this);
+
+  // Cache of event listeners removed during an event cycle
+  this._removedListeners = [];
 }
 
 /**
@@ -240,6 +243,7 @@ Delegate.prototype.off = function(eventType, selector, handler, useCapture) {
     listener = listenerList[i];
 
     if ((!selector || selector === listener.selector) && (!handler || handler === listener.handler)) {
+      this._removedListeners.push(listener);
       listenerList.splice(i, 1);
     }
   }
@@ -300,6 +304,8 @@ Delegate.prototype.handle = function(event) {
     break;
   }
 
+  var toFire = [];
+
   // Need to continuously check
   // that the specific list is
   // still populated in case one
@@ -318,23 +324,22 @@ Delegate.prototype.handle = function(event) {
         break;
       }
 
+      if(
+        target.tagName &&
+        ["button", "input", "select", "textarea"].indexOf(target.tagName.toLowerCase()) > -1 &&
+        target.hasAttribute("disabled")
+      ) {
+        // Remove things that have previously fired
+        toFire = [];
+      }
       // Check for match and fire
       // the event if there's one
       //
       // TODO:MCG:20120117: Need a way
       // to check if event#stopImmediatePropagation
       // was called. If so, break both loops.
-      if (listener.matcher.call(target, listener.matcherParam, target)) {
-        returned = this.fire(event, target, listener);
-      }
-
-      // Stop propagation to subsequent
-      // callbacks if the callback returned
-      // false
-      if (returned === false) {
-        event[EVENTIGNORE] = true;
-        event.preventDefault();
-        return;
+      else if (listener.matcher.call(target, listener.matcherParam, target)) {
+        toFire.push([event, target, listener]);
       }
     }
 
@@ -357,6 +362,28 @@ Delegate.prototype.handle = function(event) {
       break;
     }
   }
+
+  var ret;
+
+  for(i=0; i<toFire.length; i++) {
+    // Has it been removed during while the event function was fired
+    if(this._removedListeners.indexOf(toFire[i][2]) > -1) {
+      continue;
+    }
+    returned = this.fire.apply(this, toFire[i]);
+
+    // Stop propagation to subsequent
+    // callbacks if the callback returned
+    // false
+    if (returned === false) {
+      toFire[i][0][EVENTIGNORE] = true;
+      toFire[i][0].preventDefault();
+      ret = false;
+      break;
+    }
+  }
+
+  return ret;
 };
 
 /**

test/tests/delegateTest.js

@@ -23,6 +23,8 @@ setupHelper.setUp = function() {
       + '<circle id="svg-delegate-test-clickable" cx="60" cy="60" r="50"/>'
       + '<use id="svg-delegate-test-mouseover" href="#svg-delegate-test-clickable" x="100" fill="blue"/>'
     + '</svg>'
+    + '<button disabled="true" id="btn-disabled">Normal</button>'
+    + '<button disabled="true" id="btn-disabled-alt"><i id="btn-disabled-alt-label">With label</i></button>'
   );
 };
 
@@ -708,6 +710,27 @@ buster.testCase('Delegate', {
     assert.calledOnce(spyOnElement);
   },
 
+  'Disabled buttons don\'t trigger click' : function() {
+    var delegate = new Delegate(document);
+    var spy = this.spy();
+
+    delegate.on('click', '#btn-disabled', spy);
+    var element = document.getElementById("btn-disabled");
+
+    setupHelper.fireMouseEvent(element, "click");
+    refute.called(spy);
+  },
+  'Disabled buttons with inner element don\'t trigger click' : function() {
+    var delegate = new Delegate(document);
+    var spy = this.spy();
+
+    delegate.on('click', '#btn-disabled-alt-label', spy);
+    var element = document.getElementById("btn-disabled-alt-label");
+
+    setupHelper.fireMouseEvent(element, "click");
+    refute.called(spy);
+  },
+
   'tearDown': function() {
     setupHelper.tearDown();
   }