Skip to content

Commit 75ddc28

Browse files
mbostockmbostock
committed
Fix a bug when removing a shared listener.
Previously, each listener function had a private _on property which recorded whether the listener was enabled; this was set to false when the listener was removed. (This is necessary because of the copy-on-write semantics when the array of listeners is modified while events are being dispatched.) If the listener was registered for multiple event types ("foo" and "bar") then removing the listener for any type would disable it for all types. The fix is to wrap the listener in an object. This also has better encapsulation since the state is kept private.
1 parent 5d57fa7 commit 75ddc28

4 files changed

Lines changed: 22 additions & 14 deletions

File tree

‎d3.js‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -506,24 +506,22 @@ function d3_dispatch_event() {
506506
i = -1,
507507
n = z.length,
508508
l;
509-
while (++i < n) if ((l = z[i])._on) l.apply(this, arguments);
509+
while (++i < n) if (l = z[i].on) l.apply(this, arguments);
510510
}
511511

512512
dispatch.on = function(name, listener) {
513513
var l, i;
514514

515-
// remove the old listener, if any
515+
// remove the old listener, if any (with copy-on-write)
516516
if (l = listenerByName[name]) {
517-
l._on = false;
517+
l.on = null;
518518
listeners = listeners.slice(0, i = listeners.indexOf(l)).concat(listeners.slice(i + 1));
519519
delete listenerByName[name];
520520
}
521521

522522
// add the new listener, if any
523523
if (listener) {
524-
listener._on = true;
525-
listeners.push(listener);
526-
listenerByName[name] = listener;
524+
listeners.push(listenerByName[name] = {on: listener});
527525
}
528526

529527
return dispatch;

‎d3.min.js‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/core/dispatch.js‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,24 +30,22 @@ function d3_dispatch_event() {
3030
i = -1,
3131
n = z.length,
3232
l;
33-
while (++i < n) if ((l = z[i])._on) l.apply(this, arguments);
33+
while (++i < n) if (l = z[i].on) l.apply(this, arguments);
3434
}
3535

3636
dispatch.on = function(name, listener) {
3737
var l, i;
3838

39-
// remove the old listener, if any
39+
// remove the old listener, if any (with copy-on-write)
4040
if (l = listenerByName[name]) {
41-
l._on = false;
41+
l.on = null;
4242
listeners = listeners.slice(0, i = listeners.indexOf(l)).concat(listeners.slice(i + 1));
4343
delete listenerByName[name];
4444
}
4545

4646
// add the new listener, if any
4747
if (listener) {
48-
listener._on = true;
49-
listeners.push(listener);
50-
listenerByName[name] = listener;
48+
listeners.push(listenerByName[name] = {on: listener});
5149
}
5250

5351
return dispatch;

‎test/core/dispatch-test.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,18 @@ suite.addBatch({
7272
d.foo();
7373
assert.deepEqual(those, [b]);
7474
},
75+
"removing a shared listener only affects the desired event": function(dispatch) {
76+
var d = dispatch("foo", "bar"), a = 0;
77+
function A() { ++a; }
78+
d.on("foo", A);
79+
d.on("bar", A);
80+
d.foo();
81+
d.bar();
82+
assert.equal(a, 2);
83+
d.on("foo", null);
84+
d.bar();
85+
assert.equal(a, 3);
86+
},
7587
"adding an existing listener has no effect": function(dispatch) {
7688
var d = dispatch("foo"), events = 0;
7789
function A() { ++events; }

0 commit comments

Comments
 (0)