diff --git a/src/mixins/observable.mixin.js b/src/mixins/observable.mixin.js index b5411abb..f4657553 100644 --- a/src/mixins/observable.mixin.js +++ b/src/mixins/observable.mixin.js @@ -9,12 +9,12 @@ if (!this.__eventListeners[eventName]) { return; } - + var eventListener = this.__eventListeners[eventName]; if (handler) { - fabric.util.removeFromArray(this.__eventListeners[eventName], handler); + eventListener[eventListener.indexOf(handler)] = false; } else { - this.__eventListeners[eventName].length = 0; + fabric.util.array.fill(eventListener, false); } } @@ -65,7 +65,9 @@ // remove all key/value pairs (event name -> event handler) if (arguments.length === 0) { - this.__eventListeners = { }; + for (eventName in this.__eventListeners) { + _removeEventListener.call(this, eventName); + } } // one object with key/value pairs was passed else if (arguments.length === 1 && typeof arguments[0] === 'object') { @@ -100,9 +102,11 @@ } for (var i = 0, len = listenersForEvent.length; i < len; i++) { - // avoiding try/catch for perf. reasons - listenersForEvent[i].call(this, options || { }); + listenersForEvent[i] && listenersForEvent[i].call(this, options || { }); } + this.__eventListeners[eventName] = listenersForEvent.filter(function(value) { + return value !== false; + }); return this; } diff --git a/src/util/lang_array.js b/src/util/lang_array.js index 96603a64..72a7ab71 100644 --- a/src/util/lang_array.js +++ b/src/util/lang_array.js @@ -211,6 +211,17 @@ }); } + /** + * @private + */ + function fill(array, value) { + var k = array.length; + while (k--) { + array[k] = value; + } + return array; + } + /** * @private */ @@ -242,6 +253,7 @@ * @namespace fabric.util.array */ fabric.util.array = { + fill: fill, invoke: invoke, min: min, max: max diff --git a/test/unit/observable.js b/test/unit/observable.js index bb6f00e4..74508a5b 100644 --- a/test/unit/observable.js +++ b/test/unit/observable.js @@ -169,6 +169,109 @@ test('trigger', function() { equal(context, foo); }); +test('removal of past events', function() { + var foo = { }, + event1Fired = false, event2Fired = false, + event3Fired = false, event4Fired = false, + handler1 = function() { + event1Fired = true; + foo.off('bar:baz', handler1); + }, + handler2 = function() { + event2Fired = true; + }, + handler3 = function() { + event3Fired = true; + }, + handler4 = function() { + event4Fired = true; + }; + + fabric.util.object.extend(foo, fabric.Observable); + foo.on('bar:baz', handler1); + foo.on('bar:baz', handler2); + foo.on('bar:baz', handler3); + foo.on('bar:baz', handler4); + equal(foo.__eventListeners['bar:baz'].length, 4, 'There should be 4 events registered now'); + foo.trigger('bar:baz'); + equal(foo.__eventListeners['bar:baz'].length, 3, 'There should be 3 events registered now'); + equal(event1Fired, true, 'Event 1 should fire'); + equal(event2Fired, true, 'Event 2 should fire'); + equal(event3Fired, true, 'Event 3 should fire'); + equal(event4Fired, true, 'Event 4 should fire'); +}); + +test('removal of past events inner loop', function() { + var foo = { }, + event1Fired = 0, event2Fired = 0, + event3Fired = 0, event4Fired = 0, + handler1 = function() { + event1Fired++; + foo.off('bar:baz', handler1); + equal(foo.__eventListeners['bar:baz'].length, 4, 'There should be still 4 handlers registered'); + equal(event1Fired, 1, 'Event 1 should fire once'); + equal(event2Fired, 0, 'Event 2 should not be fired yet'); + equal(event3Fired, 0, 'Event 3 should not be fired yet'); + equal(event4Fired, 0, 'Event 4 should not be fired yet'); + foo.trigger('bar:baz'); + equal(foo.__eventListeners['bar:baz'].length, 3, 'There should be 3 handlers registered now'); + }, + handler2 = function() { + event2Fired++; + }, + handler3 = function() { + event3Fired++; + }, + handler4 = function() { + event4Fired++; + }; + + fabric.util.object.extend(foo, fabric.Observable); + foo.on('bar:baz', handler1); + foo.on('bar:baz', handler2); + foo.on('bar:baz', handler3); + foo.on('bar:baz', handler4); + foo.trigger('bar:baz'); + equal(event1Fired, 1, 'Event 1 should fire once'); + equal(event2Fired, 2, 'Event 2 should fire twice'); + equal(event3Fired, 2, 'Event 3 should fire twice'); + equal(event4Fired, 2, 'Event 4 should fire twice'); +}); + +test('adding events', function() { + var foo = { }, + event1Fired = false, event2Fired = false, + event3Fired = false, event4Fired = false, + handler1 = function() { + event1Fired = true; + foo.off('bar:baz', handler1); + foo.on('bar:baz', handler3); + foo.on('bar:baz', handler4); + }, + handler2 = function() { + event2Fired = true; + }, + handler3 = function() { + event3Fired = true; + }, + handler4 = function() { + event4Fired = true; + }; + + fabric.util.object.extend(foo, fabric.Observable); + foo.on('bar:baz', handler1); + foo.on('bar:baz', handler2); + foo.trigger('bar:baz'); + equal(event1Fired, true, 'Event 1 should fire'); + equal(event2Fired, true, 'Event 2 should fire'); + equal(event3Fired, false, 'Event 3 should not fire'); + equal(event4Fired, false, 'Event 4 should not fire'); + foo.trigger('bar:baz'); + equal(event3Fired, true, 'Event 3 should be triggered now'); + equal(event4Fired, true, 'Event 4 should be triggered now'); +}); + + test('chaining', function() { var foo = { }; fabric.util.object.extend(foo, fabric.Observable);