Implementing UIA provider for TextBox and MaskedTextBox controls by NikitaSemenovAkvelon · Pull Request #7741 · dotnet/winforms

NikitaSemenovAkvelon · 2022-09-06T05:54:01Z

Fixes #3421

Proposed changes

Enabled UIA provider for TextBox with nested classes and MaskedTextBox.
Refactored TextBox's child classes to reduce extra code.
Implemented IValueProvider for TextBoxBaseAccessibleObject.
Raised AutomationFocusChangedEvent in OnGotFocus methods.
Added few unit tests.

Customer Impact

Improving development experience for TextBox with nested classes and MaskedTextBox control accessibility.

Regression?

No

Risk

Minimal

Screenshots

Before

TextBox

MaskedTextBox

After

TextBox

MaskedTextBox

Test methodology

Manually
Accessibility tools
Unit tests
CTI

Accessibility testing

Inspect
AI
Narrator

Test environment(s)

8.0.100-alpha.1.22423.9

Microsoft Reviewers: Open in CodeFlow

dkazennov

As I understand in it's principle it is similar to other PRs to add UIA provider to controls (like #7314 I did).
Looks good to me.

vladimir-krestov

Looks good, but should be carefully tested

src/System.Windows.Forms/src/System/Windows/Forms/MaskedTextBox.cs

...rms/PropertyGridInternal/PropertyGridView.GridViewTextBox.GridViewTextBoxAccessibleObject.cs

src/System.Windows.Forms/src/System/Windows/Forms/TextBox.cs

src/System.Windows.Forms/src/System/Windows/Forms/TextBoxBase.TextBoxBaseAccessibleObject.cs

src/System.Windows.Forms/src/System/Windows/Forms/UpDownBase.UpDownEdit.cs

...ystem.Windows.Forms/tests/UnitTests/System/Windows/Forms/TextBoxBaseAccessibleObjectTests.cs

...ests/UnitTests/System/Windows/Forms/UpDownBase.UpDownEdit.UpDownEditAccessibleObjectTests.cs

dmitrii-drobotov

Nice work! Did you also test that there are no memory leaks for accessible objects after your changes?

src/System.Windows.Forms/src/System/Windows/Forms/TextBoxBase.cs

dmitrii-drobotov · 2022-09-12T14:14:41Z

src/System.Windows.Forms/src/System/Windows/Forms/TextBoxBase.cs

-                        if (@object is TextBoxBaseAccessibleObject accessibleObject)
-                        {
-                            accessibleObject.ClearObjects();
-                        }


The first case was removed because of UiaCore.UiaDisconnectProvider is called from another place.

It's called only when control has SupportsUiaProviders as true, but there is RichTextBox for which it's false. I'm not sure about all the details here, but RichTextBox could still somehow have an active AccessibilityObject for which we need to call UiaCore.UiaDisconnectProvider.

Tanya-Solyanik

Looks good, I added some comments, please see them. Had you compared the resulting provider set and properties to what is required by the docs https://docs.microsoft.com/windows/win32/winauto/uiauto-supporteditcontroltype?

Tanya-Solyanik · 2022-09-15T20:35:58Z

src/System.Windows.Forms/src/System/Windows/Forms/TextBoxBase.TextBoxBaseAccessibleObject.cs

-                => Owner is TextBoxBase textBoxBase && textBoxBase.ReadOnly;
+                => Owner is TextBoxBase { ReadOnly: true };
+
+            public override string? Name => Owner.AccessibleName ?? string.Empty;


Why does the base class implementation not work? TextBox should get AccessibleName from the label placed before it in the tab order. You dropped that part of the implementation. Maybe it does not make sense for the DGV cells, but it makes sense for other TextBoxes.

When I compared my implementation with MSAA behavior, the AccessibleName was empty. It was a reason why I implemented it that way. It was probably wrong and I've reworked it through the documentation above.

I see that now accessible names match the TextBox content:

I don't think this is expected behavior because it makes Narrator announce the content twice:

TextBoxes window, Some long long text for a multiline TextBox that contains several lines with withspaces,tabs and numbers 12345, edit, Some long long text for a multiline TextBox that contains several lines with withspaces,tabs and numbers 12345,
Some long long text for a RTL TextBox, edit, Some long long text for a RTL TextBox,
Some long long text for a TextBox, edit, Some long long text for a TextBox,

If there is a label before TextBox then name will be filled from it — this functionality now works, but the default name should be empty. Also, check docs:

UIA_NamePropertyId — The name of the edit control is typically generated from a static text label. If there is not a static text label, a property value for Name must be assigned by the application developer. The Name property should never contain the textual contents of the edit control.

I see this as an application problem that should be raised by the AccessibilityInsights for the app developer to fix. Framework does not know the business logic behind this text box and can't assign a meaningful name. As long as tooling raises the issue, we are good.

If there is a code path to keep Accessible Name empty , it would be a better fix, but I wouldn't spend much time on it (I think we have a property that controls it)

src/System.Windows.Forms/src/System/Windows/Forms/TextBoxBase.TextBoxBaseAccessibleObject.cs

...m.Windows.Forms/src/System/Windows/Forms/UpDownBase.UpDownEdit.UpDownEditAccessibleObject.cs

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

Tanya-Solyanik

Please see the question about the RichTextBox

src/System.Windows.Forms/src/PublicAPI.Unshipped.txt

src/System.Windows.Forms/src/System/Windows/Forms/DataGridViewTextBoxEditingControl.cs

src/System.Windows.Forms/src/System/Windows/Forms/UpDownBase.UpDownEdit.cs

...ystem.Windows.Forms/tests/UnitTests/System/Windows/Forms/TextBoxBaseAccessibleObjectTests.cs

dmitrii-drobotov

Looks good! I also checked the accessibility docs for edit control and found some inconsistencies. For example:

If an edit control contains placeholder text (for example, a cue banner), the text should be used as the HelpText property unless the text can be edited by the user and then reused as placeholder text. For example, the Windows Internet Explorer address bar contains the text "about:Tabs" when a new tab is opened. This is not HelpText because it is a programmatic address that can be used or edited by the user.

We have the PlaceholderText property, so it's possible to set a placeholder, but the HelpText property won't be set, because it's not implemented in this PR. But what I found is that this functionality didn't work before, so this PR won't be a regression.

I didn't go through all the docs content but there is at least UIA_IsPasswordPropertyId with the same problem — it's not implemented in this PR but wasn't working before either. Should we fix all these inconsistencies in this PR or later as separate issues?

Tanya-Solyanik · 2022-09-20T18:09:29Z

@dmitrii-drobotov - great findings! Yes, please open a new issue.

Tanya-Solyanik · 2022-09-20T18:16:49Z

/azp run

azure-pipelines · 2022-09-20T18:17:03Z

Azure Pipelines successfully started running 2 pipeline(s).

Tanya-Solyanik

Thank you! Please sent to the CTI team for testing if you had not done already.

NikitaSemenovAkvelon · 2022-09-28T09:33:17Z

Fixed cases that CTI found.

src/System.Windows.Forms/src/System/Windows/Forms/TextBoxBase.TextBoxBaseAccessibleObject.cs

src/System.Windows.Forms/src/System/Windows/Forms/MaskedTextBox.cs

...rms/tests/UnitTests/System/Windows/Forms/MaskedTextBox.MaskedTextBoxAccessibleObjectTests.cs

MelonWang1 · 2022-10-13T03:38:07Z

Verified this issue with latest .NET 8.0.100-alpha.1.22512.27, Add support UIA provider for TextBox and MaskedTextBox controls, screenshort as below:
Textbox:

MaskedTextbox:

…net#7741) Enabled UIA provider for TextBox with nested classes and MaskedTextBox. Refactored TextBox's child classes to reduce extra code. Implemented IValueProvider for TextBoxBaseAccessibleObject. Raised AutomationFocusChangedEvent in OnGotFocus methods. Added few unit tests.

NikitaSemenovAkvelon requested a review from a team as a code owner September 6, 2022 05:54

ghost assigned NikitaSemenovAkvelon Sep 6, 2022

NikitaSemenovAkvelon force-pushed the Issue_3421-Implementing_TextBox_UIA_Provider branch 2 times, most recently from a97f295 to f1276a6 Compare September 6, 2022 10:13

Tanya-Solyanik requested review from Tanya-Solyanik, dkazennov, dmitrii-drobotov and vladimir-krestov September 7, 2022 17:33

Tanya-Solyanik added this to the .NET 8.0 milestone Sep 7, 2022

dkazennov previously approved these changes Sep 8, 2022

View reviewed changes

vladimir-krestov reviewed Sep 11, 2022

View reviewed changes

NikitaSemenovAkvelon dismissed dkazennov’s stale review via efaa031 September 12, 2022 12:42

NikitaSemenovAkvelon force-pushed the Issue_3421-Implementing_TextBox_UIA_Provider branch from f1276a6 to efaa031 Compare September 12, 2022 12:42

dmitrii-drobotov reviewed Sep 12, 2022

View reviewed changes

NikitaSemenovAkvelon force-pushed the Issue_3421-Implementing_TextBox_UIA_Provider branch 2 times, most recently from 12bc14f to 661b73e Compare September 13, 2022 06:17

Tanya-Solyanik reviewed Sep 15, 2022

View reviewed changes

Tanya-Solyanik added the waiting-author-feedback The team requires more information from the author label Sep 15, 2022

NikitaSemenovAkvelon force-pushed the Issue_3421-Implementing_TextBox_UIA_Provider branch from 661b73e to ed69ba3 Compare September 19, 2022 05:10

ghost removed the waiting-author-feedback The team requires more information from the author label Sep 19, 2022

NikitaSemenovAkvelon force-pushed the Issue_3421-Implementing_TextBox_UIA_Provider branch from ed69ba3 to 05e9131 Compare September 19, 2022 06:08

Tanya-Solyanik reviewed Sep 20, 2022

View reviewed changes

Tanya-Solyanik added the waiting-author-feedback The team requires more information from the author label Sep 20, 2022

ghost removed the waiting-author-feedback The team requires more information from the author label Sep 20, 2022

NikitaSemenovAkvelon force-pushed the Issue_3421-Implementing_TextBox_UIA_Provider branch from 05e9131 to cc2a609 Compare September 20, 2022 04:29

dmitrii-drobotov reviewed Sep 20, 2022

View reviewed changes

Tanya-Solyanik added the waiting-author-feedback The team requires more information from the author label Sep 20, 2022

Tanya-Solyanik previously approved these changes Sep 27, 2022

View reviewed changes

NikitaSemenovAkvelon marked this pull request as draft September 27, 2022 11:33

ghost added the draft draft PR label Sep 27, 2022

NikitaSemenovAkvelon dismissed stale reviews from Tanya-Solyanik and CodeDummy-03 via 1f003dd September 28, 2022 07:55

NikitaSemenovAkvelon force-pushed the Issue_3421-Implementing_TextBox_UIA_Provider branch 2 times, most recently from 1f003dd to ead1356 Compare September 28, 2022 08:15

NikitaSemenovAkvelon marked this pull request as ready for review September 28, 2022 09:31

NikitaSemenovAkvelon added waiting-review This item is waiting on review by one or more members of team and removed draft draft PR labels Sep 28, 2022

dmitrii-drobotov suggested changes Sep 28, 2022

View reviewed changes

src/System.Windows.Forms/src/System/Windows/Forms/TextBoxBase.TextBoxBaseAccessibleObject.cs Outdated Show resolved Hide resolved

src/System.Windows.Forms/src/System/Windows/Forms/MaskedTextBox.cs Show resolved Hide resolved

ghost added the waiting-author-feedback The team requires more information from the author label Sep 28, 2022

NikitaSemenovAkvelon force-pushed the Issue_3421-Implementing_TextBox_UIA_Provider branch from ead1356 to 067de2e Compare September 29, 2022 04:56

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

NikitaSemenovAkvelon force-pushed the Issue_3421-Implementing_TextBox_UIA_Provider branch from 067de2e to 230b77f Compare September 29, 2022 11:01

dmitrii-drobotov reviewed Sep 29, 2022

View reviewed changes

src/System.Windows.Forms/src/System/Windows/Forms/MaskedTextBox.cs Outdated Show resolved Hide resolved

...rms/tests/UnitTests/System/Windows/Forms/MaskedTextBox.MaskedTextBoxAccessibleObjectTests.cs Show resolved Hide resolved

Implemented UIA provider for TextBox and MaskedTextBox

cfe49c2

NikitaSemenovAkvelon force-pushed the Issue_3421-Implementing_TextBox_UIA_Provider branch from 230b77f to cfe49c2 Compare September 30, 2022 04:23

dmitrii-drobotov approved these changes Sep 30, 2022

View reviewed changes

Tanya-Solyanik approved these changes Oct 5, 2022

View reviewed changes

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

Tanya-Solyanik merged commit 55b0586 into dotnet:main Oct 5, 2022

ghost modified the milestones: .NET 8.0, 8.0 Preview1 Oct 5, 2022

Olina-Zhang mentioned this pull request Oct 13, 2022

For MaskedTextBox with Mask setting, the Name property in Inspect should be empty #7938

Closed

dmitrii-drobotov mentioned this pull request Oct 24, 2022

Epic - [Accessibility] Implement UIA provider for all controls #3421

Closed

70 tasks

ghost locked as resolved and limited conversation to collaborators Nov 12, 2022

Conversation

NikitaSemenovAkvelon commented Sep 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Customer Impact

Regression?

Risk

Screenshots

Before

After

Test methodology

Accessibility testing

Test environment(s)

Microsoft Reviewers: Open in CodeFlow

Uh oh!

dkazennov left a comment

Choose a reason for hiding this comment

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!

dmitrii-drobotov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dmitrii-drobotov Sep 12, 2022 • 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 left a comment

Choose a reason for hiding this comment

Uh oh!

Tanya-Solyanik Sep 15, 2022

Choose a reason for hiding this comment

Uh oh!

NikitaSemenovAkvelon Sep 19, 2022

Choose a reason for hiding this comment

Uh oh!

dmitrii-drobotov Sep 20, 2022 • 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 Sep 20, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Tanya-Solyanik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dmitrii-drobotov left a comment

Choose a reason for hiding this comment

Uh oh!

Tanya-Solyanik commented Sep 20, 2022

Uh oh!

Tanya-Solyanik commented Sep 20, 2022

Uh oh!

azure-pipelines bot commented Sep 20, 2022

Uh oh!

Tanya-Solyanik left a comment

Choose a reason for hiding this comment

Uh oh!

NikitaSemenovAkvelon commented Sep 28, 2022

Uh oh!

NikitaSemenovAkvelon commented Sep 6, 2022 •

edited

Loading

dmitrii-drobotov Sep 12, 2022 •

edited

Loading

dmitrii-drobotov Sep 20, 2022 •

edited

Loading