Fix ItemsControl Automation

[walterlv]

Fixes #6861

Main PR

Description

Why does this issue happen?

Let's see the code step by step.

1. The item in a GroupItem can be detected by the Automation Software because the GroupItem provides an automation peer named GroupItemAutomationPeer and it overrides the GetChildrenCore method which returns a list of inner items. Debug the code and we can find that the itemsControlAP is null in an ItemsControl GroupItem.

// GroupItemAutomationPeer.cs
protected override List<AutomationPeer> GetChildrenCore()
{
    GroupItem owner = (GroupItem)Owner;
    ItemsControl itemsControl = ItemsControl.ItemsControlFromItemContainer(Owner);
    if (itemsControl != null)
    {
        ItemsControlAutomationPeer itemsControlAP = itemsControl.CreateAutomationPeer() as ItemsControlAutomationPeer;
        if (itemsControlAP != null)
        {
            List<AutomationPeer> children = new List<AutomationPeer>();
            // Ignore this code because in this case it will not be executed.
            return children;
        }
    }

    return null;
}

2. The ItemsControl overrides OnCreateAutomationPeerInner method but doesn't override OnCreateAutomationPeer method so the ItemsControl will execute the UIElement.OnCreateAutomationPeer method.

// UIElement.cs
protected virtual AutomationPeer OnCreateAutomationPeer()
{
    if (!AccessibilitySwitches.ItemsControlDoesNotSupportAutomation)
    {
        AutomationNotSupportedByDefaultField.SetValue(this, true);
    }
    return null;
}

This method read a switch named Switch.System.Windows.Controls.ItemsControlDoesNotSupportAutomation. As the name says, we should set the switch to false if we want the ItemsControl provides inner items automation normally. But actually, it does not work. Let me explain why it is broken.

3. Let's view the code of the method CreateAutomationPeer. If this flag is set to false, the first if-condition goes in, then the OnCreateAutomationPeer of UIElement is called which set the AutomationNotSupportedByDefaultField flag to true. So the ap is null and the AutomationNotSupportedByDefaultField.GetValue(this) is true so the OnCreateAutomationPeerInternal will not be called. As a result, we will get a null automation peer.

// UIElement.cs
// Method: CreateAutomationPeer
if (!AccessibilitySwitches.ItemsControlDoesNotSupportAutomation)
{
    // work around (ItemsControl.GroupStyle doesn't show items in groups in the UIAutomation tree)
    AutomationNotSupportedByDefaultField.ClearValue(this);
    ap = OnCreateAutomationPeer();

    // if this element returns an explicit peer (even null), use
    // it.  But if it returns null by reaching the default method
    // above, give it a second chance to create a peer.
    // [This whole dance, including the UncommonField, would be
    // unnecessary once ItemsControl implements its own override
    // of OnCreateAutomationPeer.]
    if (ap == null && !AutomationNotSupportedByDefaultField.GetValue(this))
    {
        ap = OnCreateAutomationPeerInternal();
    }
}
else
{
    ap = OnCreateAutomationPeer();
}

If the flag is set to true (which is not expected), the else-condition goes in, then the OnCreateAutomationPeer of UIElement is called which returns null. So we also get a null automation peer.

See that? the ItemsControl.OnCreateAutomationPeerInternal is never called in this if-else and anywhere else. So this bug appears.

How does this PR fix it?

We have three alternative plans to fix this issue:

1. Rewrite the UIElement.OnCreateAutomationPeer method and change the second argument of SetValue to false so that the OnCreateAutomationPeerInner will be called due to the if-condition.
2. Let the ItemsControl override OnCreateAutomationPeer and returns null or correct peer according to the switch.
3. Let the ItemsControl override OnCreateAutomationPeer and keep the code almost the same as the one in UIElement except for the true value of the second argument of the SetValue method.

For the first one. Because the AutomationNotSupportedByDefaultField.SetValue(this, true); statement is in the ItemsControl specific switch so I can guess that this statement is only written to work for ItemsControl. But it is the opposite actually. The code will execute for any other controls not only for the ItemsControl. I don't know the impact of other controls so I give up the plan.

For the second one, the code only works for the ItemsControl so I limit the impact to inside the ItemsControl. But it's not perfect because the ItemsControl.OnCreateAutomationPeerInner will never be called and should be deleted. However, it has a special bug description comment on the method which is hard to understand.

For the third one, the impact is limited to ItemsControl and the execution steps are exactly the same as the version before this PR. But I don't like this code because I think it's a bug of the ItemsControl and this code keeps the bug hidden inside.

In conclusion, I choose the second plan to fix it.

Customer Impact

If this issue is not fixed, the automation software cannot detect the items under groups of ItemsControls.

See the image, please. The first one is the original ItemsControl in the current dotnet version and the second one is the issue-fixed version. We can see the automation software cannot detect the inner item in the ItemsControl on the current dotnet version while everything works fine on the issue-fixed version.

Regression

No. It's a long-time bug.

Testing

The new code of this pull request has been tested in the repo manually: https://github.com/walterlv/Walterlv.WpfIssues.ItemsControlAutomationIssue

I write a custom ItemsControl with only automation methods overridden. I've tested the demo app and find that the bug is fixed (as the picture above says).

// The fixed version of the ItemsControl.
public class FixedItemsControl : ItemsControl
{
    protected override AutomationPeer OnCreateAutomationPeer()
    {
        return new ItemsControlWrapperAutomationPeer(this);
    }

    private sealed class ItemsControlWrapperAutomationPeer : ItemsControlAutomationPeer
    {
        public ItemsControlWrapperAutomationPeer(ItemsControl owner) : base(owner)
        {
        }

        protected override ItemAutomationPeer CreateItemAutomationPeer(object item)
        {
            return new ItemsControlItemAutomationPeer(item, this);
        }

        protected override string GetClassNameCore()
        {
            return "ItemsControl";
        }

        protected override AutomationControlType GetAutomationControlTypeCore()
        {
            return AutomationControlType.List;
        }
    }

    private class ItemsControlItemAutomationPeer : ItemAutomationPeer
    {
        public ItemsControlItemAutomationPeer(object item, ItemsControlWrapperAutomationPeer parent)
            : base(item, parent)
        { }

        protected override AutomationControlType GetAutomationControlTypeCore()
        {
            return AutomationControlType.DataItem;
        }

        protected override string GetClassNameCore()
        {
            return "ItemsControlItem";
        }
    }
}

Risk

Maybe I've written all the risks in the description section. Maybe I need some conversations to make the plans more beautiful and make less risk.

Microsoft Reviewers: Open in CodeFlow

[lindexi]

Look good to me.

[pchaurasia14]

@walterlv - Thanks for this PR!