diff --git a/ChangeLog b/ChangeLog index c5cedbc00..4f7c6bae4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -37,6 +37,7 @@ Network: enable parsing of 'https' URL and TLS flag even even TLS not built in (#2410) removeListener while executing events no longer stops subsequent listeners from executing (#2151) jsvObjectIterator is now safe even if not called on something iterable + X.on now always allocates an array - tidies up code (fix #2559) 2v24 : Bangle.js2: Add 'Bangle.touchRd()', 'Bangle.touchWr()' Bangle.js2: After Bangle.showTestScreen, put Bangle.js into a hard off state (not soft off) diff --git a/src/jswrap_object.c b/src/jswrap_object.c index 0b45707df..f6d0c7156 100644 --- a/src/jswrap_object.c +++ b/src/jswrap_object.c @@ -19,6 +19,7 @@ #include "jswrapper.h" #include "jswrap_stream.h" #include "jswrap_functions.h" +#include "jswrap_array.h" // for splice #ifdef __MINGW32__ #include "malloc.h" // needed for alloca #endif//__MINGW32__ @@ -889,23 +890,22 @@ void jswrap_object_on_X(JsVar *parent, JsVar *event, JsVar *listener, bool addFi JsVar *eventList = jsvFindChildFromVar(parent, eventName, true); jsvUnLock(eventName); JsVar *eventListeners = jsvSkipName(eventList); + // if no array, add it if (jsvIsUndefined(eventListeners)) { - // just add the one handler on its own - jsvSetValueOfName(eventList, listener); - } else { - // we already have an array and we just add to it - // OR it's not an array but we need to make it an array - JsVar *arr = jsvNewEmptyArray(); - if (addFirst) jsvArrayPush(arr, listener); - if (jsvIsArray(eventListeners)) - jsvArrayPushAll(arr, eventListeners, false); - else - jsvArrayPush(arr, eventListeners); - if (!addFirst) jsvArrayPush(arr, listener); - jsvSetValueOfName(eventList, arr); - jsvUnLock(arr); + eventListeners = jsvNewEmptyArray(); + jsvSetValueOfName(eventList, eventListeners); + } + jsvUnLock(eventList); + if (!eventListeners) return; // out of memory + // now have an array and we just add to it + if (addFirst) { // add it first? + JsVar *elements = jsvNewArray(&listener, 1); + jswrap_array_unshift(eventListeners, elements); + jsvUnLock(elements); + } else { // or add it at the end + jsvArrayPush(eventListeners, listener); } - jsvUnLock2(eventListeners, eventList); + jsvUnLock(eventListeners); /* Special case if we're a data listener and data has already arrived then * we queue an event immediately. */ if (jsvIsStringEqual(event, "data")) { @@ -1033,16 +1033,12 @@ void jswrap_object_removeListener(JsVar *parent, JsVar *event, JsVar *callback) jsvUnLock(eventName); JsVar *eventList = jsvSkipName(eventListName); if (eventList) { - if (eventList == callback) { - // there's no array, it was a single item - jsvRemoveChild(parent, eventListName); - } else if (jsvIsArray(eventList)) { + if (jsvIsArray(eventList)) { // it's an array, search for the index JsVar *idx = jsvGetIndexOf(eventList, callback, true); - if (idx) { + if (idx) jsvRemoveChildAndUnLock(eventList, idx); - } - } + } // otherwise something is wrong, but lets just ignore it jsvUnLock(eventList); } jsvUnLock(eventListName); diff --git a/tests/test_eventemitter_removeListener.js b/tests/test_eventemitter_removeListener.js index af8909d79..c96833cb3 100644 --- a/tests/test_eventemitter_removeListener.js +++ b/tests/test_eventemitter_removeListener.js @@ -8,9 +8,12 @@ var a = {}; // should be EventEmitter, but currently all objects have it a.on("data", foo); a.emit("data", 8); -a.removeListener("data", foo); -a.emit("data", 9); - +// we need a delay, because events are processed in the next idle loop +// and if we remove the listener before then, it won't get the event setTimeout(function() { - result = x==42; + a.removeListener("data", foo); + a.emit("data", 9); }, 1); +setTimeout(function() { + result = x==42; +}, 2);