A loooooooooooong bug in IterateSubtree

[mksjgj]

now, i have time to report a very very old bug in IterateSubtree When you call in this form: ATree.IterateSubtree(nil, ACallback, nil, [], False, True); i mean: Node is nil, and ChildNodesOnly is true Your intention is to traverse the entire tree nodes (except for the Ultra root), but the first tree node will not actually be accessed The reason for this is that there is a problem with the logic of dealing with Node = Nil and Child Nordsenley = True.

function TBaseVirtualTree.IterateSubtree(Node: PVirtualNode; Callback: TVTGetNodeProc; AData: Pointer;
  Filter: TVirtualNodeStates = []; DoInit: Boolean = False; ChildNodesOnly: Boolean = False): PVirtualNode;

// Iterates through the all children and grandchildren etc. of Node (or the entire tree if Node = nil)
// and calls for each node the provided callback method (which must not be empty).
// Filter determines which nodes to consider (an empty set denotes all nodes).
// If DoInit is True then nodes which aren't initialized yet will be initialized.
// Note: During execution of the callback the application can set Abort to True. In this case the iteration is stopped
//       and the last accessed node (the one on which the callback set Abort to True) is returned to the caller.
//       Otherwise (no abort) nil is returned.

var
  Stop: PVirtualNode;
  Abort: Boolean;
  GetNextNode: TGetNextNodeProc;
  WasIterating: Boolean;

begin
  Assert(Node <> FRoot, 'Node must not be the hidden root node.');

  WasIterating := tsIterating in FStates;
  DoStateChange([tsIterating]);
  try
    // prepare function to be used when advancing
    if DoInit then
      GetNextNode := GetNext
    else
      GetNextNode := GetNextNoInit;

    Abort := False;
    if Node = nil then
      Stop := nil
    else
    begin
      if not (vsInitialized in Node.States) and DoInit then
        InitNode(Node);

      // The stopper does not need to be initialized since it is not taken into the enumeration.
      Stop := Node.NextSibling;
      if Stop = nil then
      begin
        Stop := Node;
        repeat
          Stop := Stop.Parent;
        until (Stop = FRoot) or Assigned(Stop.NextSibling);
        if Stop = FRoot then
          Stop := nil
        else
          Stop := Stop.NextSibling;
      end;
    end;

    // Use first node if we start with the root.
    if Node = nil then //<------------------------------- bug is here#1
      Node := GetFirstNoInit;

    if Assigned(Node) then
    begin
      if not (vsInitialized in Node.States) and DoInit then
        InitNode(Node);

      // Skip given node if only the child nodes are requested.
      if ChildNodesOnly then //<------------------------------- bug is here#2
      begin
        if Node.ChildCount = 0 then
          Node := nil
        else
          Node := GetNextNode(Node);
      end;

      if Filter = [] then
      begin
        // unfiltered loop
        while Assigned(Node) and (Node <> Stop) do
        begin
          Callback(Self, Node, AData, Abort);
          if Abort then
            Break;
          Node := GetNextNode(Node);
        end;
      end
      else
      begin
        // filtered loop
        while Assigned(Node) and (Node <> Stop) do
        begin
          if Node.States * Filter = Filter then
            Callback(Self, Node, AData, Abort);
          if Abort then
            Break;
          Node := GetNextNode(Node);
        end;
      end;
    end;

    if Abort then
      Result := Node
    else
      Result := nil;
  finally
    if not WasIterating then
      DoStateChange([], [tsIterating]);
  end;
end;

[joachimmarder]

Please respect our guidelines on the project homepage for submitting bugs. Please include your version of Virtual TreeView and Delphi, and attach a sample compiling project as ZIP to your report that allows to replicate the problem. If only small changes are required, a description is sufficient how a demo projects needs to be changed in order to replicate the bug. If you already have a solution, please supply a patch file.

[ErwinDenissen]

This bug fix now causes an issue with: IterateSubtree(nil, Callback, nil, [], True, True);

  // Skip given node if only the child nodes are requested.
  if ChildNodesOnly then
  begin
    if Node.ChildCount = 0 then
      Node := nil
    else
      Node := GetNextNode(Node); <-- Node must not be the hidden root node.
  end;

[joachimmarder]

This bug fix now causes an issue with: terateSubtree(nil, Callback, nil, [], True, True);

I'm sorry, I reverted my commit and tried another fix. Please test if the new code word as expected, @ErwinDenissen and @mksjgj .

[ErwinDenissen]

The issue on my side is solved.