KEMBAR78
[iOS] Fixed the crash occurred on CarouselView2 when deleting last one remaining item with loop as false by Ahamed-Ali · Pull Request #31537 · dotnet/maui · GitHub
Skip to content

Conversation

@Ahamed-Ali
Copy link
Contributor

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Root Cause of the issue

  • When removing the last item in CarouselView2 with Loop = false, the item count is updated with an additional +2. This logic is specific to the looping scenario (to handle duplicate items), but there was no handling for the non-looping case. As a result, the update produced invalid indices and caused the exception.

Description of Change

  • Added proper handling for the non-looping case (Loop = false) to prevent exceptions when removing the last item in CarouselView2.

Reference

Issues Fixed

Fixes #31535

Regression PR's

Tested the behaviour in the following platforms

  • Android
  • Windows
  • iOS
  • Mac

Screenshot

Before Issue Fix After Issue Fix
Cv2crashloopfalse.mov
Cv2crashfixed.mov

@dotnet-policy-service dotnet-policy-service bot added community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration labels Sep 9, 2025
@Ahamed-Ali Ahamed-Ali marked this pull request as ready for review September 9, 2025 11:19
@Copilot Copilot AI review requested due to automatic review settings September 9, 2025 11:19
@Ahamed-Ali Ahamed-Ali requested a review from a team as a code owner September 9, 2025 11:19
@sheiksyedm sheiksyedm added platform/ios area-controls-collectionview CollectionView, CarouselView, IndicatorView collectionview-cv2 labels Sep 9, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a crash in CarouselView2 on iOS when removing the last remaining item with Loop set to false. The crash occurred because the existing logic was incorrectly adding +2 to the item count for all scenarios, but this additional count is only needed for looping carousels.

Key Changes:

  • Added proper handling for non-looping CarouselView2 scenarios to prevent invalid index calculations
  • Added comprehensive test coverage with both HostApp UI test page and NUnit test implementation

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/Controls/src/Core/Handlers/Items2/iOS/LoopObservableItemsSource2.cs Added conditional check to use base implementation when Loop is false
src/Controls/tests/TestCases.HostApp/Issues/Issue31535.cs Created UI test page demonstrating the crash scenario with CarouselView2
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue31535.cs Added NUnit test to verify the fix prevents crashes when removing items


namespace Maui.Controls.Sample.Issues;

[Issue(IssueTracker.Github, 31535, "[iOS] Crash occured on CarouselView2 when deleting last one remaining item with loop as false", PlatformAffected.iOS)]
Copy link

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a spelling error in the issue description: 'occured' should be 'occurred'.

Suggested change
[Issue(IssueTracker.Github, 31535, "[iOS] Crash occured on CarouselView2 when deleting last one remaining item with loop as false", PlatformAffected.iOS)]
[Issue(IssueTracker.Github, 31535, "[iOS] Crash occurred on CarouselView2 when deleting last one remaining item with loop as false", PlatformAffected.iOS)]

Copilot uses AI. Check for mistakes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modified

{
public Issue31535(TestDevice device) : base(device) { }

public override string Issue => "[iOS] Crash occured on CarouselView2 when deleting last one remaining item with loop as false";
Copy link

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a spelling error in the issue description: 'occured' should be 'occurred'.

Suggested change
public override string Issue => "[iOS] Crash occured on CarouselView2 when deleting last one remaining item with loop as false";
public override string Issue => "[iOS] Crash occurred on CarouselView2 when deleting last one remaining item with loop as false";

Copilot uses AI. Check for mistakes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modified

@Ahamed-Ali Ahamed-Ali changed the title [iOS] Fixed the crash occured on CarouselView2 when deleting last one remaining item with loop as false [iOS] Fixed the crash occurred on CarouselView2 when deleting last one remaining item with loop as false Sep 9, 2025
@jsuarezruiz
Copy link
Contributor

/azp run MAUI-UITests-public

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).


[Test]
[Category(UITestCategories.CarouselView)]
public void CarouselView2ShouldNotCrashOnRemoveLastItem()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could include another test, similar but removing more than one time the last item?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current test already performs the remove action twice, so it seems you might be referring to the same scenario. Could you please confirm if this is sufficient, or would you prefer that we add more items and invoke the remove function multiple times? @jsuarezruiz

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like:

  1. Start with empty collection
  2. Add single item - should not crash
  3. Add multiple items - should work correctly
  4. Remove all items - should handle gracefully

@mattleibow mattleibow added the p/0 Work that we can't release without label Sep 11, 2025
@mattleibow mattleibow moved this from Todo to Ready To Review in MAUI SDK Ongoing Sep 11, 2025
@mattleibow mattleibow moved this from Ready To Review to In Progress in MAUI SDK Ongoing Sep 11, 2025
@PureWeen PureWeen added this to the .NET 9 SR12 milestone Sep 11, 2025
@PureWeen PureWeen moved this from In Progress to Ready To Review in MAUI SDK Ongoing Sep 11, 2025
@PureWeen
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

};

// Create Button
var button = new Button
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can include other button to add items?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the test covering scenarios.

  1. Started with Empty colleciton
    2.Added Single Item
    3.Removed single item
    4.Added Multiple Items
    5.Removed All Items

@jsuarezruiz


[Test]
[Category(UITestCategories.CarouselView)]
public void CarouselView2ShouldNotCrashOnRemoveLastItem()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like:

  1. Start with empty collection
  2. Add single item - should not crash
  3. Add multiple items - should work correctly
  4. Remove all items - should handle gracefully

@github-project-automation github-project-automation bot moved this from Ready To Review to Changes Requested in MAUI SDK Ongoing Sep 18, 2025
@PureWeen
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@github-project-automation github-project-automation bot moved this from Changes Requested to Approved in MAUI SDK Ongoing Sep 23, 2025
@PureWeen PureWeen merged commit b321d0e into dotnet:main Sep 23, 2025
130 checks passed
@github-project-automation github-project-automation bot moved this from Approved to Done in MAUI SDK Ongoing Sep 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-controls-collectionview CollectionView, CarouselView, IndicatorView collectionview-cv2 community ✨ Community Contribution p/0 Work that we can't release without partner/syncfusion Issues / PR's with Syncfusion collaboration platform/ios

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[iOS] Crash occurred on CarouselView2 when deleting last one remaining item with loop as false

5 participants