Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,19 @@ void CollectionChanged(NotifyCollectionChangedEventArgs args)
if (!_collectionViewController.TryGetTarget(out var controller))
return;

// Force UICollectionView to get the internal accounting straight
var collectionView = controller.CollectionView;
UpdateSection(collectionView);

// Force UICollectionView to get the internal accounting straight.
// Skip for Remove: standard INotifyCollectionChanged semantics require the item to be
// already removed from the backing collection before CollectionChanged fires. This means
// _groupSource has N-1 items when we receive Remove, but UIKit still has N sections.
// Calling UpdateSection here triggers GetGroupCount with UIKit's stale section index
// (N-1), which is out of range on the already-mutated _groupSource → ArgumentOutOfRangeException.
// The post-processing UpdateSection call below handles reconciliation after DeleteSections.
if (args.Action != NotifyCollectionChangedAction.Remove)
{
UpdateSection(collectionView);
}

switch (args.Action)
{
Expand Down Expand Up @@ -360,6 +370,12 @@ void Move(NotifyCollectionChangedEventArgs args)

int GetGroupCount(int groupIndex)
{
// Defensive bounds check: UIKit can call back synchronously during DeleteSections,
// and other re-entrancy scenarios may present a stale groupIndex. Return 0 instead
// of throwing ArgumentOutOfRangeException.
if ((uint)groupIndex >= (uint)_groupSource.Count)
return 0;

switch (_groupSource[groupIndex])
{
case IList list:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =>
Copy link
Copy Markdown
Collaborator

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 calls SetupBuilder() and never registers CollectionViewHandler2 for CollectionView. The handler lookup therefore throws I can't work with Microsoft.Maui.Controls.Handlers.Items2.CollectionViewHandler2 before the remove scenario is exercised, matching the supplied gate failure. Please register Handler2 for these tests, e.g. via EnsureHandlerCreated/ConfigureMauiHandlers(handlers => handlers.AddHandler<CollectionView, CollectionViewHandler2>()), or use the actually registered handler type. The same issue applies to the two other new tests using CollectionViewHandler2.

{
await WaitForUIUpdate(initialFrame, collectionView);
var uiCollectionView = handler.Controller.CollectionView;

groupData.RemoveAt(groupData.Count - 1);

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; }
Expand Down
Loading