dotnet / wpf

WPF is a .NET Core UI framework for building Windows desktop applications.
MIT License
7.03k stars 1.16k forks source link

PositionInSet and SizeOfSet do not take into account hidden/collapsed elemnts #8642

Open mslukebo opened 8 months ago

mslukebo commented 8 months ago

Description

Elements that present a collection of items (such as a Menu with MenuItems or a ListView with ListViewItems) do not take into account non-visible elements when calculating PositionInSet and SizeOfSet. This results in screen readers announcing incorrect information.

Reproduction Steps

Create a WPF project that contains one such element. For example, consider the following XAML:

<Window x:Class="BlankWpf.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
        xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
        xmlns:local="clr-namespace:BlankWpf"
        mc:Ignorable="d"
        Title="MainWindow" Height="450" Width="800">
    <Grid>
        <ListView Width="500">
            <ListViewItem Content="A" />
            <ListViewItem Content="B" Visibility="Collapsed" />
            <ListViewItem Content="C" />
        </ListView>
    </Grid>
</Window>

Compile this project, open Windows Narrator, and select one of the two visible items in the list.

Expected behavior

When selecting the first item, Windows Narrator should announce "1 of 2." When selecting the second item (C), Windows Narrator should announce "2 of 2."

Actual behavior

When selecting the first item, Windows Narrator announces "1 of 3." When selecting the second item (C), Windows Narrator announces "3 of 3."

Regression?

No response

Known Workarounds

No response

Impact

This is an accessibility issue, since screen readers announce incorrect information.

Configuration

No response

Other information

No response

miloush commented 2 months ago

For the record, documented behavior:

Hidden rows and columns, depending on the provider implementation, may be loaded in the logical tree and will therefore be reflected in the IGridProvider::RowCount and IGridProvider::ColumnCount properties. If the hidden rows and columns have not yet been loaded they will not be counted.

This suggests that if it's loaded in the logical tree, it should be reflected in the count. This could be a breaking change and unless we have a good reason (such as an accessibility standard saying otherwise), we shouldn't change this.

If this is fixed to the "expected behavior", then you can have reported count less than the number of items you can retrieve using GetItem, the item you get might not be the one you think and that would be much worse.

mslukebo commented 2 months ago

For the record, documented behavior:

Hidden rows and columns, depending on the provider implementation, may be loaded in the logical tree and will therefore be reflected in the IGridProvider::RowCount and IGridProvider::ColumnCount properties. If the hidden rows and columns have not yet been loaded they will not be counted.

This suggests that if it's loaded in the logical tree, it should be reflected in the count. This could be a breaking change and unless we have a good reason (such as an accessibility standard saying otherwise), we shouldn't change this.

If this is fixed to the "expected behavior", then you can have reported count less than the number of items you can retrieve using GetItem, the item you get might not be the one you think and that would be much worse.

This is a fair point. The root bug report is that my WPF app is being flagged with an accessibility bug caused by the "actual behavior" in this bug report.

The part of the docs that say "Hidden rows and columns, depending on the provider implementation, may be loaded in the logical tree" (emphasis mine) is interesting. Maybe the correct way to implement this is to simply not load these collapsed items into the logical tree then?

miloush commented 2 months ago

So far I would claim the bug is on whoever thinks that the count should correspond to visible/selectable/otherwise-interesting items. It is also unclear to me how this is expected to work with virtualization.

Yes if you don't add the items to the collection they wouldn't be counted. You could possibly use collection view with filtering for that.

my WPF app is being flagged with an accessibility bug

By whom? Is there any justification or reference on why it should behave differently?

mslukebo commented 2 months ago

My organization has accessibility reviewers that find accessibility problems in our applications. These experts have identified the "actual behavior" as a bug.

The claim is that users who rely on screen reader information will be confused since the position/size of set do not accurately represent the controls they can navigate between. It is confusing to use arrow keys while navigating a menu and jump from "1 and 3" to "3 of 3." Where did the second element go?

I could see an argument that this is a bug with Narrator. Perhaps Narrator should be smart enough to understand that the second element isn't visible, so it needs to subtract 1 from the position/count. This might be asking too much, though.

WPF already has special cases for things like this. There is already code that ignores Separators from the count.

I don't think developers should have to create custom collection view filters just to ensure correct accessiblity information is announced. The problem should be fixed either in WPF or Narrator.

I haven't done it, but it might be worth investigating what Narrator announces for a similar setup in other frameworks (Win32, UWP, MAUI, etc).

miloush commented 2 months ago

Well Narrator is reporting the numbers, it is not making the inference that they are all selectable or visible. The accessibility reviewer does.

Any maybe they are right, but if we are going to make a breaking change I would like to see a standard that demands such interpretation, and GetItem should certainly be adjusted too.

I don't think developers should have to create custom collection view filters just to ensure correct accessiblity information is announced.

I agree with that. It all boils down to what is "correct".

it might be worth investigating what Narrator announces for a similar setup in other frameworks (Win32, UWP, MAUI, etc).

That could certainly provide some argument to align.

I don't see any code in the GridViewAutomationPeer to deal with Separators (though you can certainly put a Separator there). The only peer who does that is MenuItemAutomationPeer and from what I can see the GetItem is possibly broken for that.