[Pkg-javascript-commits] [sockjs-client] 07/350: Implement DOM Level 3 behavior instead

[tonnerre at ancient-solutions.com]

This is an automated email from the git hooks/post-receive script.

tonnerre-guest pushed a commit to branch upstream
in repository sockjs-client.

commit a6b1e66a26cb658942f24796c505667216c6f275
Author: David Benjamin <davidben at mit.edu>
Date:   Fri Aug 2 20:54:51 2013 -0400

    Implement DOM Level 3 behavior instead
    
    Instead of having removeEventListener affect the current dispatchEvent, but not
    addEventListener, have neither do it. This is DOM Level 3's behavior rather
    than DOM Level 2's behavior.
    
    As of writing, IE and older Opera matches the Level 3 behavior and other
    browsers match the Level 2 behavior.  Mimic the IE behavior because it's
    simpler and this is not a critical edge case. See
    https://github.com/sockjs/sockjs-client/pull/127 for discussion.
    
    Also revise the unit test to actually test the case when the listener list goes
    empty which was the original goal of this entire exercise.
---
 lib/reventtarget.js             | 32 +++++++++++---------------------
 tests/html/src/unittests.coffee | 33 ++++++++++++++++++++++++++-------
 2 files changed, 37 insertions(+), 28 deletions(-)

diff --git a/lib/reventtarget.js b/lib/reventtarget.js
index 22c63b9..aed5f85 100644
--- a/lib/reventtarget.js
+++ b/lib/reventtarget.js
@@ -9,15 +9,6 @@
 /* Simplified implementation of DOM2 EventTarget.
  *   http://www.w3.org/TR/DOM-Level-2-Events/events.html#Events-EventTarget
  */
-
-var indexOfListener = function(list, listener) {
-    for (var i = 0; i < list.length; i++) {
-        if (list[i].listener === listener)
-            return i;
-    }
-    return -1;
-};
-
 var REventTarget = function() {};
 REventTarget.prototype.addEventListener = function (eventType, listener) {
     if(!this._listeners) {
@@ -27,9 +18,11 @@ REventTarget.prototype.addEventListener = function (eventType, listener) {
         this._listeners[eventType] = [];
     }
     var arr = this._listeners[eventType];
-    if(indexOfListener(arr, listener) === -1) {
-        arr.push({listener: listener, doomed: false});
+    if(utils.arrIndexOf(arr, listener) === -1) {
+        // Make a copy so as not to interfere with a current dispatchEvent.
+        arr = arr.concat([listener]);
     }
+    this._listeners[eventType] = arr;
     return;
 };
 
@@ -38,13 +31,11 @@ REventTarget.prototype.removeEventListener = function (eventType, listener) {
         return;
     }
     var arr = this._listeners[eventType];
-    var idx = indexOfListener(arr, listener);
+    var idx = utils.arrIndexOf(arr, listener);
     if (idx !== -1) {
-        // Removing an event listener that has not yet run
-        // mid-dispatch causes the listener to not run.
-        arr[idx].doomed = true;
         if(arr.length > 1) {
-            arr.splice(idx, 1);
+            // Make a copy so as not to interfer with a current dispatchEvent.
+            this._listeners[eventType] = arr.slice(0, idx).concat( arr.slice(idx+1) );
         } else {
             delete this._listeners[eventType];
         }
@@ -64,12 +55,11 @@ REventTarget.prototype.dispatchEvent = function (event) {
         this['on'+t].apply(this, args);
     }
     if (this._listeners && t in this._listeners) {
-        // Make a copy of the listeners list; listeners added
-        // mid-dispatch should NOT run.
-        var listeners = this._listeners[t].slice(0);
+        // Grab a reference to the listeners list. removeEventListener may
+        // remove the list.
+        var listeners = this._listeners[t];
         for(var i=0; i < listeners.length; i++) {
-            if (!listeners[i].doomed)
-                listeners[i].listener.apply(this, args);
+            listeners[i].apply(this, args);
         }
     }
 };
diff --git a/tests/html/src/unittests.coffee b/tests/html/src/unittests.coffee
index 4931243..7b6ca63 100644
--- a/tests/html/src/unittests.coffee
+++ b/tests/html/src/unittests.coffee
@@ -222,7 +222,7 @@ test 'detectProtocols', ->
             ['websocket', 'iframe-htmlfile', 'iframe-xhr-polling'])
 
 test "EventEmitter", ->
-    expect(6)
+    expect(7)
     r = new SockJS('//1.2.3.4/wrongurl', null,
                    {protocols_whitelist: []})
     r.addEventListener 'message', -> ok(true)
@@ -236,26 +236,45 @@ test "EventEmitter", ->
     r.removeEventListener 'message', bluff
     r.dispatchEvent({type:'message'})
 
-    # Listeners added mid-dispatch do not get run, however listeners
-    # removed mid-dispatch that have not yet run also don't run.
-    log = []
+    # Per DOM Level 3, addEventListener and removeEventListener does not effect
+    # a pending dispatchEvent. This is consistent with IE and older Opera, but
+    # not other browsers (which implement the DOM Level 2 behavior). IE's is
+    # simpler, so we mimic it as this is not an important edge case.
+    #
+    # See https://github.com/sockjs/sockjs-client/pull/127
     handler0 = -> log.push(0)
     handler1 = ->
         log.push(1)
         r.removeEventListener 'test', handler0
         r.removeEventListener 'test', handler2
         r.addEventListener 'test', handler3
+        r.addEventListener 'test', handler4
     handler2 = -> log.push(2)
-    handler3 = -> log.push(3)
+    handler3 = ->
+        log.push(3)
+        r.removeEventListener 'test', handler1
+        r.removeEventListener 'test', handler3
+        r.removeEventListener 'test', handler4
+    handler4 = -> log.push(4)
     r.addEventListener 'test', handler0
     r.addEventListener 'test', handler1
     r.addEventListener 'test', handler2
 
+    # Should run the registered listeners when first calling.
+    log = []
+    r.dispatchEvent({type:'test'})
+    deepEqual(log, [0, 1, 2])
+
+    # Should run the new listeners and not crash even though we remove the last
+    # one mid-dispatch.
+    log = []
     r.dispatchEvent({type:'test'})
-    deepEqual(log, [0, 1])
+    deepEqual(log, [1, 3, 4])
+
+    # All event listeners gone. Should run none.
     log = []
     r.dispatchEvent({type:'test'})
-    deepEqual(log, [1, 3])
+    deepEqual(log, [])
 
     # Adding the same eventlistener should be indempotent (sockjs-client #4).
     single = -> ok(true)

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-javascript/sockjs-client.git