diff --git a/src/Avalonia.Controls/IndexPath.cs b/src/Avalonia.Controls/IndexPath.cs index 32d8c2b051..6c5aaf7ad1 100644 --- a/src/Avalonia.Controls/IndexPath.cs +++ b/src/Avalonia.Controls/IndexPath.cs @@ -58,6 +58,11 @@ namespace Avalonia.Controls public int GetAt(int index) { + if (index >= GetSize()) + { + throw new IndexOutOfRangeException(); + } + return _path?[index] ?? (_index - 1); } @@ -169,5 +174,7 @@ namespace Avalonia.Controls public static bool operator >=(IndexPath x, IndexPath y) { return x.CompareTo(y) >= 0; } public static bool operator ==(IndexPath x, IndexPath y) { return x.CompareTo(y) == 0; } public static bool operator !=(IndexPath x, IndexPath y) { return x.CompareTo(y) != 0; } + public static bool operator ==(IndexPath? x, IndexPath? y) { return (x ?? default).CompareTo(y ?? default) == 0; } + public static bool operator !=(IndexPath? x, IndexPath? y) { return (x ?? default).CompareTo(y ?? default) != 0; } } } diff --git a/src/Avalonia.Controls/SelectionModel.cs b/src/Avalonia.Controls/SelectionModel.cs index e5f79fa40f..a50cf841a1 100644 --- a/src/Avalonia.Controls/SelectionModel.cs +++ b/src/Avalonia.Controls/SelectionModel.cs @@ -36,13 +36,29 @@ namespace Avalonia.Controls get => _rootNode?.Source; set { - using (var operation = new Operation(this)) + var wasNull = _rootNode.Source == null; + + if (_rootNode.Source != null) { - ClearSelection(resetAnchor: true); + using (var operation = new Operation(this)) + { + ClearSelection(resetAnchor: true); + } } _rootNode.Source = value; + RaisePropertyChanged("Source"); + + if (wasNull) + { + var e = new SelectionModelSelectionChangedEventArgs( + null, + SelectedIndices, + null, + SelectedItems); + OnSelectionChanged(e); + } } } diff --git a/src/Avalonia.Controls/SelectionModelChangeSet.cs b/src/Avalonia.Controls/SelectionModelChangeSet.cs index c0228a1cbc..bff84eca92 100644 --- a/src/Avalonia.Controls/SelectionModelChangeSet.cs +++ b/src/Avalonia.Controls/SelectionModelChangeSet.cs @@ -1,6 +1,8 @@ using System; using System.Collections.Generic; +#nullable enable + namespace Avalonia.Controls { internal class SelectionModelChangeSet @@ -14,30 +16,38 @@ namespace Avalonia.Controls public SelectionModelSelectionChangedEventArgs CreateEventArgs() { - var deselectedCount = 0; - var selectedCount = 0; + var deselectedIndexCount = 0; + var selectedIndexCount = 0; + var deselectedItemCount = 0; + var selectedItemCount = 0; foreach (var change in _changes) { - deselectedCount += change.DeselectedCount; - selectedCount += change.SelectedCount; + deselectedIndexCount += change.DeselectedCount; + selectedIndexCount += change.SelectedCount; + + if (change.Items != null) + { + deselectedItemCount += change.DeselectedCount; + selectedItemCount += change.SelectedCount; + } } var deselectedIndices = new SelectedItems( _changes, - deselectedCount, + deselectedIndexCount, GetDeselectedIndexAt); var selectedIndices = new SelectedItems( _changes, - selectedCount, + selectedIndexCount, GetSelectedIndexAt); - var deselectedItems = new SelectedItems( + var deselectedItems = new SelectedItems( _changes, - deselectedCount, + deselectedItemCount, GetDeselectedItemAt); - var selectedItems = new SelectedItems( + var selectedItems = new SelectedItems( _changes, - selectedCount, + selectedItemCount, GetSelectedItemAt); return new SelectionModelSelectionChangedEventArgs( @@ -52,7 +62,7 @@ namespace Avalonia.Controls int index) { static int GetCount(SelectionNodeOperation info) => info.DeselectedCount; - static List GetRanges(SelectionNodeOperation info) => info.DeselectedRanges; + static List? GetRanges(SelectionNodeOperation info) => info.DeselectedRanges; return GetIndexAt(infos, index, GetCount, GetRanges); } @@ -61,25 +71,25 @@ namespace Avalonia.Controls int index) { static int GetCount(SelectionNodeOperation info) => info.SelectedCount; - static List GetRanges(SelectionNodeOperation info) => info.SelectedRanges; + static List? GetRanges(SelectionNodeOperation info) => info.SelectedRanges; return GetIndexAt(infos, index, GetCount, GetRanges); } - private object GetDeselectedItemAt( + private object? GetDeselectedItemAt( List infos, int index) { - static int GetCount(SelectionNodeOperation info) => info.DeselectedCount; - static List GetRanges(SelectionNodeOperation info) => info.DeselectedRanges; + static int GetCount(SelectionNodeOperation info) => info.Items != null ? info.DeselectedCount : 0; + static List? GetRanges(SelectionNodeOperation info) => info.DeselectedRanges; return GetItemAt(infos, index, GetCount, GetRanges); } - private object GetSelectedItemAt( + private object? GetSelectedItemAt( List infos, int index) { - static int GetCount(SelectionNodeOperation info) => info.SelectedCount; - static List GetRanges(SelectionNodeOperation info) => info.SelectedRanges; + static int GetCount(SelectionNodeOperation info) => info.Items != null ? info.SelectedCount : 0; + static List? GetRanges(SelectionNodeOperation info) => info.SelectedRanges; return GetItemAt(infos, index, GetCount, GetRanges); } @@ -87,7 +97,7 @@ namespace Avalonia.Controls List infos, int index, Func getCount, - Func> getRanges) + Func?> getRanges) { var currentIndex = 0; IndexPath path = default; @@ -109,14 +119,14 @@ namespace Avalonia.Controls return path; } - private object GetItemAt( + private object? GetItemAt( List infos, int index, Func getCount, - Func> getRanges) + Func?> getRanges) { var currentIndex = 0; - object item = null; + object? item = null; foreach (var info in infos) { @@ -125,7 +135,7 @@ namespace Avalonia.Controls if (index >= currentIndex && index < currentIndex + currentCount) { int targetIndex = GetIndexAt(getRanges(info), index - currentIndex); - item = info.Items.GetAt(targetIndex); + item = info.Items?.GetAt(targetIndex); break; } @@ -135,20 +145,23 @@ namespace Avalonia.Controls return item; } - private int GetIndexAt(List ranges, int index) + private int GetIndexAt(List? ranges, int index) { var currentIndex = 0; - foreach (var range in ranges) + if (ranges != null) { - var currentCount = (range.End - range.Begin) + 1; - - if (index >= currentIndex && index < currentIndex + currentCount) + foreach (var range in ranges) { - return range.Begin + (index - currentIndex); - } + var currentCount = (range.End - range.Begin) + 1; - currentIndex += currentCount; + if (index >= currentIndex && index < currentIndex + currentCount) + { + return range.Begin + (index - currentIndex); + } + + currentIndex += currentCount; + } } throw new IndexOutOfRangeException(); diff --git a/src/Avalonia.Controls/SelectionModelSelectionChangedEventArgs.cs b/src/Avalonia.Controls/SelectionModelSelectionChangedEventArgs.cs index 4e64ee6e6f..5e2efdf331 100644 --- a/src/Avalonia.Controls/SelectionModelSelectionChangedEventArgs.cs +++ b/src/Avalonia.Controls/SelectionModelSelectionChangedEventArgs.cs @@ -15,13 +15,13 @@ namespace Avalonia.Controls public SelectionModelSelectionChangedEventArgs( IReadOnlyList? deselectedIndices, IReadOnlyList? selectedIndices, - IReadOnlyList? deselectedItems, - IReadOnlyList? selectedItems) + IReadOnlyList? deselectedItems, + IReadOnlyList? selectedItems) { DeselectedIndices = deselectedIndices ?? Array.Empty(); SelectedIndices = selectedIndices ?? Array.Empty(); - DeselectedItems = deselectedItems ?? Array.Empty(); - SelectedItems= selectedItems ?? Array.Empty(); + DeselectedItems = deselectedItems ?? Array.Empty(); + SelectedItems= selectedItems ?? Array.Empty(); } /// @@ -37,11 +37,11 @@ namespace Avalonia.Controls /// /// Gets the items that were removed from the selection. /// - public IReadOnlyList DeselectedItems { get; } + public IReadOnlyList DeselectedItems { get; } /// /// Gets the items that were added to the selection. /// - public IReadOnlyList SelectedItems { get; } + public IReadOnlyList SelectedItems { get; } } } diff --git a/src/Avalonia.Controls/SelectionNode.cs b/src/Avalonia.Controls/SelectionNode.cs index a8ad634d35..ea1a1ca664 100644 --- a/src/Avalonia.Controls/SelectionNode.cs +++ b/src/Avalonia.Controls/SelectionNode.cs @@ -84,8 +84,11 @@ namespace Avalonia.Controls { if (_source != value) { - ClearSelection(); - UnhookCollectionChangedHandler(); + if (_source != null) + { + ClearSelection(); + UnhookCollectionChangedHandler(); + } _source = value; diff --git a/src/Avalonia.Controls/Utils/SelectionTreeHelper.cs b/src/Avalonia.Controls/Utils/SelectionTreeHelper.cs index 93102a7b5b..430ecabbb8 100644 --- a/src/Avalonia.Controls/Utils/SelectionTreeHelper.cs +++ b/src/Avalonia.Controls/Utils/SelectionTreeHelper.cs @@ -132,15 +132,21 @@ namespace Avalonia.Controls.Utils private static bool IsSubSet(IndexPath path, IndexPath subset) { - bool isSubset = true; - for (int i = 0; i < subset.GetSize(); i++) + var subsetSize = subset.GetSize(); + if (path.GetSize() < subsetSize) { - isSubset = path.GetAt(i) == subset.GetAt(i); - if (!isSubset) - break; + return false; + } + + for (int i = 0; i < subsetSize; i++) + { + if (path.GetAt(i) != subset.GetAt(i)) + { + return false; + } } - return isSubset; + return true; } private static IndexPath StartPath(IndexPath path, int length) diff --git a/tests/Avalonia.Controls.UnitTests/IndexPathTests.cs b/tests/Avalonia.Controls.UnitTests/IndexPathTests.cs index 190e92ed5e..1e4aa0a2b8 100644 --- a/tests/Avalonia.Controls.UnitTests/IndexPathTests.cs +++ b/tests/Avalonia.Controls.UnitTests/IndexPathTests.cs @@ -77,5 +77,19 @@ namespace Avalonia.Controls.UnitTests Assert.Equal(0, a.CompareTo(b)); Assert.Equal(a.GetHashCode(), b.GetHashCode()); } + + [Fact] + public void Null_Equality() + { + var a = new IndexPath(null); + var b = new IndexPath(1); + + // Implementing operator == on a struct automatically implements an operator which + // accepts null, so make sure this does something useful. + Assert.True(a == null); + Assert.False(a != null); + Assert.False(b == null); + Assert.True(b != null); + } } } diff --git a/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs b/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs index b3a5e0959f..9ce0816f7d 100644 --- a/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs +++ b/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs @@ -899,6 +899,38 @@ namespace Avalonia.Controls.UnitTests }); } + [Fact] + public void SelectRangeRegressionTest() + { + RunOnUIThread.Execute(() => + { + var selectionModel = new SelectionModel() + { + Source = CreateNestedData(1, 2, 3) + }; + + // length of start smaller than end used to cause an out of range error. + selectionModel.SelectRange(IndexPath.CreateFrom(0), IndexPath.CreateFrom(1, 1)); + + ValidateSelection(selectionModel, + new List() + { + Path(0, 0), + Path(0, 1), + Path(0, 2), + Path(0), + Path(1, 0), + Path(1, 1) + }, + new List() + { + Path(), + Path(1) + }, + 1 /* selectedInnerNodes */); + }); + } + [Fact] public void Selecting_Item_Raises_SelectionChanged() { @@ -1393,6 +1425,48 @@ namespace Avalonia.Controls.UnitTests Assert.Equal(2, raised); } + [Fact] + public void Raises_SelectionChanged_With_No_Source() + { + var target = new SelectionModel(); + var raised = 0; + + target.SelectionChanged += (s, e) => + { + Assert.Empty(e.DeselectedIndices); + Assert.Empty(e.DeselectedItems); + Assert.Equal(new[] { new IndexPath(1) }, e.SelectedIndices); + Assert.Empty(e.SelectedItems); + ++raised; + }; + + target.Select(1); + + Assert.Equal(new[] { new IndexPath(1) }, target.SelectedIndices); + Assert.Empty(target.SelectedItems); + } + + [Fact] + public void Raises_SelectionChanged_With_Items_After_Source_Is_Set() + { + var target = new SelectionModel(); + var raised = 0; + + target.Select(1); + target.SelectionChanged += (s, e) => + { + Assert.Empty(e.DeselectedIndices); + Assert.Empty(e.DeselectedItems); + Assert.Equal(new[] { new IndexPath(1) }, e.SelectedIndices); + Assert.Equal(new[] { "bar" }, e.SelectedItems); + ++raised; + }; + + target.Source = new[] { "foo", "bar", "baz" }; + + Assert.Equal(1, raised); + } + [Fact] public void RetainSelectionOnReset_Retains_Selection_On_Reset() { @@ -1494,6 +1568,7 @@ namespace Avalonia.Controls.UnitTests Assert.Equal(1, raised); } + private int GetSubscriberCount(AvaloniaList list) { return ((INotifyCollectionChangedDebug)list).GetCollectionChangedSubscribers()?.Length ?? 0;