Success! Looks like we've fixed this one. According to our records the fix was applied for a bug in our system in a recent build.
  1. #1
    Sencha User
    Join Date
    Nov 2011
    Posts
    21
    Vote Rating
    4
    Navaar is on a distinguished road

      1  

    Default [4.2.0.265] Drag & Drop and node removal on TreeView broken when nextSibling is null

    [4.2.0.265] Drag & Drop and node removal on TreeView broken when nextSibling is null


    There is a bug in onNodeCollapse in Ext.data.NodeStore that causes parts of a TreeView to dissapear when removing nodes during collapse and Drag & Drop operations.

    It seems to incorrectly calculate the indicies to remove and ends up removing everything below the parent node in the tree.

    Here is an example that will allow you to reproduce the error.

    Just run this and drag Parent4 to the ExtJS node and you will see what I mean.
    Code:
    Ext.require([
        'Ext.tree.*',
        'Ext.data.*',
        'Ext.tip.*'
    ]);
    
    
    Ext.onReady(function() {
        Ext.QuickTips.init();
    
    
        var store = Ext.create('Ext.data.TreeStore', {
            root: {
                text: 'Ext JS',
                id: 'root',
                expanded: true,
                children: [
                    {
                        text: 'parent',
                        id: 'p1',
                        expanded: true,
                        children: [
                            {
                                text: 'Parent 3',
                                id: 'p3',
                                expanded: true,
                                children: [
                                    {
                                        leaf: true,
                                        text: 'child 1',
                                        id: 'p3c1'
                                    },
                                    {
                                        leaf: true,
                                        text: 'child 2',
                                        id: 'p3c2'
                                    }
    
    
                                ]
                            },
                            {
                                text: 'Parent 4',
                                id: 'p4',
                                expanded: true,
                                children: [
                                    {
                                        leaf: true,
                                        text: 'child 1',
                                        id: 'p4c1'
                                    },
                                    {
                                        leaf: true,
                                        text: 'child 2',
                                        id: 'p4c2'
                                    }
    
    
                                ]
                            }
    
    
                        ]
                    },
                    {
                        text: 'Parent 2',
                        id: 'p2',
                        expanded: true,
                        children: [
                            {
                                leaf: true,
                                text: 'child 1',
                                id: 'p2c1'
                            },
                            {
                                leaf: true,
                                text: 'child 2',
                                id: 'p2c2'
                            }
    
    
                        ]
                    },
                    {
                        leaf: true,
                        text: 'child 1',
                        id: 'rc1'
                    },
                    {
                        leaf: true,
                        text: 'child 2',
                        id: 'rc2'
                    }
    
    
                ]
            },
            folderSort: true,
            sorters: [{
                property: 'text',
                direction: 'ASC'
            }]
        });
    
    
        var tree = Ext.create('Ext.tree.Panel', {
            store: store,
            viewConfig: {
                plugins: {
                    ptype: 'treeviewdragdrop',
                    containerScroll: true
                }
            },
            renderTo: 'tree-div',
            height: 300,
            width: 250,
            title: 'Files',
            useArrows: true,
            dockedItems: [{
                xtype: 'toolbar',
                items: [{
                    text: 'Expand All',
                    handler: function(){
                        tree.getEl().mask('Expanding tree...');
                        var toolbar = this.up('toolbar');
                        toolbar.disable();
    
    
                        tree.expandAll(function() {
                            tree.getEl().unmask();
                            toolbar.enable();
                        });
                    }
                }, {
                    text: 'Collapse All',
                    handler: function(){
                        var toolbar = this.up('toolbar');
                        toolbar.disable();
    
    
                        tree.collapseAll(function() {
                            toolbar.enable();
                        });
                    }
                }]
            }]
        });
    });

    Fix:
    To fix this code, a small adjustment to Ext.data.NodeStore is needed.
    In onNodeCollapse after it checks to make sure the records are there to be removed there is this line:
    Code:
                // Calculate the index *one beyond* the last node we are going to remove
                lastNodeIndexPlus = parent.nextSibling ? me.indexOf(parent.nextSibling) : me.getCount();
    
        // Remove the whole collapsed node set.
        me.removeAt(collapseIndex, lastNodeIndexPlus - collapseIndex);
    The issue is that if the parent node has no other siblings, it assumes it is in the root of the tree, and the call to me.getCount() returns the length of the entire tree, causing it to delete all the displayed nodes between the parent node and the end of the tree.

    I ended up addressing the issue by changing the line to this:
    Code:
    lastNodeIndexPlus = parent.nextSibling ? me.indexOf(parent.nextSibling) : collapseIndex + parent.childNodes.length;
    
        // Remove the whole collapsed node set.
        me.removeAt(collapseIndex, lastNodeIndexPlus - collapseIndex);
    I'm not sure of any other issues this might cause. It seems like you could just look at childNodes.length, or nodes.length instead of looking at the nextSibling index. Although there might be cases where another approach would be needed.

    I hope this helps and can be fixed for 4.2.0 release!

  2. #2
    Sencha - Ext JS Dev Team evant's Avatar
    Join Date
    Apr 2007
    Location
    Sydney, Australia
    Posts
    16,645
    Vote Rating
    583
    evant has a reputation beyond repute evant has a reputation beyond repute evant has a reputation beyond repute evant has a reputation beyond repute evant has a reputation beyond repute evant has a reputation beyond repute evant has a reputation beyond repute evant has a reputation beyond repute evant has a reputation beyond repute evant has a reputation beyond repute evant has a reputation beyond repute

      1  

    Default


    Thanks for the test case, we already caught this one internally and have a fix applied.

    It's similar to the one you posted, but you can't just rely on the parent node, you need to navigate all the way up the tree to find the next "sibling".
    Evan Trimboli
    Sencha Developer
    Twitter - @evantrimboli
    Don't be afraid of the source code!

  3. #3
    Sencha User
    Join Date
    Nov 2011
    Posts
    21
    Vote Rating
    4
    Navaar is on a distinguished road

      0  

    Default


    Thanks for the response.
    I tried to implement it looking up the tree to determine the range but the "remove" event is called after the node has already been removed from the tree structure.

    Altering Ext.data.NodeInterface.removeChild() so that "remove" is fired before it manipulates the tree data seems to address this issue as well.

    It's easy to reproduce by having a highly nested tree and expanding many branch nodes then collapsing a parent node. It leaves things in the tree that then get duplicated when you expand the tree again.

Thread Participants: 1