Skip to content
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

Fix ListView is in virtual mode #3963

Conversation

SergeySmirnov-Akvelon
Copy link
Contributor

@SergeySmirnov-Akvelon SergeySmirnov-Akvelon commented Sep 21, 2020

Fixes #3962

Proposed changes

  • implemented using SelectedIndices instead of SelectedItems when getting Accessible Objects, because SelectedIndices property is available in any mode (virtual and default);
  • added a check for methods from ListViewGroup that the ListView is not in virtual mode, since ListViewGroups are not supported for virtual mode;
  • replaced "groups.Count > 0" condition on "GroupsEnabled" in logic for navigation on ListView by keyboard. Together with the "groups.Count > 0" condition, it also checks that Groups is displayed and ListView is in default (not virtual) mode;
  • updated logic for "ExpandCollapseState" property ;
  • added the condition that if a ListViewItem is located in a ListViewGroup and this ListViewGroup is collapsed we return empty rectangle otherwise we call old logic for getting of rectangle
  • added check that the index should be greater than -1 and less than the number of Items in the ListView. If check is failed then we will return false, otherwise we will get the ListViewItem using index and check "Selected" property;

Customer Impact

ListView in virtual mode
During implementation of accessible objects for ListView we started to use "SelectedItems" properties for getting of required items. Unfortunately, "SelectedItems", "CheckedItems" and partially "Items" properties are not available when the ListView is in virtual mode. If trying to get any properties/methods from "SelecedItems", "CheckedItems" or get enumerator of "Items" then ListView throws IOE in virtual mode. As result we got an exceptions when tried to do any UI action when ListView is in virtual mode.
93773875-403fc080-fc29-11ea-926b-9d671694d720
After fixing issue with ListView in a virtual mode, it is again available for navigation and UI interaction:
ListView in virtual mode
ListViewGroup.ListViewGroupAccessibleObject.ExpandCollapseState
The ExpandCollapseState is not valid for non-collapsible ListViewGroup. User sees that such group has "Collapsed" state although the group cannot be collapsed:
Issue-3962-collapsed
After fixing, ExpandCollapseState is valid for non-collapsible ListViewGroup:
Issue-3962-expanded

ListViewItem.ListViewItemAccessibleObject.Bounds
If a ListViewItem is in a collapsed ListViewGroup and user tries to selects it in Inspector then user will see exception:
ListView-collapsedFails
After fixing, user can see data about ListViewItem in a collapsed ListViewGroup without application exception:
ListView-collapsedSuccesd

Regression?

  • Yes

Risk

  • Minimal

Test methodology

  • Manually
  • Unit tests
  • CTI

Test environment(s)

  • Microsoft Windows [Version 10.0.19041.388]
  • NET Core 5.0.100-rc.1.20420.14

Accessibility Testing

  • Narrator
  • Inspector
Microsoft Reviewers: Open in CodeFlow

@SergeySmirnov-Akvelon SergeySmirnov-Akvelon requested a review from a team as a code owner September 21, 2020 14:18
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon marked this pull request as draft September 21, 2020 14:20
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon changed the title Fixing issue with ListView in virtual mode Fixing exceptions in ListView is in virtual mode Sep 21, 2020
@RussKie RussKie added this to the 5.0 RC2 milestone Sep 21, 2020
@RussKie RussKie added Servicing-consider .NET Shiproom label indicating a PR seeks to enter into a branch under Tell-Mode criteria tenet-accessibility MAS violation, UIA issue; problems with accessibility standards 🪲 bug Product bug (most likely) labels Sep 21, 2020
@RussKie RussKie linked an issue Sep 22, 2020 that may be closed by this pull request
Copy link
Contributor

@vladimir-krestov vladimir-krestov left a comment

Choose a reason for hiding this comment

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

Viewed

@RussKie RussKie self-assigned this Sep 23, 2020
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon force-pushed the Issue-3962_ListView_is_in_virtual_mode_throws_exception branch 2 times, most recently from d1dad32 to 562404f Compare September 23, 2020 07:00
@RussKie RussKie changed the title Fixing exceptions in ListView is in virtual mode Fix ListView is in virtual mode Sep 23, 2020
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon force-pushed the Issue-3962_ListView_is_in_virtual_mode_throws_exception branch from 97b4697 to fa62c0d Compare September 23, 2020 19:28
@RussKie RussKie added Servicing-approved .NET Shiproom approved the PR for merge and removed Servicing-consider .NET Shiproom label indicating a PR seeks to enter into a branch under Tell-Mode criteria labels Sep 24, 2020
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon force-pushed the Issue-3962_ListView_is_in_virtual_mode_throws_exception branch 4 times, most recently from 3bd1c21 to 85bcea6 Compare September 25, 2020 14:59
@codecov
Copy link

codecov bot commented Sep 25, 2020

Codecov Report

Merging #3963 into release/5.0 will decrease coverage by 30.79424%.
The diff coverage is 83.07692%.

@@                  Coverage Diff                   @@
##           release/5.0       #3963          +/-   ##
======================================================
- Coverage     67.87270%   37.07846%   -30.79424%     
======================================================
  Files             1416         924         -492     
  Lines           507889      250984      -256905     
  Branches         41298       36986        -4312     
======================================================
- Hits            344718       93061      -251657     
+ Misses          156983      152461        -4522     
+ Partials          6188        5462         -726     
Flag Coverage Δ
#Debug 37.07846% <83.07692%> (-30.79424%) ⬇️
#production 37.07846% <83.07692%> (+0.15865%) ⬆️
#test ?

Flags with carried forward coverage won't be shown. Click here to find out more.

@SergeySmirnov-Akvelon SergeySmirnov-Akvelon force-pushed the Issue-3962_ListView_is_in_virtual_mode_throws_exception branch from 85bcea6 to efda3fd Compare September 25, 2020 15:13
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon force-pushed the Issue-3962_ListView_is_in_virtual_mode_throws_exception branch from efda3fd to e8db5db Compare September 25, 2020 16:06
@RussKie RussKie changed the base branch from release/5.0-rc2 to release/5.0 September 28, 2020 02:21
@RussKie RussKie removed this from the 5.0 RC2 milestone Sep 28, 2020
@RussKie RussKie added this to the 5.0 GA milestone Sep 28, 2020
@RussKie RussKie force-pushed the Issue-3962_ListView_is_in_virtual_mode_throws_exception branch from ad29a5d to 2a42c89 Compare September 28, 2020 04:08
Comment on lines 22 to 27
_owningListView = owningGroup.ListView ?? throw new InvalidOperationException(nameof(owningGroup.ListView));
_owningListView = owningGroup.ListView
?? (owningGroup.Items.Count > 0 && _owningGroup.Items[0].ListView != null
? _owningGroup.Items[0].ListView
: throw new InvalidOperationException(nameof(owningGroup.ListView)));
Copy link
Member

Choose a reason for hiding this comment

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

Suspected bug, tracked across #3987 and #4001

@@ -23,7 +23,7 @@ internal class ListViewItemAccessibleObject : AccessibleObject
public ListViewItemAccessibleObject(ListViewItem owningItem, ListViewGroup? owningGroup)
{
_owningItem = owningItem ?? throw new ArgumentNullException(nameof(owningItem));
_owningListView = owningItem.ListView ?? throw new InvalidOperationException(nameof(owningItem.ListView));
_owningListView = owningItem.ListView ?? owningGroup?.ListView ?? throw new InvalidOperationException(nameof(owningItem.ListView));
Copy link
Member

Choose a reason for hiding this comment

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

Suspected bug, tracked across #3987 and #4001

@RussKie RussKie marked this pull request as ready for review September 28, 2020 22:34
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon force-pushed the Issue-3962_ListView_is_in_virtual_mode_throws_exception branch 2 times, most recently from b858c97 to 3c65c95 Compare September 29, 2020 12:49
During implementation of accessible objects for ListView we started to use SelectedItems property for getting of required items. Unfortunately, this property is not available when the ListView is in virtual mode. As result we got an exceptions when tried to do any UI action when ListView is in virtual mode.

As a fix, we implemented using SelectedIndices instead of SelectedItems when getting Accessible Objects, because SelectedIndices property is available in any mode.

Also we added a check for methods in ListViewGroup that the ListView is not in virtual mode, since ListViewGroups are not supported for virtual mode.

Besides we replaced "groups.Count > 0" condition on "GroupsEnabled" in logic for navigation on ListView by keyboard. Together with the "groups.Count > 0" condition, it also checks that Groups is displayed and ListView is in default (not virtual) mode
…roperty

Original implementation for ExpandCollapseState is not valid because ListViewGroupCollapsedState can have Default (Non-collapsible) state. In this case user will see that group has "Collapsed" state, although the group does not support collapsing.

As result logic for ExpandCollapseState property was updated. If group has "Collapsed" state then we return "Collapsed" else we return "Expanded" for "Expanded" and "Default" states
If a ListViewItem is in a collapsed ListViewGroup and we try to select it in Inspector then we get exception. It happens because during trying to get "Bounds" property we send a "GETITEMRECT" message to NativeControl and receive empty result.

To avoid this issue, we added the condition that if a ListViewItem is located in a ListViewGroup and this ListViewGroup is collapsed we return empty rectangle otherwise we call old logic for getting of rectangle
The "SelectedIndexies.Contains" method tries to get ListViewItem using the index from parameter and checks "Selected" property for this ListViewItem. If we close the dialog that contains ListView  then ListView is still available to us in the inspector, but ListViewItem will have index with "-1" value. As result if we call "SelectedIndexies.Contains" method for ListViewItem with such index then we get an exception.

As a fix we added check that the index should be greater than -1 and less than the number of Items in the ListView. If check is failed then we will return false, otherwise we will get the ListViewItem using index and check "Selected" property
@vladimir-krestov vladimir-krestov force-pushed the Issue-3962_ListView_is_in_virtual_mode_throws_exception branch from 9945bc4 to bc28874 Compare September 29, 2020 13:28
@vladimir-krestov vladimir-krestov self-assigned this Sep 29, 2020
@RussKie RussKie merged commit d304584 into dotnet:release/5.0 Sep 30, 2020
@RussKie RussKie deleted the Issue-3962_ListView_is_in_virtual_mode_throws_exception branch September 30, 2020 02:20
vladimir-krestov pushed a commit to vladimir-krestov/winforms that referenced this pull request Oct 2, 2020
Related to PR dotnet#3963 and PR dotnet#3224
Fix keyboard Narrator announcing the check/uncheck state of ListView item when enabling ListView CheckBoxes property
Fixes Issue dotnet#4029
@ghost ghost locked as resolved and limited conversation to collaborators Jan 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🪲 bug Product bug (most likely) Servicing-approved .NET Shiproom approved the PR for merge tenet-accessibility MAS violation, UIA issue; problems with accessibility standards
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ListView is in virtual mode throws IOE
4 participants