Adding UIA support for TrackBar by SergeySmirnov-Akvelon · Pull Request #4939 · dotnet/winforms

SergeySmirnov-Akvelon · 2021-05-19T12:12:21Z

Proposed changes

Updated SupportsUiaProviders flag.
Added and implemented TrackBarAccessibleObject, TrackBarFirstButtonAccessibleObject,
TrackBarLastButtonAccessibleObject and TrackBarThumbAccessibleObject classes for working with the TrackBar and its elements.
Added calls to RaiseAutomationPropertyChangedEvent and RaiseAutomationEvent methods for the correct announcing of the Narrator

Customer Impact

Before fix:

ProviderDescription: "[pid:9884,providerId:0x280FE0 Main:Nested [pid:28360,providerId:0x280FE0 Main(parent link):Microsoft: MSAA Proxy (unmanaged:UIAutomationCore.dll)]; Nonclient:Microsoft: Non-Client Proxy (unmanaged:uiautomationcore.dll); Hwnd(parent link):Microsoft: HWND Proxy (unmanaged:uiautomationcore.dll)]"

After fix:

ProviderDescription: "[pid:25616,providerId:0x90BDA Main:Nested [pid:25228,providerId:0x90BDA Main(parent link):Unidentified Provider (unmanaged:coreclr.dll)]; Hwnd(parent link):Microsoft: HWND Proxy (unmanaged:uiautomationcore.dll)]"

Regression?

No

Risk

Minimcal

Test methodology

Unit tests
CTI team

Accessibility testing

Inspect
Narrator
Accessibility Insights

Test environment(s)

Microsoft Windows [Version 10.0.19041.388]
.NET Core SDK: 6.0.100-preview.3.21202.5

Microsoft Reviewers: Open in CodeFlow

vladimir-krestov

Nice! Good work!
Main points:

Check Handle using
Naming and indentation
Methods and properties ordering in files
Where are unit tests for TrackBar child base class?

src/System.Windows.Forms/src/System/Windows/Forms/TrackBar.TrackBarAccessibleObject.cs

...Forms/tests/UnitTests/AccessibleObjects/TrackBar.TrackBarFirstButtonAccessibleObjectTests.cs

....Forms/tests/UnitTests/AccessibleObjects/TrackBar.TrackBarLastButtonAccessibleObjectTests.cs

...ndows.Forms/tests/UnitTests/AccessibleObjects/TrackBar.TrackBarThumbAccessibleObjectTests.cs

RussKie

Concerned with the tests implementations. Tests should have cyclomatic complexity (CC) of 1, i.e. not if...else....
In some instances (like if (createControl) ...) we decided to break this rule, but some tests seem to be really pushing it and have CC > 2. Please consider how the tests can be changed to reduce CC.

src/System.Windows.Forms/src/System/Windows/Forms/TrackBar.TrackBarAccessibleObject.cs

src/System.Windows.Forms/src/System/Windows/Forms/TrackBar.TrackBarChildAccessibleObject.cs

src/System.Windows.Forms/tests/IntegrationTests/WinformsControlsTest/TrackBars.Designer.cs

src/System.Windows.Forms/tests/IntegrationTests/WinformsControlsTest/TrackBars.cs

...em.Windows.Forms/tests/UnitTests/AccessibleObjects/TrackBar.TrackBarAccessibleObjectTests.cs

...Forms/tests/UnitTests/AccessibleObjects/TrackBar.TrackBarFirstButtonAccessibleObjectTests.cs

vladimir-krestov

Looks better

src/System.Windows.Forms/src/System/Windows/Forms/TrackBar.TrackBarAccessibleObject.cs

src/System.Windows.Forms/src/System/Windows/Forms/TrackBar.TrackBarChildAccessibleObject.cs

SergeySmirnov-Akvelon · 2021-06-23T13:01:20Z

CTI approved

RussKie · 2021-06-29T00:51:17Z

MC

SergeySmirnov-Akvelon · 2021-06-29T07:40:59Z

@RussKie, thank you. Fixed MC

Tanya-Solyanik

Looks good, my main question is if names match those in WPF or other framework.
If you open and close a form with a trackbar under inspect, is the trackbar still rooted?
Other comments are nits.

src/System.Windows.Forms/src/Resources/SR.resx

src/System.Windows.Forms/src/System/Windows/Forms/TrackBar.TrackBarAccessibleObject.cs

Tanya-Solyanik · 2021-06-30T00:59:15Z

src/System.Windows.Forms/src/System/Windows/Forms/TrackBar.TrackBarAccessibleObject.cs

Would /// <inheritdoc/> work?

Yes. It works. But I checked, even if we delete /// <summary> and /// <inheritdoc/>, then the description from the based "AccessibleObject" class is used.
Removed unneeded <summary> from this PR

But I checked, even if we delete ///
and /// , then the description from the based "AccessibleObject" class is used.

Interesting. Maybe inheritdocs is used by the tools that generate docs?

But I checked, even if we delete /// <summary> and /// <inheritdoc/>, then the description from the based "AccessibleObject" class is used.

Are you sure? If you don't have a /// comment then you don't get intellisense nor docs for the given member.

https://docs.microsoft.com/dotnet/csharp/programming-guide/xmldoc/inheritdoc

src/System.Windows.Forms/src/System/Windows/Forms/TrackBar.TrackBarAccessibleObject.cs

src/System.Windows.Forms/src/System/Windows/Forms/TrackBar.TrackBarChildAccessibleObject.cs

SergeySmirnov-Akvelon · 2021-06-30T09:44:37Z

If you open and close a form with a trackbar under inspect, is the trackbar still rooted?

Hi, @Tanya-Solyanik. I have checked this case for Inspect and Narrator. TrackBar accessibility objects are removed from memory even if these accessibility tools are active

src/System.Windows.Forms/src/System/Windows/Forms/TrackBar.TrackBarAccessibleObject.cs

RussKie · 2021-07-01T02:58:30Z

...Forms/tests/UnitTests/AccessibleObjects/TrackBar.TrackBarFirstButtonAccessibleObjectTests.cs

Why can't we use this?

Suggested change

public static IEnumerable<object[]> TrackBarAccessibleObject_FirstButtonIsDisplayed_TestData()

=> TrackBarTestHelper.TrackBarAccessibleObject_FirstButtonIsDisplayed_TestData();

[WinFormsTheory]

[MemberData(nameof(TrackBarAccessibleObject_FirstButtonIsDisplayed_TestData))]

[WinFormsTheory]

[CommonMemberData(typeof(TrackBarTestHelper), nameof(TrackBarTestHelper.TrackBarAccessibleObject_FirstButtonIsDisplayed_TestData))]

Thank you so much!!! Your approach makes it much easier to use the "TrackBarTestHelper" class.

Updated "SupportsUiaProviders" flag. Added and implemented "TrackBarAccessibleObject", "TrackBarFirstButtonAccessibleObject", "TrackBarLastButtonAccessibleObject" and "TrackBarThumbAccessibleObject" classes for working with the TrackBar and its elements. Added calls to "RaiseAutomationPropertyChangedEvent" and "RaiseAutomationEvent" methods for the correct announcing of the Narrator Added unit tests

Tanya-Solyanik

Please see a comment about opening a bug to test if accessible object is created across the whole codebase.

SergeySmirnov-Akvelon requested a review from a team as a code owner May 19, 2021 12:12

ghost assigned SergeySmirnov-Akvelon May 19, 2021

SergeySmirnov-Akvelon force-pushed the Issue-4914_Add_UIA_support_TrackBar branch 2 times, most recently from fad0c3b to 0eeb489 Compare May 21, 2021 07:00

SergeySmirnov-Akvelon marked this pull request as draft May 21, 2021 11:18

SergeySmirnov-Akvelon force-pushed the Issue-4914_Add_UIA_support_TrackBar branch 4 times, most recently from 24c2518 to d605848 Compare May 26, 2021 09:37

SergeySmirnov-Akvelon marked this pull request as ready for review May 26, 2021 09:37

SergeySmirnov-Akvelon marked this pull request as draft May 26, 2021 16:08

SergeySmirnov-Akvelon force-pushed the Issue-4914_Add_UIA_support_TrackBar branch from d605848 to 5561a94 Compare May 27, 2021 07:56

SergeySmirnov-Akvelon marked this pull request as ready for review May 27, 2021 08:04

vladimir-krestov reviewed May 27, 2021

View reviewed changes

SergeySmirnov-Akvelon force-pushed the Issue-4914_Add_UIA_support_TrackBar branch 3 times, most recently from 0aede10 to 5c22848 Compare May 28, 2021 13:11

dreddy-work added the waiting-review This item is waiting on review by one or more members of team label Jun 1, 2021

dreddy-work requested a review from Tanya-Solyanik June 1, 2021 20:58

SergeySmirnov-Akvelon mentioned this pull request Jun 4, 2021

Adding UIA support for ScrollBar #5044

Merged

RussKie suggested changes Jun 7, 2021

View reviewed changes

ghost added the waiting-author-feedback The team requires more information from the author label Jun 7, 2021

RussKie removed the waiting-review This item is waiting on review by one or more members of team label Jun 7, 2021

vladimir-krestov reviewed Jun 7, 2021

View reviewed changes

src/System.Windows.Forms/src/System/Windows/Forms/TrackBar.TrackBarAccessibleObject.cs Outdated Show resolved Hide resolved

src/System.Windows.Forms/src/System/Windows/Forms/TrackBar.TrackBarChildAccessibleObject.cs Outdated Show resolved Hide resolved

ghost removed the waiting-author-feedback The team requires more information from the author label Jun 15, 2021

SergeySmirnov-Akvelon force-pushed the Issue-4914_Add_UIA_support_TrackBar branch 2 times, most recently from 11cf2c1 to dff3736 Compare June 15, 2021 14:30

RussKie added the waiting-author-feedback The team requires more information from the author label Jun 16, 2021

SergeySmirnov-Akvelon force-pushed the Issue-4914_Add_UIA_support_TrackBar branch from dff3736 to 1e68e15 Compare June 17, 2021 13:35

ghost removed the waiting-author-feedback The team requires more information from the author label Jun 17, 2021

SergeySmirnov-Akvelon force-pushed the Issue-4914_Add_UIA_support_TrackBar branch from e06da86 to 914be23 Compare June 23, 2021 06:51

RussKie previously approved these changes Jun 23, 2021

View reviewed changes

RussKie added the waiting-author-feedback The team requires more information from the author label Jun 29, 2021

SergeySmirnov-Akvelon dismissed RussKie’s stale review via f6dddeb June 29, 2021 07:37

SergeySmirnov-Akvelon force-pushed the Issue-4914_Add_UIA_support_TrackBar branch from 914be23 to f6dddeb Compare June 29, 2021 07:37

ghost removed the waiting-author-feedback The team requires more information from the author label Jun 29, 2021

Tanya-Solyanik reviewed Jun 30, 2021

View reviewed changes

RussKie added the waiting-author-feedback The team requires more information from the author label Jun 30, 2021

ghost removed the waiting-author-feedback The team requires more information from the author label Jun 30, 2021

SergeySmirnov-Akvelon force-pushed the Issue-4914_Add_UIA_support_TrackBar branch from f6dddeb to 144d622 Compare June 30, 2021 08:01

SergeySmirnov-Akvelon added the waiting-review This item is waiting on review by one or more members of team label Jun 30, 2021

RussKie reviewed Jul 1, 2021

View reviewed changes

RussKie added waiting-author-feedback The team requires more information from the author and removed waiting-review This item is waiting on review by one or more members of team labels Jul 1, 2021

ghost removed the waiting-author-feedback The team requires more information from the author label Jul 1, 2021

SergeySmirnov-Akvelon force-pushed the Issue-4914_Add_UIA_support_TrackBar branch 2 times, most recently from 20b698d to fd63914 Compare July 5, 2021 09:43

SergeySmirnov-Akvelon force-pushed the Issue-4914_Add_UIA_support_TrackBar branch from fd63914 to e840b5d Compare July 5, 2021 14:27

Tanya-Solyanik approved these changes Jul 8, 2021

View reviewed changes

RussKie approved these changes Jul 9, 2021

View reviewed changes

RussKie merged commit 575687a into dotnet:main Jul 9, 2021

RussKie deleted the Issue-4914_Add_UIA_support_TrackBar branch July 9, 2021 03:46

ghost added this to the 6.0 Preview7 milestone Jul 9, 2021

ghost locked as resolved and limited conversation to collaborators Jan 28, 2022

-        public static IEnumerable<object[]> TrackBarAccessibleObject_FirstButtonIsDisplayed_TestData()
-            => TrackBarTestHelper.TrackBarAccessibleObject_FirstButtonIsDisplayed_TestData();
-        [WinFormsTheory]
-        [MemberData(nameof(TrackBarAccessibleObject_FirstButtonIsDisplayed_TestData))]
+        [WinFormsTheory]
+        [CommonMemberData(typeof(TrackBarTestHelper), nameof(TrackBarTestHelper.TrackBarAccessibleObject_FirstButtonIsDisplayed_TestData))]

Conversation

SergeySmirnov-Akvelon commented May 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Customer Impact

Regression?

Risk

Test methodology

Accessibility testing

Test environment(s)

Microsoft Reviewers: Open in CodeFlow

Uh oh!

vladimir-krestov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RussKie left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vladimir-krestov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

SergeySmirnov-Akvelon commented Jun 23, 2021

Uh oh!

RussKie commented Jun 29, 2021

Uh oh!

SergeySmirnov-Akvelon commented Jun 29, 2021

Uh oh!

Tanya-Solyanik left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Tanya-Solyanik Jun 30, 2021

Choose a reason for hiding this comment

Uh oh!

SergeySmirnov-Akvelon Jun 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Tanya-Solyanik Jul 2, 2021

Choose a reason for hiding this comment

Uh oh!

RussKie Jul 5, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SergeySmirnov-Akvelon commented Jun 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

RussKie Jul 1, 2021

Choose a reason for hiding this comment

Uh oh!

SergeySmirnov-Akvelon Jul 1, 2021

Choose a reason for hiding this comment

SergeySmirnov-Akvelon commented May 19, 2021 •

edited

Loading

Tanya-Solyanik left a comment •

edited

Loading

SergeySmirnov-Akvelon Jun 30, 2021 •

edited

Loading

SergeySmirnov-Akvelon commented Jun 30, 2021 •

edited

Loading