The issue is reproduced because we have cached the ListViewItem.Acces…

…sibilityObject. After the changes in #4910, we added logic that, with each call to the "AccessibilityObject" property, checked that the ListView exists and, if absent, threw an exception. (#6352) We now return null as a ListViewItem.AccessibilityObject instead of returning an exception. Also, after the focused ListViewItem is removed, it remains cached in the FocusedItem property. It also forces the Accessibility tool to try to retrieve the data of the ListViewItem that is not displayed. As a fix, a logic was added to reset the Focused flag from the ListViewItem.
dotnet · Feb 3, 2022 · 2a7cc86 · 2a7cc86
1 parent ff8a062
commit 2a7cc86
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 37 deletions.
diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/ListView.ListViewNativeItemCollection.cs b/src/System.Windows.Forms/src/System/Windows/Forms/ListView.ListViewNativeItemCollection.cs
@@ -427,6 +427,8 @@ public void RemoveAt(int index)
 
                 owner.ApplyUpdateCachedItems();
                 int itemID = DisplayIndexToID(index);
+
+                this[index].Focused = false;
                 this[index].UnHost(true);
 
                 if (owner.IsHandleCreated)

diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/ListViewGroup.cs b/src/System.Windows.Forms/src/System/Windows/Forms/ListViewGroup.cs
@@ -78,17 +78,7 @@ public ListViewGroup(string? header, HorizontalAlignment headerAlignment) : this
         }
 
         internal AccessibleObject AccessibilityObject
-        {
-            get
-            {
-                if (_accessibilityObject is null)
-                {
-                    _accessibilityObject = new ListViewGroupAccessibleObject(this, ListView?.Groups.Contains(this) == false);
-                }
-
-                return _accessibilityObject;
-            }
-        }
+            => _accessibilityObject ??= new ListViewGroupAccessibleObject(this, ListView?.Groups.Contains(this) == false);
 
         /// <summary>
         ///  The text displayed in the group header.

diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/ListViewItem.cs b/src/System.Windows.Forms/src/System/Windows/Forms/ListViewItem.cs
@@ -227,7 +227,13 @@ internal virtual AccessibleObject AccessibilityObject
         {
             get
             {
-                ListView owningListView = listView ?? Group.ListView ?? throw new InvalidOperationException(nameof(listView));
+                ListView owningListView = listView ?? Group?.ListView;
+                if (owningListView is null)
+                {
+                    _accessibilityObject = null;
+                    return _accessibilityObject;
+                }
+
                 if (_accessibilityObject is null || owningListView.View != _accessibilityObjectView)
                 {
                     _accessibilityObjectView = owningListView.View;

diff --git a/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/ListViewItemTests.cs b/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/ListViewItemTests.cs
@@ -852,15 +852,16 @@ public static IEnumerable<object[]> BackColor_Set_TestData()
             yield return new object[] { Color.Red, Color.Red };
         }
 
-        [Theory]
+        [WinFormsTheory]
         [MemberData(nameof(BackColor_Set_TestData))]
         public void ListViewItem_BackColor_GetWithOwner_ReturnsExpected(Color value, Color expected)
         {
-            var listView = new ListView
+            using ListView listView = new()
             {
                 BackColor = value
             };
-            var item = new ListViewItem();
+
+            ListViewItem item = new();
             listView.Items.Add(item);
             Assert.Equal(expected, item.BackColor);
 
@@ -884,12 +885,12 @@ public void ListViewItem_BackColor_Set_GetReturnsExpected(Color value, Color exp
             Assert.Equal(expected, item.BackColor);
         }
 
-        [Theory]
+        [WinFormsTheory]
         [MemberData(nameof(BackColor_Set_TestData))]
         public void ListViewItem_BackColor_SetWithOwner_GetReturnsExpected(Color value, Color expected)
         {
-            var listView = new ListView();
-            var item = new ListViewItem();
+            using ListView listView = new();
+            ListViewItem item = new();
             listView.Items.Add(item);
 
             item.BackColor = value;
@@ -908,15 +909,16 @@ public static IEnumerable<object[]> ForeColor_Set_TestData()
             yield return new object[] { Color.Red, Color.Red };
         }
 
-        [Theory]
+        [WinFormsTheory]
         [MemberData(nameof(ForeColor_Set_TestData))]
         public void ListViewItem_ForeColor_GetWithOwner_ReturnsExpected(Color value, Color expected)
         {
-            var listView = new ListView
+            using ListView listView = new()
             {
                 ForeColor = value
             };
-            var item = new ListViewItem();
+
+            ListViewItem item = new();
             listView.Items.Add(item);
             Assert.Equal(expected, item.ForeColor);
 
@@ -929,23 +931,24 @@ public void ListViewItem_ForeColor_GetWithOwner_ReturnsExpected(Color value, Col
         [MemberData(nameof(ForeColor_Set_TestData))]
         public void ListViewItem_ForeColor_Set_GetReturnsExpected(Color value, Color expected)
         {
-            var item = new ListViewItem
+            ListViewItem item = new()
             {
                 ForeColor = value
             };
+
             Assert.Equal(expected, item.ForeColor);
 
             // Set same.
             item.ForeColor = value;
             Assert.Equal(expected, item.ForeColor);
         }
 
-        [Theory]
+        [WinFormsTheory]
         [MemberData(nameof(ForeColor_Set_TestData))]
         public void ListViewItem_ForeColor_SetWithOwner_GetReturnsExpected(Color value, Color expected)
         {
-            var listView = new ListView();
-            var item = new ListViewItem();
+            using ListView listView = new();
+            ListViewItem item = new();
             listView.Items.Add(item);
 
             item.ForeColor = value;
@@ -956,15 +959,16 @@ public void ListViewItem_ForeColor_SetWithOwner_GetReturnsExpected(Color value,
             Assert.Equal(expected, item.ForeColor);
         }
 
-        [Theory]
+        [WinFormsTheory]
         [CommonMemberData(typeof(CommonTestHelperEx), nameof(CommonTestHelperEx.GetFontTheoryData))]
         public void ListViewItem_Font_GetWithOwner_ReturnsExpected(Font value)
         {
-            var listView = new ListView
+            using ListView listView = new()
             {
                 Font = value
             };
-            var item = new ListViewItem();
+
+            ListViewItem item = new();
             listView.Items.Add(item);
             Assert.Equal(value ?? Control.DefaultFont, item.Font);
 
@@ -981,22 +985,24 @@ public void ListViewItem_Font_Set_GetReturnsExpected(Font value)
             {
                 Font = value
             };
+
             Assert.Equal(value ?? Control.DefaultFont, item.Font);
 
             // Set same.
             item.Font = value;
             Assert.Equal(value ?? Control.DefaultFont, item.Font);
         }
 
-        [Theory]
+        [WinFormsTheory]
         [CommonMemberData(typeof(CommonTestHelperEx), nameof(CommonTestHelperEx.GetFontTheoryData))]
         public void ListViewItem_Font_SetWithOwner_GetReturnsExpected(Font value)
         {
-            var listView = new ListView
+            using ListView listView = new()
             {
                 Font = SystemFonts.CaptionFont
             };
-            var item = new ListViewItem();
+
+            ListViewItem item = new();
             listView.Items.Add(item);
 
             item.Font = value;
@@ -1007,11 +1013,11 @@ public void ListViewItem_Font_SetWithOwner_GetReturnsExpected(Font value)
             Assert.Equal(value ?? SystemFonts.CaptionFont, item.Font);
         }
 
-        [Fact]
+        [WinFormsFact]
         public void ListViewItem_EnsureVisible_HasListViewWithoutHandle_Nop()
         {
-            var listView = new ListView();
-            var item = new ListViewItem();
+            using ListView listView = new();
+            ListViewItem item = new();
             listView.Items.Add(item);
             item.EnsureVisible();
         }
@@ -1023,11 +1029,11 @@ public void ListViewItem_EnsureVisible_NoListView_Nop()
             item.EnsureVisible();
         }
 
-        [Fact]
+        [WinFormsFact]
         public void ListViewItem_Remove_HasListView_Success()
         {
-            var listView = new ListView();
-            var item = new ListViewItem();
+            using ListView listView = new();
+            ListViewItem item = new();
             listView.Items.Add(item);
             item.Remove();
             Assert.Empty(listView.Items);
@@ -1063,5 +1069,23 @@ private static void AssertEqualListViewSubItem(ListViewItem.ListViewSubItem[] ex
                 Assert.Equal(expected[i].Tag, actual[i].Tag);
             }
         }
+
+        [Fact]
+        public void ListViewItem_AccessibilityObject_ReturnsNull_IfListViewNotExists()
+        {
+            ListViewItem item = new();
+
+            Assert.Null(item.AccessibilityObject);
+        }
+
+        [WinFormsFact]
+        public void ListViewItem_AccessibilityObject_ReturnsExpected_IfListViewExists()
+        {
+            using ListView listView = new();
+            ListViewItem item = new();
+            listView.Items.Add(item);
+
+            Assert.NotNull(item.AccessibilityObject);
+        }
     }
 }