dotnet / winforms

Windows Forms is a .NET UI framework for building Windows desktop applications.
MIT License
4.32k stars 958 forks source link

InvalidOperationException thrown when calling OnItemChecked in class derived from ListView (in VirtualMode) #11658

Open djoyner-aveva opened 3 weeks ago

djoyner-aveva commented 3 weeks ago

.NET version

.NET 8

Did it work in .NET Framework?

Yes

Did it work in any of the earlier releases of .NET Core or .NET 5+?

No response

Issue description

We have a class derived from ListView. It calls OnItemChecked in order to raise the ItemChecked event to subscribers. In the ItemCheckedEventArgs we pass in a cloned ListViewItem so that we can set the Checked property correctly. This worked fine in .NET Framework as OnItemChecked just raised the event. However, in .NET 8, it's throwing an InvalidOperationException: "Cannot get accessiblity object when ListViewItem is not attached to a ListView." An accessibility object has been created for the ListView (which seems reasonable), however, the cloned ListViewItem is not hosted by the ListView, which I think is the cause of the exception.

Steps to reproduce

Derive a class from ListView Ensure VirtualMode and Checkboxes are enabled Ensure the accessibility object is created (not sure how to force this?) Add a method that calls OnItemChecked - something like:

ListViewItem lvi = HitTest(...); // Any code that gets a ListViewItem from the ListView
var update = (ListViewItem) lvi.Clone();
update.Checked = true;
Debug.Assert(update.Checked);
Debug.Assert(update.ListView == null);
OnItemChecked(new ItemCheckedEventArgs(update)); // throws exception
elachlan commented 3 weeks ago

@djoyner-aveva Can you please provide a stack trace?

elachlan commented 3 weeks ago

@Olina-Zhang can your team please test this?

Zheng-Li01 commented 3 weeks ago

@djoyner-aveva , created the project based on your description to reproduce the issue, no exception pops up as below screenshot, could you please check my reproduce project and correct it if there missing any steps. WinFormsApp25.zip GH11658

djoyner-aveva commented 3 weeks ago

@Zheng-Li01

Thanks for the sample. I've modified it slightly to add the control to the form, and this reproduces the error:

WinFormsApp3.zip

djoyner-aveva commented 3 weeks ago

@elachlan Call stack from sample app is: System.InvalidOperationException: Cannot get accessiblity object when ListViewItem is not attached to a ListView. at System.Windows.Forms.ListViewItem.get_AccessibilityObject() at System.Windows.Forms.ListView.OnItemChecked(ItemCheckedEventArgs e) at WinFormsApp3.ListViewTest.TryCrash() in D:\Test Projects\WinFormsApp3\WinFormsApp3\ListViewTest.cs:line 30 at WinFormsApp3.Form1.button1_Click(Object sender, EventArgs e) in D:\Test Projects\WinFormsApp3\WinFormsApp3\Form1.cs:line 12 at System.Windows.Forms.Button.OnClick(EventArgs e) at System.Windows.Forms.Button.OnMouseUp(MouseEventArgs mevent) at System.Windows.Forms.Control.WmMouseUp(Message& m, MouseButtons button, Int32 clicks) at System.Windows.Forms.Control.WndProc(Message& m) at System.Windows.Forms.ButtonBase.WndProc(Message& m) at System.Windows.Forms.NativeWindow.Callback(HWND hWnd, MessageId msg, WPARAM wparam, LPARAM lparam)

Zheng-Li01 commented 3 weeks ago

@djoyner-aveva, thanks for your project, cannot reproduce it on both .NET Framework and .NET that no exceptions pops up after clicked the button as below screenshot, could you please have a check? GHRT11658

elachlan commented 3 weeks ago

@Zheng-Li01, you need to trigger the Accessibility Object via Accessibility insights for windows (I think).

Zheng-Li01 commented 3 weeks ago

@elachlan thanks for your guidence, yes, the issue can reproduce from .NET 6.0 ~ 9.0 via Accessibility insights for windows as below screenshot. image

elachlan commented 3 weeks ago

@djoyner-aveva Just so I understand the use case clearly, why are you cloning the ListViewItem?

We can change the OnItemChecked to only trigger the AccessibilityObject change when the item exists in the Items collection, but it seems unusual to pass an item in that doesn't exist in the collection.

protected virtual void OnItemChecked(ItemCheckedEventArgs e)
{
    _onItemChecked?.Invoke(this, e);

    if (!CheckBoxes)
    {
        return;
    }

    if (IsAccessibilityObjectCreated && Items.Contains(e.Item))
    {
        ListViewItem item = e.Item;
        ToggleState oldValue = item.Checked ? ToggleState.ToggleState_Off : ToggleState.ToggleState_On;
        ToggleState newValue = item.Checked ? ToggleState.ToggleState_On : ToggleState.ToggleState_Off;
        item.AccessibilityObject.RaiseAutomationPropertyChangedEvent(UIA_PROPERTY_ID.UIA_ToggleToggleStatePropertyId, (VARIANT)(int)oldValue, (VARIANT)(int)newValue);
    }
}

Maybe even

if (IsAccessibilityObjectCreated && e.Item.ListView == this)
djoyner-aveva commented 3 weeks ago

@elachlan Thanks for your question. We're cloning it because writing to Checked on the original ListViewItem doesn't work:

item.Checked = true;
Debug.Assert(item.Checked); // fails

We have the data for the checked states stored outside of the ListView and we want to update them in response to an ItemChecked event. I've updated the sample app to try and demonstrate this (hopefully it's a bit more meaningful).

WinFormsApp3.zip

djoyner-aveva commented 3 weeks ago

@elachlan And I like the look of your second suggestion - that was the fix I had in mind

if (IsAccessibilityObjectCreated && e.Item.ListView == this)

lonitra commented 1 week ago

@SimonZhao888 Could you confirm that this does not repro in Framework?

Tanya-Solyanik commented 1 week ago

@SimonZhao888 - please add a manual test case that clones a ListView item to the WinformsControlsApp

SimonZhao888 commented 1 week ago

@SimonZhao888 Could you confirm that this does not repro in Framework?

@lonitra , this does not repro in Framework.