Fix Clear and Remove after Sort in TreeView (#10377)

* Fix Clear and Remove after Sort in TreeView * Always set the index * Use old implementation * Use list instead of array * Invert if
dotnet · Nov 29, 2023 · 172405c · 172405c
1 parent 36a1b26
commit 172405c
Show file tree

Hide file tree

Showing 2 changed files with 118 additions and 11 deletions.
diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/Controls/TreeView/TreeNode.cs b/src/System.Windows.Forms/src/System/Windows/Forms/Controls/TreeView/TreeNode.cs
@@ -88,7 +88,7 @@ internal TreeNodeImageIndexer StateImageIndexer
     }
 
     internal int _index;                  // our index into our parents child array
-    internal readonly List<TreeNode> _childNodes = new();
+    internal List<TreeNode> _childNodes = new();
     internal TreeNode? _parent;
     internal TreeView? _treeView;
     private bool _expandOnRealization;
@@ -1194,23 +1194,83 @@ internal int AddSorted(TreeView parentTreeView, TreeNode node)
 
     private void SortChildren(TreeView? parentTreeView)
     {
-        var nodeCount = _childNodes.Count;
-        if (nodeCount > 0)
+        if (_childNodes.Count <= 0)
         {
-            List<TreeNode> newOrder = new(nodeCount);
-            if (parentTreeView?.TreeViewNodeSorter is null)
-            {
-                _childNodes.Sort((x, y) => Application.CurrentCulture.CompareInfo.Compare(x.Text, y.Text, CompareOptions.None));
-            }
-            else
+            return;
+        }
+
+        List<TreeNode> newOrder = new(_childNodes.Count);
+        if (parentTreeView is null || parentTreeView.TreeViewNodeSorter is null)
+        {
+            CompareInfo compare = Application.CurrentCulture.CompareInfo;
+            for (int i = 0; i < _childNodes.Count; i++)
             {
-                _childNodes.Sort(parentTreeView.TreeViewNodeSorter.Compare);
+                newOrder.Add(_childNodes[i]);
+
+                int min = -1;
+                for (int j = 0; j < _childNodes.Count; j++)
+                {
+                    if (_childNodes[j] is null)
+                    {
+                        continue;
+                    }
+
+                    if (min == -1)
+                    {
+                        min = j;
+                        continue;
+                    }
+
+                    if (compare.Compare(_childNodes[j].Text, _childNodes[min].Text) <= 0)
+                    {
+                        min = j;
+                    }
+                }
+
+                Debug.Assert(min != -1, "Bad sorting");
+                newOrder[i] = _childNodes[min];
+                _childNodes[min] = null!;
+                newOrder[i]._index = i;
+                newOrder[i].SortChildren(parentTreeView);
             }
 
+            _childNodes = newOrder;
+        }
+        else
+        {
+            IComparer sorter = parentTreeView.TreeViewNodeSorter;
             for (int i = 0; i < _childNodes.Count; i++)
             {
-                _childNodes[i].SortChildren(parentTreeView);
+                newOrder.Add(_childNodes[i]);
+
+                int min = -1;
+                for (int j = 0; j < _childNodes.Count; j++)
+                {
+                    if (_childNodes[j] is null)
+                    {
+                        continue;
+                    }
+
+                    if (min == -1)
+                    {
+                        min = j;
+                        continue;
+                    }
+
+                    if (sorter.Compare(_childNodes[j] /*previous*/, _childNodes[min] /*current*/) <= 0)
+                    {
+                        min = j;
+                    }
+                }
+
+                Debug.Assert(min != -1, "Bad sorting");
+                newOrder[i] = _childNodes[min];
+                _childNodes[min] = null!;
+                newOrder[i]._index = i;
+                newOrder[i].SortChildren(parentTreeView);
             }
+
+            _childNodes = newOrder;
         }
     }
 

diff --git a/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/TreeViewTests.cs b/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/TreeViewTests.cs
@@ -7113,6 +7113,53 @@ public void TreeView_TreeViewNodeSorter_ComparesTreeNodes()
         treeView.TreeViewNodeSorter = treeSorter;
     }
 
+    [WinFormsTheory]
+    [InlineData("A", "B")]
+    [InlineData("A", "A")]
+    [InlineData("B", "A")]
+    public void Clear_AfterSort_ShouldNotGetStuck(string firstNodeText, string secondNodeText)
+    {
+        using TreeView treeView = new();
+        TreeNode parent = new("Parent");
+        treeView.Nodes.Add(parent);
+
+        TreeNode firstNode = new(firstNodeText);
+        parent.Nodes.Add(firstNode);
+        TreeNode secondNode = new(secondNodeText);
+        parent.Nodes.Add(secondNode);
+
+        treeView.Sort();
+
+        Action action = () => parent.Nodes.Clear();
+
+        action.ExecutionTime().Should().BeLessThanOrEqualTo(TimeSpan.FromMilliseconds(500));
+        action.Should().NotThrow();
+    }
+
+    [WinFormsTheory]
+    [InlineData("A", "B")]
+    [InlineData("A", "A")]
+    [InlineData("B", "A")]
+    public void Remove_AfterSort_ShouldNotThrowException(string firstNodeText, string secondNodeText)
+    {
+        using TreeView treeView = new();
+        TreeNode parent = new("Parent");
+        treeView.Nodes.Add(parent);
+
+        TreeNode firstNode = new(firstNodeText);
+        parent.Nodes.Add(firstNode);
+        TreeNode secondNode = new(secondNodeText);
+        parent.Nodes.Add(secondNode);
+
+        treeView.Sort();
+
+        parent.Nodes.Remove(firstNode);
+
+        Action action = () => parent.Nodes.Remove(secondNode);
+
+        action.Should().NotThrow();
+    }
+
     private class SubTreeView : TreeView
     {
         public new bool CanEnableIme => base.CanEnableIme;