Fix ItemsControl Automation Again

[walterlv]

Fixes #8679

Main PR

Description

The PR #6862 addressed an issue with ItemsControl Automation reported in #6861. However, it inadvertently introduced breaking changes, as reported in #8679. Another PR, #8712, attempts to resolve this issue by partially reverting the changes made in #6862, but this approach does not fundamentally address the problem.

Why does #8679 occur?

Actually, the button "ItemButton1" does not disappear. Instead, its type is changed from Button to DataItem by the ItemsControlWrapperAutomationPeer.

How does this PR resolve #8679?

I believe the proper way to fix #8679 is to maintain the original type of the UIElement, rather than reverting the changes of #6862. This is because the ItemsControlWrapperAutomationPeer is designed to present DataItems from ItemsControl to the UIA tree when the ItemsControlDoesNotSupportAutomation switch is off. However, the approach in #8712, which returns null, results in ItemsControl inconsistent behavior: sometimes reports DataItems (as in groups) and other times reports null (as in different scenarios). This inconsistency does not align with the intended function of the ItemsControlDoesNotSupportAutomation switch.

To fix this, I have introduced a new type, ItemsControlElementAutomationPeer, as a counterpart to the existing ItemsControlItemAutomationPeer. This preserves the original automation functionality of UIElement. As a result, we can resolve both issues #6862 and #8679, while also adhering to the intended purpose of the ItemsControlDoesNotSupportAutomation switch.

Customer Impact

• Comparing to .NET 7 and earlier versions, this change introduces DataItem to the UIA tree for plain lists in ItemsControl, and adds UIA support for grouped items.
• Comparing to .NET 8, this change alters the UIA type of UIElement within ItemsControl when the UIElement has its own AutomationPeer. This ensures that button-like controls in ItemsControl behave as they did in .NET 7 and earlier versions.

Regression

Testing

I have expanded the existing test code and incorporated the contributions from @Tobiidave in #8679.

https://github.com/walterlv/Walterlv.WpfIssues.ItemsControlAutomationIssue

The revised code has been tested and functions correctly.

Risk

This modification introduces breaking changes to the ItemsControl AutomationPeer. We should discuss this change thoroughly before proceeding with the merge.

Microsoft Reviewers: Open in CodeFlow

[lindexi]

LGTM

[jimm98y]

I don't think this fixes the issue reported in #8679. I've just tried the changes in my updated test app and it gives me this result:

- TopButton : ControlType.Button
-- TopButton : ControlType.Text
-  : ControlType.List
-- ItemButton1 : ControlType.Button
--- ItemButton1 : ControlType.Text

We can compare to the original NET7/NET6 behavior:

- TopButton : ControlType.Button
-- TopButton : ControlType.Text
- ItemButton1 : ControlType.Button
-- ItemButton1 : ControlType.Text

You can notice the automation tree is different and there is an additional "List" element and the ItemButton1 is on a different level in the tree. Therefore it looks to me that this new PR just changes the current behavior from "broken with a possible workaround" to just "broken".

Please consider people with thousands of UI tests coded against the NET7/NET6 automation tree having to redo them all because of this breaking change in NET8. This is currently causing us huge issues with NET8 migration and I believe we are not the only ones affected based upon the #8679 comments.

PR #8712 tried to limit the impact of the PR #6862 to just "Groups" and otherwise, it fully restored the original pre-NET8 behavior, which this PR does not. Furthermore, this PR prevents us from using the _ItemsControlDoesNotSupportAutomation switch to restore the original NET7/NET6 behavior.

[lindexi]

@jimm98y So, are you suggesting that we should maintain the same behavior as the previous version, even if it was characterized by a bug? Thank you.

[jimm98y]

The so called "previous behavior" has been there since NET Framework, which makes me wonder if it can still be called a "bug". Isn't it a "feature"? Looking at the code, I fully agree with @walterlv and I actually like the new consistent behavior in this pull request.

Unfortunately, looking at our collection of test cases that are currently all broken as a result of this PR, I must also weight in the amount of work required to rework everything on our side and that effort would be significant. I believe the same is true not just for us, but I can imagine any larger project based upon WPF would be affected, making NET8 migration even more difficult. From this standpoint yes, even if it means having another switch that restores the original behavior, I think maintaining the same behavior as NET6/NET7 makes sense.

It is true #8712 introduced an inconsistent behavior, preferring the backwards compatibility over behavior consistency. However, it was an intentional behavior based upon the current situation and options we have, which I'd like to elaborate a little bit:

Rolling back the entire #6862 would fix our issues, re-introducing the original problem. Since the original problem dates back to NET Framework days, I think an important consideration here is how many customers are actually affected by this. In my opinion a fix for #6862 affects only new projects that have started on NET8, while issue #8679 is affecting all projects that started long time ago on NET Framework, consist of large code bases and are currently being migrated over to NET8. Anyway, this fixes scenarios from #8679 and breaks all scenarios from #6862, which begs a question: can we do better?

The way I understood #6862 was that it was primarily about grouped items. Was my understanding not correct? I believe the primary issue could be simply characterized by "content of groups in ItemsControl is invisible to automation". What is the impact of fixing this issue? How does it change the automation tree? I am no automation expert so I might be wrong, but I came to the conclusion that we can add the missing items into the tree (they were all leaves, weren't they?) while not affecting any existing tree hierarchy, which means all the test cases should still work like before NET8, but also the new cases addressed by #6862 will continue working.

So that's exactly what #8712 tried to do: restore the original behavior while trying to not break #6862. Is it ideal? No.

Other options?
Introduce a feature switch that when set, fully restores the original NET Framework/NET6/NET7 behavior. My main concern with such switch would be how to ensure it will not break in the next NET9 release... Another concern would be how to actually set it, because when I tried to set "Switch.System.Windows.Controls.ItemsControlDoesNotSupportAutomation" in OnStartup, non-zero value was already cached in System.Windows.AccessibilitySwitches and I had to use reflection on private fields to actually set it.

[walterlv]

I strongly agree with @jimm98y 's viewpoint. It's essential to consider the behaviors from previous .NET Framework / .NET 6 / .NET 7. At the same time, I'm inclined to do the right thing by maintaining consistency in automated behavior.

Therefore, personally, I prefer to continue fixing the switch in this PR: revert entirely to the previous behavior when ItemsControlDoesNotSupportAutomation is set to true; when it's not set or set to false, adopt a consistent behavior.

[walterlv]

I've tested this code below to set the AppContext switch and find it worked.

public class Program
{
    [STAThread]
    static void Main()
    {
        AppContext.SetSwitch("Switch.System.Windows.Controls.ItemsControlDoesNotSupportAutomation", true);

        var app = new App();
        app.InitializeComponent();
        app.Run();
    }
}

This means that if it is late on startup, it can be set before the WPF initialization which reflection is not needed.

[rchauhan18]

I've tested this code below to set the AppContext switch and find it worked.

public class Program
{
    [STAThread]
    static void Main()
    {
        AppContext.SetSwitch("Switch.System.Windows.Controls.ItemsControlDoesNotSupportAutomation", true);

        var app = new App();
        app.InitializeComponent();
        app.Run();
    }
}

This means that if it is late on startup, it can be set before the WPF initialization which reflection is not needed.

I have tried this workaround on a sample application and the tree view order is same as previous version.
.NET 7:

.NET 8 (without workaround)

.NET 8 (with switch setting ItemsControlDoesNotSupportAutomation as True)

@jimm98y Could you confirm if this workaround is effective for you?
In the meantime, we will investigate this issue further and explore alternative solutions.

[jimm98y]

It's working, thank you very much!

[majomix]

Hello, I'm joining this thread to mention this is a critical issue for our test automation. We're awaiting the update.