jsplumb / community-edition

The community edition of jsPlumb, versions 1.x - 6.x
https://jsplumbtoolkit.com
Other
232 stars 20 forks source link

OrphanAll attempting to orphan detached element #258

Closed csandwith closed 1 year ago

csandwith commented 1 year ago

Hi all, I've managed to put JSPlumb into a bit of a state. I'll try to break it down as reasonably as I can, with the caveat that JSPlumb is fairly integrated into our webapp, so it's not 100% clear where the true fault lies.

When I move a node (within a group), a nodeMoveEnd event fires and one of our functions is called. Asynchronously, this function calls another function that, in turn, calls jsPlumb.clear.

Clear falls through some wrappers and ends up in jsPlumb.reset which calls removeAllGroups. For each managedGroup, removeGroup is called on it, which calls orphanAll on the group.

All of this works just fine, with one caveat - If I have a group with a node Foo, if I add a second node Bar to that group, then remove Bar, and then start dragging/dropping Foo around (multiple times typically), I can get JSPlumb into a state where we fall down this stack as described above, and orphanAll has an array of elements that still contains Bar - I'm not entire sure how I managed to get it into this state, but what ends up happening is _orphan gets called on this Bar element, which attempts to do el.parentNode.removeChild, but parentNode is null, so the whole thing blows up and I end up in a very weird like half-reset state.

By static analysis, it is clear that, once orphanAll is done collecting these new positions, it clears the elements array anyway - I propose a modification to the code that, in _orphan, returns null if parentNode is null and, in orphanAll, simply skips items that return null from _orphan.

I'll be the first to admit that, logically speaking, the issue here is probably on our end, that somehow when we remove this node we aren't correctly removing it from JSPlumb, but I'm not sure why this issue (which was quite a beast to track down) would be so challenging to reproduce, which is to say that in most cases JSPlumb works exactly as expected.

Anyway, there'll be a pull request coming in for this ticket here shortly, I'll let you guys decide if I'm crazy or not.

csandwith commented 1 year ago

...aaand checking out the code I see that there's a non-zero chance I'm using a very old version of JSPlumb.

Apparently, per 4.0.0-RC23, "Removed _orphan method from Group and updated GroupManager's orphan method to work for all cases."...so shoot, let me try getting up to current and see if our code still works

csandwith commented 1 year ago

Confirmed, yeah, we're way behind. I'm closing this ticket as "Probably irrelevant to the project"