Make sure we call event listeners in the order they are registered (fix

#4743)
Leaflet · Aug 1, 2016 · fe51cf8 · fe51cf8
1 parent c01adbf
commit fe51cf8
Show file tree

Hide file tree

Showing 2 changed files with 87 additions and 58 deletions.
diff --git a/spec/suites/core/EventsSpec.js b/spec/suites/core/EventsSpec.js
@@ -35,6 +35,62 @@ describe('Events', function () {
 			// expect(spy6.callCount).to.be(1);
 		});
 
+		it('fires all listeners in the order they are added', function () {
+			var obj = new L.Evented(),
+			    ctx1 = new L.Class(),
+			    ctx2 = new L.Class(),
+			    count = {one: 0, two: 0, three: 0, four: 0};
+
+			function listener1(e) {
+				count.one++;
+				expect(count.two).to.eql(0);
+			}
+
+			function listener2(e) {
+				count.two++;
+				expect(count.one).to.eql(1);
+				expect(count.three).to.eql(0);
+				if (count.two === 1) {
+					expect(this).to.eql(ctx2);
+				} else if (count.two === 2) {
+					expect(this).to.eql(ctx1);
+				} else {
+					expect(this).to.eql(obj);
+				}
+			}
+
+			function listener3(e) {
+				count.three++;
+				expect(count.two).to.eql(3);
+				expect(count.four).to.eql(0);
+				if (count.three === 1) {
+					expect(this).to.eql(ctx1);
+				} else if (count.three === 2) {
+					expect(this).to.eql(ctx2);
+				}
+			}
+
+			function listener4(e) {
+				count.four++;
+				expect(count.three).to.eql(2);
+			}
+
+			obj.on('test', listener1, ctx1);
+			obj.on('test', listener2, ctx2);
+			obj.on('test', listener2, ctx1);  // Same listener but with different context.
+			obj.on('test', listener2);  // Same listener but without context.
+			obj.on('test', listener3, ctx1);
+			obj.on('test', listener3, ctx2);
+			obj.on('test', listener4, ctx2);
+
+			obj.fireEvent('test');
+
+			expect(count.one).to.be(1);
+			expect(count.two).to.be(3);
+			expect(count.three).to.be(2);
+			expect(count.four).to.be(1);
+		});
+
 		it('provides event object to listeners and executes them in the right context', function () {
 			var obj = new L.Evented(),
 			    obj2 = new L.Evented(),

diff --git a/src/core/Events.js b/src/core/Events.js
@@ -96,102 +96,81 @@ L.Evented = L.Class.extend({
 		var typeListeners = this._events[type];
 		if (!typeListeners) {
 			typeListeners = {
-				listeners: {},
+				listeners: [],
 				count: 0
 			};
 			this._events[type] = typeListeners;
 		}
 
-		var contextId = context && context !== this && L.stamp(context),
-		    newListener = {fn: fn, ctx: context};
-
-		if (!contextId) {
-			contextId = 'no_context';
-			newListener.ctx = undefined;
-		}
-
-		// fn array for context
-		var listeners = typeListeners.listeners[contextId];
-		if (!listeners) {
-			listeners = [];
-			typeListeners.listeners[contextId] = listeners;
+		if (context === this) {
+			// Less memory footprint.
+			context = undefined;
 		}
+		var newListener = {fn: fn, ctx: context},
+		    listeners = typeListeners.listeners;
 
 		// check if fn already there
 		for (var i = 0, len = listeners.length; i < len; i++) {
-			if (listeners[i].fn === fn) {
+			if (listeners[i].fn === fn && listeners[i].ctx === context) {
 				return;
 			}
 		}
 
-		listeners.push(newListener);
+		typeListeners.listeners.push(newListener);
 		typeListeners.count++;
 	},
 
 	_off: function (type, fn, context) {
 		var typeListeners,
-		    contextId,
 		    listeners,
 		    i,
 		    len;
 
 		if (!this._events) { return; }
 
+		typeListeners = this._events[type];
+
+		if (!typeListeners) {
+			return;
+		}
+
+		listeners = typeListeners.listeners;
+
 		if (!fn) {
 			// Set all removed listeners to noop so they are not called if remove happens in fire
-			typeListeners = this._events[type];
-			if (typeListeners) {
-				for (contextId in typeListeners.listeners) {
-					listeners = typeListeners.listeners[contextId];
-					for (i = 0, len = listeners.length; i < len; i++) {
-						listeners[i].fn = L.Util.falseFn;
-					}
-				}
-				// clear all listeners for a type if function isn't specified
-				delete this._events[type];
+			for (i = 0, len = listeners.length; i < len; i++) {
+				listeners[i].fn = L.Util.falseFn;
 			}
+			// clear all listeners for a type if function isn't specified
+			delete this._events[type];
 			return;
 		}
 
-		typeListeners = this._events[type];
-		if (!typeListeners) {
-			return;
-		}
 
-		contextId = context && context !== this && L.stamp(context);
-		if (!contextId) {
-			contextId = 'no_context';
+		if (context === this) {
+			context = undefined;
 		}
 
-		listeners = typeListeners.listeners[contextId];
 		if (listeners) {
 
 			// find fn and remove it
 			for (i = 0, len = listeners.length; i < len; i++) {
 				var l = listeners[i];
+				if (l.ctx !== context) { continue; }
 				if (l.fn === fn) {
 
 					// set the removed listener to noop so that's not called if remove happens in fire
 					l.fn = L.Util.falseFn;
 					typeListeners.count--;
 
-					if (len > 1) {
-						if (!this._isFiring) {
-							listeners.splice(i, 1);
-						} else {
-							/* copy array in case events are being fired */
-							typeListeners.listeners[contextId] = listeners.slice();
-							typeListeners.listeners[contextId].splice(i, 1);
-						}
-					} else {
-						delete typeListeners.listeners[contextId];
+					if (this._isFiring) {
+						/* copy array in case events are being fired */
+						listeners = listeners.slice();
 					}
+					listeners.splice(i, 1);
 
 					return;
 				}
-				if (listeners.length === 0) {
-					delete typeListeners.listeners[contextId];
-				}
 			}
 		}
 	},
@@ -210,16 +189,10 @@ L.Evented = L.Class.extend({
 
 			if (typeListeners) {
 				this._isFiring = true;
-
-				// each context
-				for (var contextId in typeListeners.listeners) {
-					var listeners = typeListeners.listeners[contextId];
-
-					// each fn in context
-					for (var i = 0, len = listeners.length; i < len; i++) {
-						var l = listeners[i];
-						l.fn.call(l.ctx || this, event);
-					}
+				var listeners = typeListeners.listeners;
+				for (var i = 0, len = listeners.length; i < len; i++) {
+					var l = listeners[i];
+					l.fn.call(l.ctx || this, event);
 				}
 
 				this._isFiring = false;