Opened 8 years ago

Closed 7 years ago

#12953 closed defect (fixed)

Tree: node cache can interfere with the appearance of a re-added node

Reported by: Updownquark Owned by:
Priority: high Milestone: 1.9
Component: Dijit Version: 1.6.1
Keywords: dijit tree node cache Cc:
Blocked By: Blocking:

Description

When a tree node is removed from display in the dijit Tree, the node is placed into a cache mapped by ID. This is intended to improve performance for DnD operations so dragged nodes do not need to be re-created as a result of dragging within the same tree.

A problem can occur, though, when a node is removed from the tree's display and then re-appears later with the same ID, but different display data. The operation that occurs within dijit._TreeNode as a result of dijit.Tree.model.onChildrenChange checks the cache for a node mapped to the identity of the item. If the node already exists, the node is re-added to the tree as-is with no check on the item that the node was added for. This may result in incorrect display of data if the item's content has changed since it was removed.

The fix is easy. A single call to dijit._TreeNode._updateItemClasses after the node is retrieved from the cache. This does not completely solve the problem, as it does not address the node's children; but it makes it possible for developers to detect the problem and address it themselves. The performance hit for what the cache was intended for should be very minimal, since _updateItemClasses is a very small method.

The simple fix is to add the 2 lines

else
    node._updateItemClasses(item);

after

if(!node){
    ...
}

in dijit._TreeNode.setChildItems (the new code should be inserted at line 344 in the current trunk).

Change History (2)

comment:1 Changed 8 years ago by bill

Summary: Tree's node cache can interfere with the appearance of a re-added nodeTree: node cache can interfere with the appearance of a re-added node

Thanks for the report. Can you attach a test case?

Also, IIRC I didn't put in the cache for performance (although it will help), but rather so we don't lose the opened/closed state of the dragged node's descendants, and also perhaps the list of selected nodes.

comment:2 Changed 7 years ago by bill

Milestone: tbd1.9
Resolution: fixed
Status: newclosed

I think this is addressed by [29552] and [29553].

Note: See TracTickets for help on using tickets.