KEMBAR78
Revert "Add Accessibility Selected for iOS CollectionView (#29014)" by PureWeen · Pull Request #29827 · dotnet/maui · GitHub
Skip to content

Revert "Add Accessibility Selected for iOS CollectionView (#29014)" #29827

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 6, 2025
Merged

Revert "Add Accessibility Selected for iOS CollectionView (#29014)" #29827

merged 1 commit into from
Jun 6, 2025

Conversation

@PureWeen
Copy link
Member

@PureWeen PureWeen commented Jun 4, 2025 •
edited
Loading

Description of Change

From what I can tell so far what's happening is that the selecteditem change here

maui/src/Controls/src/Core/Handlers/Items/iOS/SelectableItemsViewController.cs

Lines 64 to 81 in b095e3d

void FormsSelectItem(NSIndexPath indexPath)
{
var mode = ItemsView.SelectionMode;
switch (mode)
{
case SelectionMode.None:
break;
case SelectionMode.Single:
ItemsView.SelectedItem = GetItemAtIndex(indexPath);
break;
case SelectionMode.Multiple:
ItemsView.SelectedItems.Add(GetItemAtIndex(indexPath));
break;
}
CollectionView.CellForItem(indexPath)?.UpdateSelectedAccessibility(true);
}

Fires on the AutoCompleteControl which then causes the AutoCompleteControl to close and invalidate the NSIndexPath, which then causes a crash on that subsequent line that's setting a value for accessibility.

I haven't added a test yet replicating the exception because I can't currently reproduce it in the same way. I've confirmed that this will fix the issue for the customer so I'm going to revert this for SR7 and SR8 for now and we will look at adding a more correct fix and set of tests for SR9

Issues Fixed

Fixes #29791

@Copilot Copilot AI review requested due to automatic review settings June 4, 2025 19:40
@PureWeen PureWeen requested a review from a team as a code owner June 4, 2025 19:40
@PureWeen PureWeen requested review from rmarinho and tj-devel709 June 4, 2025 19:40
Copilot
Copilot AI reviewed Jun 4, 2025
View reviewed changes
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 reverts the changes made in "Add Accessibility Selected for iOS CollectionView (#29014)" by removing tests and host app components related to the feature as well as reverting modifications to accessibility handling in iOS handlers and extension methods.

  • Removed automated tests from TestCases.Shared.Tests and host app pages related to Issue21375 and Issue21375_2.
  • Removed calls to UpdateSelectedAccessibility from iOS controller files and reverted the associated accessibility extension methods.
  • Deleted associated XAML files and backing code from the host app.

Reviewed Changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue21375_2.cs Removed tests for Issue21375_2.
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue21375.cs Removed tests for Issue21375.
src/Controls/tests/TestCases.HostApp/Issues/Issue21375_2.xaml.cs Removed host app code for Issue21375_2.
src/Controls/tests/TestCases.HostApp/Issues/Issue21375_2.xaml Removed XAML for Issue21375_2.
src/Controls/tests/TestCases.HostApp/Issues/Issue21375.xaml.cs Removed host app code for Issue21375.
src/Controls/tests/TestCases.HostApp/Issues/Issue21375.xaml Removed XAML for Issue21375.
src/Controls/src/Core/Platform/iOS/Extensions/AcessibilityExtensions.cs Reverted accessibility changes; notice the renaming change.
src/Controls/src/Core/Handlers/Items2/iOS/SelectableItemsViewController2.cs Removed calls to UpdateSelectedAccessibility to revert behavior.
src/Controls/src/Core/Handlers/Items/iOS/SelectableItemsViewController.cs Removed calls to UpdateSelectedAccessibility to revert behavior.
Comments suppressed due to low confidence (1)

src/Controls/src/Core/Platform/iOS/Extensions/AcessibilityExtensions.cs:5

  • The class name 'AcessibilityExtensions' seems to contain a typo. Consider renaming it to 'AccessibilityExtensions' to ensure consistency with naming conventions.
internal static class AcessibilityExtensions

@rmarinho
Copy link
Member

rmarinho commented Jun 5, 2025

/azp run

@azure-pipelines
Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@PureWeen PureWeen closed this Jun 5, 2025
@baaaaif
Copy link

baaaaif commented Jun 5, 2025

just fyi - i've linked this revert here
https://feedback.telerik.com/maui/1689494-ios-radcollectionview-nullreferenceexception-app-crash
I think similar accessibility changes in the Telerik CollectionView lead to crashes logged in Sentry.
Perhaps 1 or 2 sentences would be helpful as to why the code can remain

@PureWeen PureWeen reopened this Jun 5, 2025
@PureWeen PureWeen marked this pull request as draft June 5, 2025 21:30
@baaaaif
Copy link

baaaaif commented Jun 6, 2025

Hi @PureWeen ,
that was perhaps a bit misleading.
It's not the Maui changes themselves that are causing the crash in Telerik. But Telerik has implemented virtually the same accessibility code in its own RadCollectionView (at least that's what it looks like to me after researching the crash with ILSpy), and now I'm experiencing crashes.
But maybe a Telerik developer will chime in and comment on this :)

@PureWeen PureWeen marked this pull request as ready for review June 6, 2025 14:04
@PureWeen PureWeen changed the base branch from release/9.0.1xx-sr7 to main June 6, 2025 14:14
@PureWeen
Copy link
Member Author

PureWeen commented Jun 6, 2025

/backport to release/9.0.1xx-sr7

@github-actions
Copy link
Contributor

github-actions bot commented Jun 6, 2025

Started backporting to release/9.0.1xx-sr7: https://github.com/dotnet/maui/actions/runs/15492650967

@github-actions github-actions bot mentioned this pull request Jun 6, 2025
Merged
@rmarinho rmarinho merged commit 767fcfd into main Jun 6, 2025
122 of 129 checks passed
@rmarinho rmarinho deleted the revert_29014 branch June 6, 2025 23:17
@praveenkumarkarunanithi praveenkumarkarunanithi mentioned this pull request Jun 9, 2025
Merged
4 tasks
@Tamilarasan-Paranthaman Tamilarasan-Paranthaman mentioned this pull request Jun 25, 2025
Closed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code review Copilot Copilot left review comments

@rmarinho rmarinho rmarinho approved these changes

@tj-devel709 tj-devel709 Awaiting requested review from tj-devel709 tj-devel709 was automatically assigned from dotnet/dotnet-maui-reviewers

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Catalyst] CollectionView Crash

3 participants

@PureWeen @rmarinho @baaaaif