Fixng 3214 - UIA providers support for Button control #3215

M-Lipin · 2020-05-05T14:31:37Z

Adding UIA providers support for Button's accessibility.

Fixes #3214

Proposed changes

Moving ButtonAccessibleObject to a separate file.
Moving ButtonBaseAccessibleObject to a separate file.
Adding UIA providers support for Button control accessi

Customer Impact

Improving development experience for Button control accessibility.
Prerequisite for Narrator focus is not moving in proper logical order when user is navigating by using caps + arrow keys (tab index is ignored) #2164

Regression?

No

Risk

Miniaml

Screenshots

Before

After

Test methodology

Manual testing.
Unit tests.
Automation tests (to be implemented).

Accessibility testing

Test environment(s)

.NET Core 5.0
Version: 5.0.100-alpha.1.20073.10
Commit: 29f4d693a9
Runtime Environment:
OS Name: Windows
OS Version: 10.0.18363
OS Platform: Windows
RID: win10-x64
Base Path: C:\Program Files\dotnet\sdk\5.0.100-alpha1-05536
Host (useful for support):
Version: 5.0.0-alpha.1.20072.3
Commit: c3dc1fdfdc

Microsoft Reviewers: Open in CodeFlow

M-Lipin · 2020-05-05T14:34:42Z

@Ryuugamine please implement unit tests.

RussKie

Tests

src/System.Windows.Forms/src/System/Windows/Forms/Button.ButtonAccessibleObject.cs

src/System.Windows.Forms/src/System/Windows/Forms/ButtonBase.ButtonBaseAccessibleObject.cs

RussKie

Multiple NRT issues, the PR will need to be rebased on top of #3273

src/System.Windows.Forms/src/System/Windows/Forms/ButtonBase.ButtonBaseAccessibleObject.cs

RussKie · 2020-06-10T07:34:23Z

src/System.Windows.Forms/src/System/Windows/Forms/Button.ButtonAccessibleObject.cs

+                {
+                    UiaCore.UIA.NamePropertyId => Name,
+                    UiaCore.UIA.AutomationIdPropertyId
+                        => _owningButton.IsHandleCreated ? _owningButton.Name : string.Empty,


#3244 (comment)

Thanks, fixed

src/System.Windows.Forms/src/System/Windows/Forms/Button.ButtonAccessibleObject.cs

src/System.Windows.Forms/src/System/Windows/Forms/ButtonBase.ButtonBaseAccessibleObject.cs

src/System.Windows.Forms/src/System/Windows/Forms/Button.cs

src/System.Windows.Forms/src/System/Windows/Forms/ButtonBase.ButtonBaseAccessibleObject.cs

src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/ButtonAccessibleObjectTests.cs

src/System.Windows.Forms/src/System/Windows/Forms/ButtonBase.ButtonBaseAccessibleObject.cs

src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/ButtonAccessibleObjectTests.cs

RussKie · 2020-06-25T01:37:46Z

...ows.Forms/tests/UnitTests/System/Windows/Forms/ButtonBase.ButtonBaseAccessibleObjectTests.cs

+
+namespace System.Windows.Forms.Tests
+{
+    public class ButtonBase_ButtonBaseAccessibleObjectTests


I updated the test docs to reflect the decision
https://github.com/dotnet/winforms/blob/master/Documentation/testing.md#naming

/cc: @hughbe FYI

RussKie · 2020-06-25T01:38:39Z

src/System.Windows.Forms/src/System/Windows/Forms/ButtonBase.ButtonBaseAccessibleObject.cs

+            {
+            }
+
+            private ButtonBase OwningButton => (ButtonBase)Owner;


Have we reached an agreement in this?

Yes, it should be changed to a local readonly field

RussKie

1 naming Q, other than that looks good 👍

vladimir-krestov

LGTM but need rework ButtonBaseAccessibleObject constructor

vladimir-krestov · 2020-06-25T07:13:34Z

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

@@ -6710,4 +6710,7 @@ Stack trace where the illegal operation occurred was:
  <data name="InputLanguageCultureNotFound" xml:space="preserve">
    <value>Could not find an InputLanguage for culture '{0}'.</value>
  </data>
+  <data name="Argument_InvalidValueType" xml:space="preserve"> 


Comment
And also think about merge conflicts with the same resource in #3228

Probably, but we need these changes in both PRs

vladimir-krestov · 2020-06-25T07:14:16Z

src/System.Windows.Forms/src/System/Windows/Forms/Button.ButtonAccessibleObject.cs

+                    _ => base.GetPropertyValue(propertyID)
+                };
+
+            internal override bool IsPatternSupported(UiaCore.UIA patternId)


It is not necessary now due to #3412

vladimir-krestov · 2020-06-25T07:15:32Z

src/System.Windows.Forms/src/System/Windows/Forms/ButtonBase.ButtonBaseAccessibleObject.cs

+            public ButtonBaseAccessibleObject(Control owner)
+                : base((owner as ButtonBase) ?? throw new ArgumentException(string.Format(SR.Argument_InvalidValueType, nameof(Owner), typeof(ButtonBase))))
+            {
+            }
+
+            private ButtonBase OwningButton => (ButtonBase)Owner;


Comment, please make like this

vladimir-krestov · 2020-06-25T07:17:44Z

src/System.Windows.Forms/src/System/Windows/Forms/ButtonBase.ButtonBaseAccessibleObject.cs

+            {
+            }
+
+            private ButtonBase OwningButton => (ButtonBase)Owner;


Yes, it should be changed to a local readonly field

vladimir-krestov · 2020-06-25T07:20:11Z

...ows.Forms/tests/UnitTests/System/Windows/Forms/ButtonBase.ButtonBaseAccessibleObjectTests.cs

+        [InlineData(false, false, AccessibleRole.HelpBalloon)]
+        public void ButtonBase_CreateAccessibilityInstance_InvokeWithRole_ReturnsExpected(bool createControl, bool defaultRole, AccessibleRole expectedAccessibleRole)
+        {
+            using var control = new SubButtonBase();


Please add a blank line before if everywhere

codecov · 2020-06-25T14:13:31Z

Codecov Report

Merging #3215 into master will increase coverage by 31.55858%.
The diff coverage is 99.42197%.

@@                 Coverage Diff                  @@
##              master       #3215          +/-   ##
====================================================
+ Coverage   66.95113%   98.50971%   +31.55857%     
====================================================
  Files           1338         448         -890     
  Lines         504925      251964      -252961     
  Branches       40866        4155       -36711     
====================================================
- Hits          338053      248209       -89844     
+ Misses        161315        3038      -158277     
+ Partials        5557         717        -4840

Flag	Coverage Δ
#Debug	`98.50971% <99.42197%> (+31.55857%)`	⬆️
#production	`?`
#test	`98.50971% <99.42197%> (-0.00814%)`	⬇️

SergeySmirnov-Akvelon · 2020-06-30T07:20:48Z

CTI approved

Adding UIA providers support for Button's accessibility.

RussKie · 2020-06-30T10:29:40Z

/azp run

azure-pipelines · 2020-06-30T10:29:50Z

Azure Pipelines successfully started running 1 pipeline(s).

M-Lipin requested a review from a team as a code owner May 5, 2020 14:31

ghost assigned M-Lipin May 5, 2020

M-Lipin changed the title ~~Moving Button and ButtonBase accessible objects to separate files.~~ Fixng 3214 - UIA providers support for Button control May 5, 2020

M-Lipin assigned Ryuugamine May 5, 2020

RussKie requested changes May 7, 2020

View reviewed changes

ghost added the 📭 waiting-author-feedback The team requires more information from the author label May 7, 2020

RussKie marked this pull request as draft May 7, 2020 01:53

RussKie requested changes May 13, 2020

View reviewed changes

ghost removed the 📭 waiting-author-feedback The team requires more information from the author label May 13, 2020

RussKie assigned SergeySmirnov-Akvelon and unassigned Ryuugamine Jun 9, 2020

RussKie reviewed Jun 10, 2020

View reviewed changes

vladimir-krestov mentioned this pull request Jun 10, 2020

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

Closed

70 tasks

SergeySmirnov-Akvelon force-pushed the Issue_3214_UIA_Accessibility_for_Button branch 2 times, most recently from bdca5d9 to adf6bc3 Compare June 18, 2020 08:49

RussKie marked this pull request as ready for review June 19, 2020 04:05

RussKie requested changes Jun 19, 2020

View reviewed changes

ghost added the 📭 waiting-author-feedback The team requires more information from the author label Jun 19, 2020

SergeySmirnov-Akvelon force-pushed the Issue_3214_UIA_Accessibility_for_Button branch from bb2d37c to 7165d80 Compare June 24, 2020 14:47

RussKie reviewed Jun 25, 2020

View reviewed changes

vladimir-krestov reviewed Jun 25, 2020

View reviewed changes

SergeySmirnov-Akvelon force-pushed the Issue_3214_UIA_Accessibility_for_Button branch from 589b3b6 to 41c92ea Compare June 25, 2020 13:55

SergeySmirnov-Akvelon force-pushed the Issue_3214_UIA_Accessibility_for_Button branch 3 times, most recently from ace8d53 to af91c7e Compare June 26, 2020 14:13

SergeySmirnov-Akvelon removed the 📭 waiting-author-feedback The team requires more information from the author label Jun 26, 2020

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

Moving Button and ButtonBase accessible objects to separate files.

db2d0c1

Adding UIA providers support for Button's accessibility.

SergeySmirnov-Akvelon force-pushed the Issue_3214_UIA_Accessibility_for_Button branch from af91c7e to db2d0c1 Compare June 30, 2020 09:37

dotnet deleted a comment from azure-pipelines bot Jun 30, 2020

RussKie approved these changes Jun 30, 2020

View reviewed changes

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

RussKie merged commit fdb4350 into dotnet:master Jun 30, 2020

RussKie deleted the Issue_3214_UIA_Accessibility_for_Button branch June 30, 2020 10:50

ghost added this to the 5.0 Preview8 milestone Jun 30, 2020

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixng 3214 - UIA providers support for Button control #3215

Fixng 3214 - UIA providers support for Button control #3215

M-Lipin commented May 5, 2020 •

edited by RussKie

M-Lipin commented May 5, 2020

RussKie left a comment

RussKie left a comment

RussKie Jun 10, 2020

SergeySmirnov-Akvelon Jun 10, 2020

RussKie Jun 25, 2020

RussKie Jun 25, 2020

vladimir-krestov Jun 25, 2020

RussKie left a comment

vladimir-krestov left a comment

vladimir-krestov Jun 25, 2020

SergeySmirnov-Akvelon Jun 25, 2020

vladimir-krestov Jun 25, 2020

vladimir-krestov Jun 25, 2020

vladimir-krestov Jun 25, 2020

vladimir-krestov Jun 25, 2020

codecov bot commented Jun 25, 2020

SergeySmirnov-Akvelon commented Jun 30, 2020

RussKie commented Jun 30, 2020

azure-pipelines bot commented Jun 30, 2020

Fixng 3214 - UIA providers support for Button control #3215

Fixng 3214 - UIA providers support for Button control #3215

Conversation

M-Lipin commented May 5, 2020 • edited by RussKie

Proposed changes

Customer Impact

Regression?

Risk

Screenshots

Before

After

Test methodology

Accessibility testing

Test environment(s)

Microsoft Reviewers: Open in CodeFlow

M-Lipin commented May 5, 2020

RussKie left a 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

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

vladimir-krestov 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

codecov bot commented Jun 25, 2020

Codecov Report

SergeySmirnov-Akvelon commented Jun 30, 2020

RussKie commented Jun 30, 2020

azure-pipelines bot commented Jun 30, 2020

M-Lipin commented May 5, 2020 •

edited by RussKie