how to fix TTreeView bug when using TreeNode.MoveTo?

Question

Using TreeNode.MoveTo(...) method sometimes does not work properly and raises an "Access Violation" exception.

mosttimes works and some time not.

Mosttimes 'Access Violation in module COMCTL32.DLL. Read of address FEEEFEFA' and program crash/freeze.

here is my code.

procedure TForm1.FormShow(Sender: TObject);
var
  I, sectioncount: Integer;
  parent, child: TTreeNode;
begin
  sectioncount := 0;
  for I := 0 to 19 do
  begin

    if I mod 5 = 0 then
    begin
      parent := TreeView1.Items.AddChild(nil, 'Section: ' + IntToStr(sectioncount));
      Inc(sectioncount);
    end;

    child := TreeView1.Items.AddChild(parent, 'Child: ' + IntToStr(I));

  end;

  TreeView1.Items[0].Expand(True);
end;

procedure TForm1.TreeView1DragDrop(Sender, Source: TObject; X, Y: Integer);
var src, dst : TTreeNode;
I : Integer;
begin
   dst := TreeView1.DropTarget;

   for I := 0 to TreeView1.SelectionCount - 1 do
   begin
     src := TreeView1.Selections[I];

     src.MoveTo(dst,naInsert);
   end;

end;

procedure TForm1.TreeView1DragOver(Sender, Source: TObject; X, Y: Integer;
  State: TDragState; var Accept: Boolean);
begin
Accept := true;
end;

add a from to a project, add a tree view set tree view dragmode dmAutomatic and multiselect true.

and then

select 3 consecutive node with following order with ctrl. select middle node, select bottom node, select top node. and drag the nodes by first node to another place you can see the AV error.

or select three node top to bottom and drag from bottom node AV arise.

or select three node in following order with control key :- first 'child 1' then 'child 2' then 'child 0' finally drag and drop node by selecting 'Child 0'

Answer 1

One clear problem is that when you call MoveTo, you invalidate the for loop.

 for I := 0 to TreeView1.SelectionCount - 1 do
 begin
   src := TreeView1.Selections[I];
   src.MoveTo(dst,naInsert);
 end;

After the call to MoveTo, you will find that SelectionCount is no longer what is was when you entered the loop. For instance, I'm looking at a case here where SelectionCount is 3 when the loop begins, but is 1 after the first call to MoveTo. That means that the subsequence use of Selections[I] are out of bounds.

You'll need to solve the problem by making a note of the selected nodes first, and then moving them.

procedure TForm1.TreeView1DragDrop(Sender, Source: TObject; X, Y: Integer);
var
  i: Integer;
  src, dst: TTreeNode;
  nodesToMove: TArray<TTreeNode>;
begin
  dst := TreeView1.DropTarget;
  SetLength(nodesToMove, TreeView1.SelectionCount);
  for i := 0 to high(nodesToMove) do
    nodesToMove[i] := TreeView1.Selections[i];
  for src in nodesToMove do
    src.MoveTo(dst, naInsert);
end;

---

Beyond that problem, I can reproduce the access violation. It seems that the items needs to be moved in a very particular order. It seems that you need to move the bottom node first, then the next preceding node, and the top node last. This code seems to workaround that problem:

procedure TForm1.TreeView1DragDrop(Sender, Source: TObject; X, Y: Integer);
var
  i: Integer;
  src, dst: TTreeNode;
  nodesToMove: TList<TTreeNode>;
begin
  dst := TreeView1.DropTarget;
  nodesToMove := TList<TTreeNode>.Create;
  try
    for i := TreeView1.Items.Count-1 downto 0 do
      if TreeView1.Items[i].Selected then
        nodesToMove.Add(TreeView1.Items[i]);
    for src in nodesToMove do
      src.MoveTo(dst, naInsert);
  finally
    nodesToMove.Free;
  end;
end;

It's not very satisfactory though, and it's clear that I've not yet understood what's going on here.

I cannot look into this any further right now, but I'll leave the answer here as I think it will help other answerers dig deeper. Hopefully somebody will be able to explain what's going on with the AV.

---

OK, I've dug a bit deeper. The problem appears to be related to the code inside MoveTo that tries to maintain the selection state of the node being moved. I've not yet dug into what the problem is, but it seems to me that you won't be able to do much from the outside to avoid the problem, beyond taking over implementation of selection preserving.

Accordingly I propose the following workaround as the best that I have come up with yet:

procedure TForm1.TreeView1DragDrop(Sender, Source: TObject; X, Y: Integer);
var
  i: Integer;
  src, dst: TTreeNode;
  nodesToMove: TArray<TTreeNode>;
begin
  dst := TreeView1.DropTarget;
  SetLength(nodesToMove, TreeView1.SelectionCount);
  for i := 0 to high(nodesToMove) do
    nodesToMove[i] := TreeView1.Selections[i];
  TreeView1.ClearSelection;
  for src in nodesToMove do
  begin
    src.MoveTo(dst, naInsert);
    TreeView1.Select(src, [ssCtrl]);
  end;
end;

Here we do the following:

1. Make a note of the selected nodes, the nodes that need to move.
2. Clear the selection.
3. Move each node to its new destination, and once moved, add that moved node to the selection.

Answer 2

This is a first chance exception thrown by the common controls library that you do not need to act upon. It can be a bug or a deliberate exception, in either case there's nothing to pursue, the exception is handled fine by the library itself.

The Delphi debugger may have a problem with handling the exception though. With my XE2 test, when I choose "continue" in the "debugger exception notification", I'd expect the program to continue running as normal. However, an exception dialog interrupts program execution. There's no problem running outside the debugger though, you will not see any kind of dialog that interrupts the move operation.

Note that this is only relevant with one of the duplication steps you present (the last one). With others, there's a 'list out of bounds" exception thrown by the RTL which is caused by your code, which you need to correct.