Removing listeners can cause an exception depending on how you call off()

[githubdorey]

Line 3277 (version 2.3.4) can cause an exception when removing listeners. If you do an off() and then off("name.listener") can cause exceptions.

I use several plugins like resize.js, draggable.js. Each have their own clean up function that removes listeners.

Example of my code. var shapes= draw.set()

function cleanupExtensions() {
    shapes.selectize(false, {deepSelect:true});
    shapes.each(function(i) {
      this.resize('stop');
      this.draggable(false);
    });
}

...
shapes.off(); // cleanup my own listeners
cleanupExtensions();

By invoking shapes.off() i believe
delete SVG.listeners[index]
is called and when this.resize('stop'); and this.draggable(false); are called then
if (SVG.listeners[index][ev] && SVG.listeners[index][ev][ns]) {
will cause an exception. Because they are doing individual off() calls. Like this.el.off("lt.resize");

My fix is to add a null check in the if statement.

if (SVG.listeners[index] && SVG.listeners[index][ev] && SVG.listeners[index][ev][ns]) {

SVG.off = function(node, event, listener) {
  var index = SVG.handlerMap.indexOf(node)
    , ev    = event && event.split('.')[0]
    , ns    = event && event.split('.')[1]

  if(index == -1) return

  if (listener) {
    if(typeof listener == 'function') listener = listener._svgjsListenerId
    if(!listener) return

    // remove listener reference
    if (SVG.listeners[index][ev] && SVG.listeners[index][ev][ns || '*']) {
      // remove listener
      node.removeEventListener(ev, SVG.listeners[index][ev][ns || '*'][listener], false)

      delete SVG.listeners[index][ev][ns || '*'][listener]
    }

  } else if (ns && ev) {
    // remove all listeners for a namespaced event
    if (SVG.listeners[index][ev] && SVG.listeners[index][ev][ns]) {  // THIS IS WHERE THE EXCEPTION THROWN
      for (listener in SVG.listeners[index][ev][ns])
        SVG.off(node, [ev, ns].join('.'), listener)

      delete SVG.listeners[index][ev][ns]
    }

  } else if (ns){
    // remove all listeners for a specific namespace
    for(event in SVG.listeners[index]){
        for(namespace in SVG.listeners[index][event]){
            if(ns === namespace){
                SVG.off(node, [event, ns].join('.'))
            }
        }
    }

  } else if (ev) {
    // remove all listeners for the event
    if (SVG.listeners[index][ev]) {
      for (namespace in SVG.listeners[index][ev])
        SVG.off(node, [ev, namespace].join('.'))

      delete SVG.listeners[index][ev]
    }

  } else {
    // remove all listeners on a given node
    for (event in SVG.listeners[index])
      SVG.off(node, event)

    delete SVG.listeners[index]

  }
}

[Fuzzyma]

Good catch! Can you pls create a testcase which shows that error? That would help a lot and speed up the fixing process.

However: You should avoid calling a general off() on an element. You could end up removing listeners which arent yours which then would lead to wrong behavior.
Not every plugin will gracefully accept your interference as svg.select or svg.resize do.
So you better get used to using a namespace for your own bindings, too.

[githubdorey]

svg-test-off-exception.zip

There is just one file. It is not a Jasmine test case but just an html file that shows the error.

The off() works for me in this case because I have a controlled Set of objects that I apply it to, so I can be lazy and just call off() And having no listeners on those objects is exactly what I want.

[Fuzzyma]

The intention of this test is to have our code better tested with higher code coverage and that this bug is not reintroduced later. So a jasmine test is what I was aiming for :D