Do not count invisible UI elements towards size of/position in set

[mslukebo]

Fixes #8642

Description

PositionInSet and SizeOfSet for items currently count all items in their container, resulting in potentially incorrect information. This PR updates the logic to exclude non-visible UI elements from these counts.

Customer Impact

Screen readers may report incorrect information with the current implementation.

Testing

I tested this patched PresentationFramework.dll against the repro mentioned in #8642 and confirmed Windows Narrator reads the expected values.

Risk

These changes only effect non-grouping paths. I am not familiar with how the grouped collections calculate set size/position, so those values may still be inaccurate.

Microsoft Reviewers: Open in CodeFlow

[mslukebo]

I did a quick search in the PresentationFramework library to see if anyone else calls these methods, and found a special case for MenuItems. With the changes from #1977, my original changes would cause menu items with invisible separators to under-report size of/position in menu item sets. I updated the MenuItem automation peer code to only subtract visible separators from the count. Please let me know if there's a better way to deal with this case.

[rchauhan18]

@mslukebo Thanks for adding a fix to resolve the issue. The current solution will work for the sample mentioned in the issue, where the ListItem are directly added.

However, in case when we bind the data to the List and use IsVisible property of an object to make the ListItem visible. The narrator is still calling the wrong sequence and total count. This is because in this case the child is an object of a class that is bound to ListItem and will always return false for this condition.

if (child is UIElement { Visibility: not Visibility.Visible })
                    {
                        position -= 1;
                    }

Here is the sample code:

 <ListView Name="myListView" ItemsSource="{Binding YourCollection}">
    <ListView.View>
        <GridView>
            <GridViewColumn Header="Name" DisplayMemberBinding="{Binding Name}" />
        </GridView>
    </ListView.View>
    <ListView.ItemContainerStyle>
        <Style TargetType="ListViewItem">
            <Style.Triggers>
                <DataTrigger Binding="{Binding IsVisible}" Value="False">
                    <Setter Property="Visibility" Value="Collapsed" />
                </DataTrigger>
            </Style.Triggers>
        </Style>
    </ListView.ItemContainerStyle>
</ListView>

[mslukebo]

@mslukebo Thanks for adding a fix to resolve the issue. The current solution will work for the sample mentioned in the issue, where the ListItem are directly added.

However, in case when we bind the data to the List and use IsVisible property of an object to make the ListItem visible. The narrator is still calling the wrong sequence and total count. This is because in this case the child is an object of a class that is bound to ListItem and will always return false for this condition.

if (child is UIElement { Visibility: not Visibility.Visible })
                    {
                        position -= 1;
                    }

Here is the sample code:

 <ListView Name="myListView" ItemsSource="{Binding YourCollection}">
    <ListView.View>
        <GridView>
            <GridViewColumn Header="Name" DisplayMemberBinding="{Binding Name}" />
        </GridView>
    </ListView.View>
    <ListView.ItemContainerStyle>
        <Style TargetType="ListViewItem">
            <Style.Triggers>
                <DataTrigger Binding="{Binding IsVisible}" Value="False">
                    <Setter Property="Visibility" Value="Collapsed" />
                </DataTrigger>
            </Style.Triggers>
        </Style>
    </ListView.ItemContainerStyle>
</ListView>

I see. What is the proper way to handle this case then? Is there a way to get the item container for a child?

[mslukebo]

@rchauhan18 I did some testing and can address the case you describe by using code similar to the following:

// Some items may not be visible, so we don't want to count them
foreach (var child in itemCollection.CollectionView)
{
    if (child is UIElement { Visibility: not Visibility.Visible })
    {
        size -= 1;
        continue;
    }

    var container = itemsControl.ItemContainerGenerator.ContainerFromItem(child);
    if (container == null || container is UIElement { Visibility: not Visibility.Visible })
    {
        size -= 1;
        continue;
    }
}

This fixes the "x of y" that narrator says, but doesn't address the grid/table size.

My test case looks like

public partial class MainWindow : Window
{
    public List<MyObject> MyObjects { get; }

    public MainWindow()
    {
        InitializeComponent();
        this.MyObjects = new List<MyObject>()
        {
            new MyObject("A", true),
            new MyObject("B", false),
            new MyObject("C", true),
        };

        this.DataContext = this;
    }

    public record MyObject(string Name, bool IsVisible);
}

<ListView Width="500" ItemsSource="{Binding MyObjects}">
    <ListView.View>
        <GridView>
            <GridViewColumn Header="Name" DisplayMemberBinding="{Binding Name}" />
        </GridView>
    </ListView.View>
    <ListView.ItemContainerStyle>
        <Style TargetType="ListViewItem">
            <Style.Triggers>
                <DataTrigger Binding="{Binding IsVisible}" Value="False">
                    <Setter Property="Visibility" Value="Collapsed" />
                </DataTrigger>
            </Style.Triggers>
        </Style>
    </ListView.ItemContainerStyle>
</ListView>

When tabbing into the listview, it'll say "Enter table 3 by 1 (...) selected 1 of 2". This is likely because GridViewAutomationPeer's ITableProvider implementation doesn't take into account invisible rows when calculating IGridProvider.RowCount. There are likely other implementations of this interface that would need to take invisible items into account too, such as TableAutomationPeer and DataGridAutomationPeer (really anything that implements IGridProvider).

Options therefore are

1. Ignore the case mentioned above for this PR
2. Update my PR with the new itemsControl.ItemContainerGenerator.ContainerFromItem check to address the sizeof/positionin set issue. Ignore the IGridProvider.RowCount issue and solve with a separate PR
3. Try to address the IGridProvider.RowCount issue in this PR as well

What would you like me to do?

[rchauhan18]

@mslukebo I am adding a sample application which contain 4 cases. https://github.com/rchauhan18/A11y-TestApplication/tree/master My suggestion is that the fix should work for all these cases.

Case 1: ListItem directly added

Your current code is passing for this case.

Case 2: ListItem with data binding

Your approach of obtaining the item container from the object and checking its visibility should work in this case. Regarding "Enter table" count, I think so we can add the similar code to GridProvider.RowCount Here is a suggestion:

int IGridProvider.RowCount
{
    get
    {
-       return _listview.Items.Count;
+       int rowcount = _listview.Items.Count;
+        foreach(var item in _listview.Items)
+       {
+            if (item is UIElement { Visibility: not Visibility.Visible })
+           {
+               rowcount -= 1;
+                continue;
+           }
+
+            var container = _listview.ItemContainerGenerator.ContainerFromItem(item);
+           if (container == null || container is UIElement { Visibility: not Visibility.Visible })
+            {
+               rowcount -= 1;
+                continue;
+            }
+        }
+       return rowcount;
    }
}

Case 3: Using ListBox items

Similar as ListView. It is working properly if we add items directly to the ListBox, but failing when using data binding without the update.

Case 4: Grouping ListItem

In this case the narrator is failing to announce the correct order. This is because in case of grouping, the position and size are returned by FindPositionInGroup. Suggestion: We can solve this similarly as above case. We can remove the count of non-visible items.

In WPF when there is a grouping, narrator announces the name and position of group, then announces the position item and size of the group. Example: " [Name of the group] group, [Group Position] of [Total number of groups], Enter Table [Total size of table] ItemName [position in group] and [size of group] "

[mslukebo]

@rchauhan18 thanks for the suggestions. My question is if you want this PR to address 1, Only ListItems/ListBoxItems added directly (i.e. the current state of my PR) 2. ListItems/ListBoxItems added both directly or with data binding (i.e. update with my suggestion in my previous comment) 3. All of the above, plus grouping 4. All of the above, plus fixing GridProvider.RowCount implementations

I'm not too familiar with this codebase, so I'd prefer if my PR only addressed 1 and/or 2 and left the rest for people more familiar with the solutions. If you want, I can take a stab fixing all the issues in this PR though.

[Kuldeep-MS]

Hi @mslukebo, thank you for your contribution. While we appreciate your suggestion, we prefer to merge pull requests that include a full fix unless the issue is urgent or particularly difficult to resolve. That being said, you don't need to fix all four cases mentioned above - we'll take care of it from here.

@rchauhan18, could you please update the pull request to include fixes for those four cases?

[rchauhan18]

@mslukebo I am adding a sample application which contain 4 cases. https://github.com/rchauhan18/A11y-TestApplication/tree/master My suggestion is that the fix should work for all these cases.

Case 1: ListItem directly added

Your current code is passing for this case.

Case 2: ListItem with data binding

Your approach of obtaining the item container from the object and checking its visibility should work in this case. Regarding "Enter table" count, I think so we can add the similar code to GridProvider.RowCount Here is a suggestion:

int IGridProvider.RowCount
{
    get
    {
-       return _listview.Items.Count;
+       int rowcount = _listview.Items.Count;
+        foreach(var item in _listview.Items)
+       {
+            if (item is UIElement { Visibility: not Visibility.Visible })
+           {
+               rowcount -= 1;
+                continue;
+           }
+
+            var container = _listview.ItemContainerGenerator.ContainerFromItem(item);
+           if (container == null || container is UIElement { Visibility: not Visibility.Visible })
+            {
+               rowcount -= 1;
+                continue;
+            }
+        }
+       return rowcount;
    }
}

Case 3: Using ListBox items

Similar as ListView. It is working properly if we add items directly to the ListBox, but failing when using data binding without the update.

Case 4: Grouping ListItem

In this case the narrator is failing to announce the correct order. This is because in case of grouping, the position and size are returned by FindPositionInGroup. Suggestion: We can solve this similarly as above case. We can remove the count of non-visible items.

In WPF when there is a grouping, narrator announces the name and position of group, then announces the position item and size of the group. Example: " [Name of the group] group, [Group Position] of [Total number of groups], Enter Table [Total size of table] ItemName [position in group] and [size of group] "

@mslukebo Will it be okay with you if we add the suggested fix on your branch?

[mslukebo]

@mslukebo Will it be okay with you if we add the suggested fix on your branch?

Yes, please feel free to work on top of my changes.

[miloush]

Let's make it clear this is always going to be a best effort. Developer can trivially "circumvent" the counting, make the item empty, non-selectable etc.

However, I am not quite sure the original issue is valid. The item is in the collection, whether it's visible or not does not change that fact.

[miloush]

See the documentation:

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.

[rchauhan18]

Let's make it clear this is always going to be a best effort. Developer can trivially "circumvent" the counting, make the item empty, non-selectable etc.

However, I am not quite sure the original issue is valid. The item is in the collection, whether it's visible or not does not change that fact.

@miloush I agree with you that we cannot use the visibility property to update the RowCount.

According to documentation, RowCount should be the count of all the element loaded in the logical tree. This is as per the design. Thanks for sharing the document.

[rchauhan18]

@mslukebo On further investigation, I found that the current fix won't work in WPF as it will return incorrect count in few cases (one is mentioned by @miloush) In case of Virtualization the current fix will fail as only few items will be loaded as a UI Element and other which are not loaded will be considered as not visible despite their property set to visible and the count will be incorrect.

I have a workaround, to update the PositionInSet and SizeOfSet in the application itself after loading.

private void UpdatePositionInSet(object sender, RoutedEventArgs e)
{
    int position = 1;
    int size = 0;
    foreach (var item in MyItemCollection)
    {
        if (item.IsVisible)
        {
            var container = myListView.ItemContainerGenerator.ContainerFromItem(item);
            AutomationProperties.SetPositionInSet(myListView.ItemContainerGenerator.ContainerFromItem(item) as UIElement, position++);
            size++;
        }
    }
    foreach (var item in MyItemCollection)
    {
        AutomationProperties.SetSizeOfSet(myListView.ItemContainerGenerator.ContainerFromItem(item) as UIElement, size);
    }
        
}

Sample application: https://github.com/rchauhan18/ListViewSample Can you confirm, will the workaround work for you?