-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[iOS] Fixed the crash occurred on CarouselView2 when deleting last one remaining item with loop as false #31537
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
Conversation
…ning item with loop as false
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.
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)] |
Copilot
AI
Sep 9, 2025
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.
There's a spelling error in the issue description: 'occured' should be 'occurred'.
| [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.
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.
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"; |
Copilot
AI
Sep 9, 2025
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.
There's a spelling error in the issue description: 'occured' should be 'occurred'.
| 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.
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.
modified
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
|
||
| [Test] | ||
| [Category(UITestCategories.CarouselView)] | ||
| public void CarouselView2ShouldNotCrashOnRemoveLastItem() |
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.
Could include another test, similar but removing more than one time the last item?
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.
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
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.
Something like:
- Start with empty collection
- Add single item - should not crash
- Add multiple items - should work correctly
- Remove all items - should handle gracefully
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
| }; | ||
|
|
||
| // Create Button | ||
| var button = new Button |
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.
Can include other button to add items?
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.
Added the test covering scenarios.
- Started with Empty colleciton
2.Added Single Item
3.Removed single item
4.Added Multiple Items
5.Removed All Items
|
|
||
| [Test] | ||
| [Category(UITestCategories.CarouselView)] | ||
| public void CarouselView2ShouldNotCrashOnRemoveLastItem() |
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.
Something like:
- Start with empty collection
- Add single item - should not crash
- Add multiple items - should work correctly
- Remove all items - should handle gracefully
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
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
CarouselView2with 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
Reference
Issues Fixed
Fixes #31535
Regression PR's
Tested the behaviour in the following platforms
Screenshot
Cv2crashloopfalse.mov
Cv2crashfixed.mov