Removing events with element.click(null) throws JS error

[pjfsilva]

Bug report

The docs clearly state that to remove an element we can use element.click(null) but doing that throws an JS error:

svg.js:3430 Uncaught TypeError: Cannot read property 'bind' of null at Function.SVG.on (svg.js:3430)

Looking at events.js code, line 33, it's quite clear this is expected:

SVG.on = function(node, event, listener, binding, options) {
  // create listener, get object-index
  var l     = listener.bind(binding || node.instance || node)

Calling element.click(null) will result on that listener being null, causing a JS error when we try to do 'listener.bind'

Fiddle

https://jsfiddle.net/c4f8udrb/

Just open developer tools and click mousedown ON and then OFF or just OFF.

Explanation

Now, I'm not sure if this is a documentation error or a bug on SVG.js. Looking at the source code it seems SVG.JS never intended to work that way since there is no code indication of supporting this.

I was looking at the older SVG.js version that I was using (1.1.0-5-gc66d24a) and that had some code for passing null on SVG.Element.prototype[event] = function(f) so I'm not sure if that was lost along the way or the docs are wrong and to be honest I don't have a strong opinion if that should be supported or not :)

So I'll just wait for the devs opinion on what's the expected result. Either way that should be a nice test case to add to the suite ;)

[Fuzzyma]

Jeah - I guess you are right. That "feature" was lost by converting it to 2.x. However after thinking about it, it might be better to just remove the part of the docs. The "sugar" methods like click, mousedown ... are only that - sugar and relay there call to on('click', fn). Having extra logic which maps it to off('click') when null is passed feels wrong and might even confuse users who use click AND bind because click(null) would remove ALL click listeners bound with click AND on.

What do you think?

[saivan]

Firstly, thanks for the awesome bug report! This is super clear and easy to follow :) I'm afraid that @Fuzzyma is right. This might have been a code change at some point that got lost in the docs somewhere.

It depends on how we intend for the users to use these methods. If we are going with event handlers, then .on() makes sense. But should .off() remove all of the attached event handlers, or only the ones we've bound?

Otherwise, I'm all for simplicity, and I do think .click(null)is simple. Just not so sure what is best in this case :P I'd be interested in hearing what you'd expect to happen here too.

[Fuzzyma]

Reimplemented. See 2.7.0