-
Notifications
You must be signed in to change notification settings - Fork 2k
[iOS] Fix ArgumentOutOfRangeException in ObservableGroupedSource when removing a group from grouped CollectionView #34692
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1db8c3b
de54a54
ea354b4
4d933de
a7a523a
1f00297
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,6 +49,125 @@ await CreateHandlerAndAddToWindow<CollectionViewHandler>(collectionView, async h | |
| }); | ||
| } | ||
|
|
||
| // Regression tests for https://github.com/dotnet/maui/issues/34691 | ||
| // ObservableGroupedSource.CollectionChanged() called UpdateSection() before Remove, | ||
| // causing GetGroupCount to access a stale section index → ArgumentOutOfRangeException. | ||
| // Each test forces a synchronous UIKit layout pass after removal to exercise the fix. | ||
|
|
||
| [Fact(DisplayName = "Removing last section from grouped CollectionView does not crash (Handler2)")] | ||
| public async Task ItemsSourceGroupedRemoveLastSectionDoesNotCrash() | ||
| { | ||
| SetupBuilder(); | ||
|
|
||
| var data = new List<string> { "item 1", "item 2", "item 3" }; | ||
| var groupData = new ObservableCollection<CollectionViewStringGroup> | ||
| { | ||
| new("Header 1", data), | ||
| new("Header 2", data), | ||
| new("Header 3", data), | ||
| }; | ||
|
|
||
| var collectionView = new CollectionView | ||
| { | ||
| IsGrouped = true, | ||
| ItemsSource = groupData, | ||
| ItemTemplate = new DataTemplate(() => new Label()), | ||
| }; | ||
|
|
||
| var initialFrame = collectionView.Frame; | ||
|
|
||
| await CreateHandlerAndAddToWindow<CollectionViewHandler2>(collectionView, async handler => | ||
| { | ||
| await WaitForUIUpdate(initialFrame, collectionView); | ||
| var uiCollectionView = handler.Controller.CollectionView; | ||
|
|
||
| groupData.RemoveAt(groupData.Count - 1); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [major] Regression Prevention — This removes the backing group before verifying that the native UICollectionView has realized the original sections. If |
||
|
|
||
| uiCollectionView.SetNeedsLayout(); | ||
| uiCollectionView.LayoutIfNeeded(); | ||
|
|
||
| Assert.Equal(groupData.Count, (int)uiCollectionView.NumberOfSections()); | ||
| }); | ||
| } | ||
|
|
||
| [Fact(DisplayName = "Removing first section from grouped CollectionView does not crash (Handler2)")] | ||
| public async Task ItemsSourceGroupedRemoveFirstSectionDoesNotCrash() | ||
| { | ||
| SetupBuilder(); | ||
|
|
||
| var data = new List<string> { "item 1", "item 2" }; | ||
| var groupData = new ObservableCollection<CollectionViewStringGroup> | ||
| { | ||
| new("Header 1", data), | ||
| new("Header 2", data), | ||
| new("Header 3", data), | ||
| }; | ||
|
|
||
| var collectionView = new CollectionView | ||
| { | ||
| IsGrouped = true, | ||
| ItemsSource = groupData, | ||
| ItemTemplate = new DataTemplate(() => new Label()), | ||
| }; | ||
|
|
||
| var initialFrame = collectionView.Frame; | ||
|
|
||
| await CreateHandlerAndAddToWindow<CollectionViewHandler2>(collectionView, async handler => | ||
| { | ||
| await WaitForUIUpdate(initialFrame, collectionView); | ||
| var uiCollectionView = handler.Controller.CollectionView; | ||
|
|
||
| groupData.RemoveAt(0); | ||
| uiCollectionView.SetNeedsLayout(); | ||
| uiCollectionView.LayoutIfNeeded(); | ||
| Assert.Equal(groupData.Count, (int)uiCollectionView.NumberOfSections()); | ||
|
|
||
| groupData.RemoveAt(0); | ||
| uiCollectionView.SetNeedsLayout(); | ||
| uiCollectionView.LayoutIfNeeded(); | ||
| Assert.Equal(groupData.Count, (int)uiCollectionView.NumberOfSections()); | ||
| }); | ||
| } | ||
|
|
||
| [Fact(DisplayName = "Removing all sections one by one from grouped CollectionView does not crash (Handler2)")] | ||
| public async Task ItemsSourceGroupedRemoveAllSectionsOneByOneDoesNotCrash() | ||
| { | ||
| SetupBuilder(); | ||
|
|
||
| var data = new List<string> { "item 1", "item 2" }; | ||
| var groupData = new ObservableCollection<CollectionViewStringGroup> | ||
| { | ||
| new("Header 1", data), | ||
| new("Header 2", data), | ||
| new("Header 3", data), | ||
| }; | ||
|
|
||
| var collectionView = new CollectionView | ||
| { | ||
| IsGrouped = true, | ||
| ItemsSource = groupData, | ||
| ItemTemplate = new DataTemplate(() => new Label()), | ||
| }; | ||
|
|
||
| var initialFrame = collectionView.Frame; | ||
|
|
||
| await CreateHandlerAndAddToWindow<CollectionViewHandler2>(collectionView, async handler => | ||
| { | ||
| await WaitForUIUpdate(initialFrame, collectionView); | ||
| var uiCollectionView = handler.Controller.CollectionView; | ||
|
|
||
| while (groupData.Count > 0) | ||
| { | ||
| groupData.RemoveAt(groupData.Count - 1); | ||
| uiCollectionView.SetNeedsLayout(); | ||
| uiCollectionView.LayoutIfNeeded(); | ||
| Assert.Equal(groupData.Count, (int)uiCollectionView.NumberOfSections()); | ||
| } | ||
|
|
||
| Assert.Empty(groupData); | ||
| }); | ||
| } | ||
|
|
||
| class CollectionViewStringGroup : List<string> | ||
| { | ||
| public string GroupHeader { get; private set; } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[major] Regression Prevention - These new tests request
CreateHandlerAndAddToWindow<CollectionViewHandler2>(), but the test setup only callsSetupBuilder()and never registersCollectionViewHandler2forCollectionView. The handler lookup therefore throwsI can't work with Microsoft.Maui.Controls.Handlers.Items2.CollectionViewHandler2before the remove scenario is exercised, matching the supplied gate failure. Please register Handler2 for these tests, e.g. viaEnsureHandlerCreated/ConfigureMauiHandlers(handlers => handlers.AddHandler<CollectionView, CollectionViewHandler2>()), or use the actually registered handler type. The same issue applies to the two other new tests usingCollectionViewHandler2.