JAM-Software
CopyTo fails when target has uninitialized child nodes
From [email protected] on September 01, 2013 08:46:23
What steps will reproduce the problem? 1. Run attached sample project. 2. Click [Populate Tree] to set up the nodes. 3. Expand the first node "Colors" 4. Drag the child node "Red" and drop it on the "Shapes" node (without hovering long enough for the node to auto expand). 5. Expand the "Shapes" node. What is the expected output? What do you see instead? Expected output after the drag drop described above is: the "Shapes" node should contain the following four child nodes: Red, Circle, Square, Triangle.
Instead the "Shapes" node only contains the single node that was dragged: "Red". What version of the product are you using? On what operating system? V5.1.3 Win7 Please provide any additional information below. The problem occurs because the InternalConnectNode function when using attachmodes: amAddChildFirst or amAddChildLast naively adds the "Red" node without regard to the as yet uninitialized children: Circle, Square, Triangle. That causes those three nodes to never be included.
To fix the problem, I added this code to the start of both case items amAddChildFirst and amAddChildLast:
amAddChildFirst:
begin
// Start patch by Blacky: Sep 01, 2013
if (vsHasChildren in Destination.States) and (Destination.ChildCount = 0) then
ReinitNode(Destination,true);
// End patch by Blacky: Sep 01, 2013
...
amAddChildLast:
begin
// Start patch by Blacky: Sep 01, 2013
if (vsHasChildren in Destination.States) and (Destination.ChildCount = 0) then
ReinitNode(Destination,true);
// End patch by Blacky: Sep 01, 2013
It's a sledge-hammer approach, since only the immediate children of the destination node need to be initialized (not all children recursively), but I couldn't find a tree function that just initializes the immediate children.
Attachment: vstBug_002.zip
Original issue: http://code.google.com/p/virtual-treeview/issues/detail?id=363
From [email protected] on September 02, 2013 12:13:55
Does the problem also occur with V5.2.0? Does anyone have an idea for a less invasive fix?
From [email protected] on September 02, 2013 17:55:26
Yes, the problem still occurs in V5.2.0
The problem (I think) is an incorrect assumption in the InternalConenctNode function. That function assumes that if the Destination.FirstChild (or Destination.LastChild) pointer is nil, then the node has no children. It does not check to see whether or not the node is flagged as having children that are just not initialized yet.
That leads to a design issue/question: If a node is "connected" (via InternalConnectNode) into a target node as a child of that target node (i.e. amAddChildFirst or amAddChildLast), then should all the children in the target node be initialized prior to the new node being connected? I think yes, all the child nodes must always be initialized prior to connecting a new node, otherwise how can the new child node's next/prev sibling pointers be set correctly?
After looking at how the treeview code handles this situation when expanding a node via a mouse click, the lighter and more correct approach is to call InitChildren(Destination) instead of ReinitNode(Destination,true).
My initial sample code did not work to demonstrate this fix. I've updated the sample code (attached) so that it works correctly with this new patch.
Regards Paul
Attachment: vstBug_002_update_001.zip