swisnl / jQuery-contextMenu

jQuery contextMenu plugin & polyfill
https://swisnl.github.io/jQuery-contextMenu/
MIT License
2.24k stars 744 forks source link

Memory leak after calling $.contextMenu('destroy') #647

Closed last-Programmer closed 5 years ago

last-Programmer commented 6 years ago

Hi, We are using the context menu in our application and it seems to be that calling destroy does not remove the call back handlers from memory. It seems to be that there is a memory leak. Please detach all the event handlers on destroy.

Thanks

bbrala commented 6 years ago

Hmm, thanks die the report.

last-Programmer commented 6 years ago

this is happening when we have this in the callback (using typescript) : (key,opt)=> this.doSomething() If we have a static method in the callback memory leak is not happening.

bbrala commented 6 years ago

Ok, that makes things a little more complicated. But i could at least have look at what lingers when it unregisters. It should unset quite a lot, but we might've forgotten something.

last-Programmer commented 6 years ago

Any Update on this?

bbrala commented 6 years ago

Hmm, i've been looking into this, and perhaps this is the problem;

We call .off to remove the event handlers for the menu on destroy. But it seems there is a call with 2 namespaces in this line, which when looking at the documentation of jQuery.off() might mean the events arn't being removed.

The .off() method removes event handlers that were attached with .on(). See the discussion of delegated and directly bound events on that page for more information. Calling .off() with no arguments removes all handlers attached to the elements. Specific event handlers can be removed on elements by providing combinations of event names, namespaces, selectors, or handler function names. When multiple filtering arguments are given, all of the arguments provided must match for the event handler to be removed.

Could you test this locally? Perhaps just change the above line from

$document.off('.contextMenu .contextMenuAutoHide');

to

$document.off('.contextMenu');
$document.off('.contextMenuAutoHide');
last-Programmer commented 6 years ago

Thank you for the follow up. I tested with the above changes. and it does not help. here is the snapshot of the memory leak image

bbrala commented 6 years ago

I don't have much experience debugging memory leaks, but doesn't this snapshot tell us that the _this is the source of the problem? This probably is the result of the compilation of the typescript to keep make sure the callback is executed with the proper this context.

Also the fact it doesn't happen when using a static callback it kinda points in that direction. Unfortunately i don't use TS, so i can't easily test this.

last-Programmer commented 5 years ago

We are not able to track the source of the problem and unable to fix it.