Fix a bug when clearing event listeners.

We weren't deleting the reference to the removed listener, so we were leaking
memory and returning a stale value on a subsequent get. Also, we now stash the
capture value when setting an event listener, so that we can automatically
remove or replace a listener without the caller needing to specify capture.

d3.v2.js

@@ -1972,10 +1972,20 @@ d3_selectionPrototype.on = function(type, listener, capture) {
 
   // remove the old event listener, and add the new event listener
   return this.each(function(d, i) {
-    var node = this;
+    var node = this,
+        o = node[name];
 
-    if (node[name]) node.removeEventListener(type, node[name], capture);
-    if (listener) node.addEventListener(type, node[name] = l, capture);
+    // remove the old listener, if any (using the previously-set capture)
+    if (o) {
+      node.removeEventListener(type, o, o.$);
+      delete node[name];
+    }
+
+    // add the new listener, if any (remembering the capture flag)
+    if (listener) {
+      node.addEventListener(type, node[name] = l, l.$ = capture);
+      l._ = listener; // stash the unwrapped listener for get
+    }
 
     // wrapped event listener that preserves i
     function l(e) {
@@ -1987,9 +1997,6 @@ d3_selectionPrototype.on = function(type, listener, capture) {
         d3.event = o;
       }
     }
-
-    // stash the unwrapped listener for retrieval
-    l._ = listener;
   });
 };
 d3_selectionPrototype.each = function(callback) {

src/core/selection-on.js

@@ -12,10 +12,20 @@ d3_selectionPrototype.on = function(type, listener, capture) {
 
   // remove the old event listener, and add the new event listener
   return this.each(function(d, i) {
-    var node = this;
+    var node = this,
+        o = node[name];
 
-    if (node[name]) node.removeEventListener(type, node[name], capture);
-    if (listener) node.addEventListener(type, node[name] = l, capture);
+    // remove the old listener, if any (using the previously-set capture)
+    if (o) {
+      node.removeEventListener(type, o, o.$);
+      delete node[name];
+    }
+
+    // add the new listener, if any (remembering the capture flag)
+    if (listener) {
+      node.addEventListener(type, node[name] = l, l.$ = capture);
+      l._ = listener; // stash the unwrapped listener for get
+    }
 
     // wrapped event listener that preserves i
     function l(e) {
@@ -27,8 +37,5 @@ d3_selectionPrototype.on = function(type, listener, capture) {
         d3.event = o;
       }
     }
-
-    // stash the unwrapped listener for retrieval
-    l._ = listener;
   });
 };

test/core/selection-on-test.js

@@ -30,7 +30,18 @@ suite.addBatch({
       form.on("submit", null);
       form.append("input").attr("type", "submit").node().click();
       assert.equal(fail, 0);
+      assert.isUndefined(form.on("submit"));
     },
+    /* Regrettably, JSDOM ignores the capture flag, so we can't test this yet…
+    "removing a listener doesn't require the capture flag": function(body) {
+      var form = body.append("form"), fail = 0;
+      form.on("submit", function() { ++fail; }, true);
+      form.on("submit", null);
+      form.append("input").attr("type", "submit").node().click();
+      assert.equal(fail, 0);
+      assert.isUndefined(form.on("submit"));
+    },
+    */
     "ignores removal of non-matching event listener": function(body) {
       body.append("form").on("submit", null);
     },
@@ -42,8 +53,7 @@ suite.addBatch({
       assert.equal(foo, 1);
       assert.equal(bar, 1);
     },
-    /*
-    Not really sure how to test this one…
+    /* Not really sure how to test this one…
     "observes the specified capture flag": function(body) {
     },
     */