Fabricjs doesn't release mouse event handlers on dispose

[ComboStyle]

Version

2.3.0

Test Case

http://jsfiddle.net/Da7SP/1660/

Goal

Refresh the canvas object frequently, refreshing means to dispose() the current fabric canvas and then recreate it immediately.

Steps to reproduce

• Open your chrome console (or any console)
• Click your mouse on the canvas(in it) and do no release it (mouse:down)
• Wait 1 second and now release and move your mouse around
• Watch as the console is getting filled with infinite errors any time you move the mouse anywhere on the webpage.

Expected Behavior

Dispose() should clean everything related to fabric, allowing you to recreate a new fabric if you wish so, and should always clean all event handlers and all data

Actual Behavior

Dispose() won't really clean fabric if a mouse event is already in progress (mouse down), causing a bug that will fail the browser until a full webpage refresh

Final words

It seems like fabric is registering some GLOBAL(document) events on mouse:down and do not release them on dispose causing errors if you happen to call dispose() while the user clicked on something in the canvas

When I run getEventListeners(document) [on my webpage not on the fiddle, in the fiddle you would need to get the document of the html VM window not the fiddle window] I get:
{click: Array(2)}
Each time I recreate this bug and list the getEventListeners(document) I get:
{click: Array(2), touchend: Array(1), touchmove: Array(1), mouseup: Array(1), mousemove: Array(1)}
And if I cause it again:
{click: Array(2), touchend: Array(2), touchmove: Array(2), mouseup: Array(2), mousemove: Array(2)}

So it seems like touchend,touchmove,mouseup,mousemove do not get cleaned if an event was mid-air while disposing.

[asturur]

i have to admit this is a weird setup, but i also think we should clear them better.

Also the dispose is not clearing some dimension and if you are on retina screen the canvas size grow indefintely

[ComboStyle]

@asturur

I set a very short timeframe for the setInterval here, normally I clear each 5 seconds to adjust the canvas size and some other params, it still has a chance to blowup if the user is holding his mouse when I dispose()

I am on a retina screen, what do you mean by not clearing some dimensions? is there a way to recreate a fresh fabric object without having to refresh the page? that is basically what I am trying to achieve.

Thanks

[asturur]

dispose should do it, needs to be fixed for events.

[asturur]

look my canvas size if i run that fiddle on retina

[ComboStyle]

@asturur oh I see, the width/height bug seems to only happen when you do not specify width,height I do specify those in my code so I haven't encountered it on my tests

[asturur]

now that i think of it, since the canvas event handlers do get cleared, what i m thinking that may be missing is that we appen an event to document in order to handle mouse move outside the canvas
Probably that one does not get registered, i ll check and fix.

[asturur]

if you want to build fabric from here and have a test:
061d047

i ll do when i can.

[ComboStyle]

@asturur I tried to build and I got some errors about fabric constructor when I tried to run, probably my fault, the fix seems logical so I will just wait for the official build when it comes (no pressure I have a workaround for meantime)

Thanks for the quick response :)