Fixing issue with announcing of PropertyGrid expanded state #7242

dkazennov · 2022-05-31T17:55:38Z

Fixes #6656
Finishes PR #6737

Proposed changes

The problem is being reproduced due to competitive requests to Narrator. Periodically it announces expanded state information and periodically skips.
A possible fix is to replace the standard method RaiseAutomationPropertyChangedEvent with the InternalRaiseAutomationNotification (RaiseAutomationNotification) method. In this case, we force the expanded state information to be announced, which helps avoid a possible contention problem.
Unit test PropertyGridView_GridEntry_AccessibleObject_GetsNotification

Customer Impact

Before

After

Regression?

Yes (we have unstable behavior)

Risk

Minimal

Test methodology

CTI team
Manually
Unit test

Accessibility testing

Narrator
JAWS
NVDA

Test environment(s)

Microsoft Windows [Version 120.2212.4170.0]
.NET Core SDK: 7.0.100-preview.3.22179.4

Microsoft Reviewers: Open in CodeFlow

...ows.Forms/tests/UnitTests/System/Windows/Forms/PropertyGridInternal/PropertyGridViewTests.cs

dkazennov · 2022-06-07T09:33:53Z

New unit test works as expected. There are some unstable tests here though.

dmitrii-drobotov

Looks good, just a few nits

dmitrii-drobotov · 2022-06-14T10:25:12Z

...ows.Forms/tests/UnitTests/System/Windows/Forms/PropertyGridInternal/PropertyGridViewTests.cs

+ public System.Drawing.Size SizeProperty
+ {
+ get; set;
+ }


Suggested change

public System.Drawing.Size SizeProperty

{

get; set;

}

public System.Drawing.Size SizeProperty { get; set; }

dmitrii-drobotov · 2022-06-14T10:26:09Z

...ows.Forms/tests/UnitTests/System/Windows/Forms/PropertyGridInternal/PropertyGridViewTests.cs

+
+ private class TestGridEntry : GridEntry
+ {
+ readonly PropertyGridView _propertyGridView;


Suggested change

readonly PropertyGridView _propertyGridView;

private readonly PropertyGridView _propertyGridView;

dmitrii-drobotov · 2022-06-14T10:31:40Z

...ows.Forms/tests/UnitTests/System/Windows/Forms/PropertyGridInternal/PropertyGridViewTests.cs

+ {
+ readonly PropertyGridView _propertyGridView;
+
+ public TestGridEntry(PropertyGrid ownerGrid, GridEntry peParent, PropertyGridView propertyGridView)


Suggested change

public TestGridEntry(PropertyGrid ownerGrid, GridEntry peParent, PropertyGridView propertyGridView)

public TestGridEntry(PropertyGrid ownerGrid, GridEntry parent, PropertyGridView propertyGridView)

dmitrii-drobotov · 2022-06-14T10:40:39Z

...ows.Forms/tests/UnitTests/System/Windows/Forms/PropertyGridInternal/PropertyGridViewTests.cs

+ propertyGridView.DropDownControl(new Control());
+
+ Assert.Equal(1, accessibleObject.RaiseAutomationNotificationCallCount);
+ }


The test is still missing IsHandleCreated checks, as @vladimir-krestov suggested.

The test is still missing IsHandleCreated checks, as @vladimir-krestov suggested.

The test creates Handles for controls so Assert.False(IsHandleCreated) fails the test.

I think you can do Assert.True(IsHandleCreated) in that case

dmitrii-drobotov · 2022-06-14T10:43:15Z

...ows.Forms/tests/UnitTests/System/Windows/Forms/PropertyGridInternal/PropertyGridViewTests.cs

+
+ public int RaiseAutomationNotificationCallCount { get; set; }
+
+ internal override bool InternalRaiseAutomationNotification(AutomationNotificationKind notificationKind, AutomationNotificationProcessing notificationProcessing, string notificationText)


Suggested change

internal override bool InternalRaiseAutomationNotification(AutomationNotificationKind notificationKind, AutomationNotificationProcessing notificationProcessing, string notificationText)

internal override bool InternalRaiseAutomationNotification(AutomationNotificationKind notificationKind,

AutomationNotificationProcessing notificationProcessing, string notificationText)

dmitrii-drobotov · 2022-06-14T10:44:34Z

...ows.Forms/tests/UnitTests/System/Windows/Forms/PropertyGridInternal/PropertyGridViewTests.cs

+ {
+ }
+
+ public int RaiseAutomationNotificationCallCount { get; set; }


Suggested change

public int RaiseAutomationNotificationCallCount { get; set; }

public int RaiseAutomationNotificationCallCount { get; private set; }

RussKie

👍

RussKie · 2022-06-15T01:03:54Z

...ows.Forms/tests/UnitTests/System/Windows/Forms/PropertyGridInternal/PropertyGridViewTests.cs

+ public System.Drawing.Size SizeProperty
+ { get; set; }


Suggested change

public System.Drawing.Size SizeProperty

{ get; set; }

public System.Drawing.Size SizeProperty { get; set; }

vladimir-krestov · 2022-06-23T11:56:28Z

/azp run

azure-pipelines · 2022-06-23T11:56:42Z

Azure Pipelines successfully started running 2 pipeline(s).

vladimir-krestov

Looks good to me! Please send it to CTI

dkazennov · 2022-06-23T16:35:43Z

CTI Testing requested.

dkazennov · 2022-07-04T10:58:49Z

CTI test passed successfully. Let's rerun Pipeline tests.

Tanya-Solyanik · 2022-07-20T21:22:01Z

@dkazennov - looks like Vladimir can re-run tests, please ask him to get a faster turnaround. Or message in teams. I missed this request.

Tanya-Solyanik · 2022-07-20T21:22:49Z

/azp run

azure-pipelines · 2022-07-20T21:23:02Z

Azure Pipelines successfully started running 2 pipeline(s).

vladimir-krestov · 2022-07-25T08:14:07Z

/azp run

azure-pipelines · 2022-07-25T08:14:20Z

Azure Pipelines successfully started running 2 pipeline(s).

Tanya-Solyanik · 2022-07-25T18:00:58Z

/azp run

azure-pipelines · 2022-07-25T18:01:18Z

Azure Pipelines successfully started running 2 pipeline(s).

Tanya-Solyanik · 2022-07-25T18:08:20Z

...ows.Forms/tests/UnitTests/System/Windows/Forms/PropertyGridInternal/PropertyGridViewTests.cs

+ Assert.True(propertyGridView.IsAccessibilityObjectCreated);
+ Assert.Equal(testGridEntry, selectedGridEntry);
+ Assert.Equal(testGridEntry, gridEntries[selectedRow]);
+ propertyGridView.DropDownControl(new Control());


Control that you new-up is not Disposed

Tanya-Solyanik

I suspect that your test is crashing the test process. Does it pass in the VS? Does it pass from the command line? Had you tried running it individually -

dotnet test --filter <filter expression> command by switching to the desired test project directory first.

per https://github.com/dotnet/winforms/blob/main/docs/testing.md#adding-new-unit-tests

Tanya-Solyanik · 2022-07-25T18:14:49Z

...ows.Forms/tests/UnitTests/System/Windows/Forms/PropertyGridInternal/PropertyGridViewTests.cs

+
+ using Button button = new();
+ propertyGrid.SelectedObject = button;
+ GridEntryCollection gridEntries = propertyGridView.TestAccessor().Dynamic._allGridEntries;


Please look up if property grid view is re-created when selection object is set. If this is the case, please split this test into several, each dedicated to a specific selection.

dkazennov · 2022-07-28T08:27:47Z

@Tanya-Solyanik so we concluded to drop off the unit test since it is unstable and there is almost no test coverage for announcements.

…o Narrator. Periodically it announces expanded state information and periodically skips. A possible fix is to replace the standard method "RaiseAutomationPropertyChangedEvent" with the "RaiseAutomationNotification" method. In this case, we force the expanded state information to be announced, which helps avoid a possible contention problem.

dkazennov requested a review from a team as a code owner May 31, 2022 17:55

ghost assigned dkazennov May 31, 2022

dkazennov added the draft draft PR label May 31, 2022

dkazennov mentioned this pull request May 31, 2022

Fixing issue with announcing of PropertyGrid expanded state #6737

Closed

dkazennov requested a review from vladimir-krestov May 31, 2022 17:59

ghost removed the draft draft PR label May 31, 2022

dreddy-work marked this pull request as draft May 31, 2022 20:45

ghost added the draft draft PR label Jun 1, 2022

dkazennov force-pushed the Issue_6656_Announcing_PG_Expanded_State branch from 917e39e to 5b5df9f Compare June 1, 2022 10:29

vladimir-krestov reviewed Jun 1, 2022

View reviewed changes

dkazennov force-pushed the Issue_6656_Announcing_PG_Expanded_State branch 3 times, most recently from 890a171 to 1c3e2e6 Compare June 7, 2022 09:26

dkazennov added waiting-review This item is waiting on review by one or more members of team tenet-accessibility MAS violation, UIA issue; problems with accessibility standards and removed draft draft PR labels Jun 7, 2022

dkazennov requested review from Tanya-Solyanik and RussKie June 7, 2022 09:33

dkazennov force-pushed the Issue_6656_Announcing_PG_Expanded_State branch from 1c3e2e6 to 30dccba Compare June 7, 2022 10:42

ghost added the draft draft PR label Jun 7, 2022

dkazennov removed the draft draft PR label Jun 8, 2022

dkazennov marked this pull request as ready for review June 8, 2022 13:09

dmitrii-drobotov reviewed Jun 14, 2022

View reviewed changes

dkazennov force-pushed the Issue_6656_Announcing_PG_Expanded_State branch 2 times, most recently from 1d96540 to 7978cb6 Compare June 14, 2022 16:44

RussKie reviewed Jun 15, 2022

View reviewed changes

dkazennov force-pushed the Issue_6656_Announcing_PG_Expanded_State branch from 7978cb6 to e240517 Compare June 15, 2022 07:11

runfoapp bot mentioned this pull request Jun 17, 2022

Flaky System.Windows.Forms.UITests.ComboBoxTests.ComboBox_Select_Item_By_UpArrowKeyAsync #7320

Closed

vladimir-krestov added the 📬 waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s) label Jun 23, 2022

vladimir-krestov previously approved these changes Jun 23, 2022

View reviewed changes

Tanya-Solyanik reviewed Jul 25, 2022

View reviewed changes

dkazennov dismissed vladimir-krestov’s stale review via 145ea76 July 28, 2022 08:40

dkazennov force-pushed the Issue_6656_Announcing_PG_Expanded_State branch from e240517 to 145ea76 Compare July 28, 2022 08:40

Tanya-Solyanik approved these changes Aug 1, 2022

View reviewed changes

Tanya-Solyanik merged commit d40b19d into dotnet:main Aug 1, 2022

ghost added this to the 7.0 RC1 milestone Aug 1, 2022

ghost locked as resolved and limited conversation to collaborators Sep 1, 2022

RussKie removed the waiting-review This item is waiting on review by one or more members of team label Oct 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing issue with announcing of PropertyGrid expanded state #7242

Fixing issue with announcing of PropertyGrid expanded state #7242

dkazennov commented May 31, 2022 •

edited

Loading

dkazennov commented Jun 7, 2022 •

edited

Loading

dmitrii-drobotov left a comment

dmitrii-drobotov Jun 14, 2022

dmitrii-drobotov Jun 14, 2022

dmitrii-drobotov Jun 14, 2022

dmitrii-drobotov Jun 14, 2022

dkazennov Jun 14, 2022

dmitrii-drobotov Jun 14, 2022

dmitrii-drobotov Jun 14, 2022

dmitrii-drobotov Jun 14, 2022

RussKie left a comment

RussKie Jun 15, 2022

vladimir-krestov commented Jun 23, 2022

azure-pipelines bot commented Jun 23, 2022

vladimir-krestov left a comment

dkazennov commented Jun 23, 2022

dkazennov commented Jul 4, 2022

Tanya-Solyanik commented Jul 20, 2022 •

edited

Loading

Tanya-Solyanik commented Jul 20, 2022

azure-pipelines bot commented Jul 20, 2022

vladimir-krestov commented Jul 25, 2022

azure-pipelines bot commented Jul 25, 2022

Tanya-Solyanik commented Jul 25, 2022

azure-pipelines bot commented Jul 25, 2022

Tanya-Solyanik Jul 25, 2022

Tanya-Solyanik left a comment •

edited

Loading

Tanya-Solyanik Jul 25, 2022

dkazennov commented Jul 28, 2022 •

edited

Loading

	readonly PropertyGridView _propertyGridView;
	private readonly PropertyGridView _propertyGridView;

	public TestGridEntry(PropertyGrid ownerGrid, GridEntry peParent, PropertyGridView propertyGridView)
	public TestGridEntry(PropertyGrid ownerGrid, GridEntry parent, PropertyGridView propertyGridView)


		public int RaiseAutomationNotificationCallCount { get; set; }

		internal override bool InternalRaiseAutomationNotification(AutomationNotificationKind notificationKind, AutomationNotificationProcessing notificationProcessing, string notificationText)

	internal override bool InternalRaiseAutomationNotification(AutomationNotificationKind notificationKind, AutomationNotificationProcessing notificationProcessing, string notificationText)
	internal override bool InternalRaiseAutomationNotification(AutomationNotificationKind notificationKind,
	AutomationNotificationProcessing notificationProcessing, string notificationText)

	public System.Drawing.Size SizeProperty
	{ get; set; }
	public System.Drawing.Size SizeProperty { get; set; }

Fixing issue with announcing of PropertyGrid expanded state #7242

Fixing issue with announcing of PropertyGrid expanded state #7242

Conversation

dkazennov commented May 31, 2022 • edited Loading

Proposed changes

Customer Impact

Before

After

Regression?

Risk

Test methodology

Accessibility testing

Test environment(s)

Microsoft Reviewers: Open in CodeFlow

dkazennov commented Jun 7, 2022 • edited Loading

dmitrii-drobotov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RussKie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vladimir-krestov commented Jun 23, 2022

azure-pipelines bot commented Jun 23, 2022

vladimir-krestov left a comment

Choose a reason for hiding this comment

dkazennov commented Jun 23, 2022

dkazennov commented Jul 4, 2022

Tanya-Solyanik commented Jul 20, 2022 • edited Loading

Tanya-Solyanik commented Jul 20, 2022

azure-pipelines bot commented Jul 20, 2022

vladimir-krestov commented Jul 25, 2022

azure-pipelines bot commented Jul 25, 2022

Tanya-Solyanik commented Jul 25, 2022

azure-pipelines bot commented Jul 25, 2022

Choose a reason for hiding this comment

Tanya-Solyanik left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dkazennov commented Jul 28, 2022 • edited Loading

dkazennov commented May 31, 2022 •

edited

Loading

dkazennov commented Jun 7, 2022 •

edited

Loading

Tanya-Solyanik commented Jul 20, 2022 •

edited

Loading

Tanya-Solyanik left a comment •

edited

Loading

dkazennov commented Jul 28, 2022 •

edited

Loading