New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moving node to another tree fails when children present #1689

Closed
ashishmarwal opened this Issue Mar 7, 2014 · 2 comments

Comments

Projects
None yet
2 participants
@ashishmarwal

ashishmarwal commented Mar 7, 2014

A simple case can be found at: http://jsfiddle.net/ashishmarwal/42Sxu/

myTree:   rootNode
          /\      \
         /  \      \
        /    \      \
    Node 1  Node 2  Node 3
            / \
           /   \
    Node 2.1    Node 2.2

In the example demonstrated above, I am trying to move Node 2 along with it's children to targetTree. The move runs into a call to _adoptNode() with undefined node.

Upon debugging I realized that this is happening during the move of child nodes. In the first pass when Node 2.1 (index 0) is removed from the tree for Node 2, the underlying children array gets modified and the indexes become invalid. So the second pass doesn't find anything present at the expected position (index 1):

for (var i = 0, len = node.children.length; i < len; i++) {
     this._adoptNode(node.children[i], {silent: true});
}

I saw a comment in the same file's insertNode() function talking about something similar, but that doesn't convince me as in that case the functionality doesn't accomplish its goal.

Thanks!
Ashish

@rgrove rgrove self-assigned this Mar 7, 2014

@rgrove

This comment has been minimized.

Contributor

rgrove commented Mar 12, 2014

Thanks for the report and the test case! I'll look into this soon.

In the meantime, a simple workaround for this would probably be to just manually remove Node 2 from its original tree before adding it to the targetTree.

@ashishmarwal

This comment has been minimized.

ashishmarwal commented Mar 18, 2014

Thank you Ryan!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment