Skip to content

Adding UIA support for WebBrowser#7314

Merged
RussKie merged 1 commit intodotnet:mainfrom
dkazennov:Issue_3421_WebBrowser_ShouldSupport_UIAProvider
Jun 28, 2022
Merged

Adding UIA support for WebBrowser#7314
RussKie merged 1 commit intodotnet:mainfrom
dkazennov:Issue_3421_WebBrowser_ShouldSupport_UIAProvider

Conversation

@dkazennov
Copy link
Contributor

@dkazennov dkazennov commented Jun 16, 2022 •
edited by ghost
Loading

Partially implements #3421
For .Net 7

Proposed changes

  • Updated "SupportsUiaProviders" flag to WebBrowser
  • Added and implemented WebBrowser.WebBrowserAccessibleObject
  • Added unit tests
  • Testing application
    WebBrowserWinFormsApp.zip

Customer Impact

Screenshots

Before

before

ProviderDescription: "[pid:22832,providerId:0x402AA Main:Nested [pid:27400,providerId:0x402AA Main(parent link):Microsoft: MSAA Proxy (unmanaged:UIAutomationCore.dll)]; Hwnd(parent link):Microsoft: HWND Proxy (unmanaged:uiautomationcore.dll)]"

After

after

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

Regression?

  • No

Risk

  • Minimal

Test methodology

  • Unit tests
  • Manual with custom application

Accessibility testing

  • Inspect
  • Accessibility Insights

Test environment

dotnet : 7.0.0-preview.4.22229.4

Microsoft Reviewers: Open in CodeFlow

@dkazennov dkazennov requested a review from a team as a code owner June 16, 2022 14:06
@ghost ghost assigned dkazennov Jun 16, 2022
@dkazennov dkazennov added the tenet-accessibility MAS violation, UIA issue; problems with accessibility standards label Jun 16, 2022
@dkazennov dkazennov force-pushed the Issue_3421_WebBrowser_ShouldSupport_UIAProvider branch from 0f28ee9 to 15873a3 Compare June 16, 2022 14:46
@RussKie
Copy link
Contributor

RussKie commented Jun 17, 2022

To be honest I don't think we should be spending any time updating any of WebBrowser API.

  1. IE is officially retired and out of support as of June 15, 2022.
  2. We're planning to deprecated all related API (see Mark WebBrowser and all related (e.g., HTML* types) as obsolete #6964).

/cc: @merriemcgaw @Tanya-Solyanik

@dkazennov
Copy link
Contributor Author

dkazennov commented Jun 17, 2022 •
edited
Loading

Fixed tests: ControlAccessibleObject_GetPropertyValue_AccessKey_ReturnExpected
added typeof(WebBrowser) to s_controlsIgnoringTextChangesForTests (since WebBrowserBase has public override string Text property which throws exception in setter).

@dkazennov dkazennov force-pushed the Issue_3421_WebBrowser_ShouldSupport_UIAProvider branch from 15873a3 to fc1ea12 Compare June 17, 2022 07:36
@dkazennov
Copy link
Contributor Author

dkazennov commented Jun 17, 2022

  1. WebBrowser class implements UIA provider now. New class WebBrowserAccessibleObject passes unit tests.
  2. Custom application works with WebBrowser control (both original and changed) as intended.
  3. New unit tests work as intended.

I believe that everything works now. So can we keep this fix as it is already complete?

vladimir-krestov
vladimir-krestov reviewed Jun 17, 2022
View reviewed changes
Copy link
Contributor

@vladimir-krestov vladimir-krestov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see risk of implementing this fast fix, even if IE is outdated.
The main reason - #3037. We need to provide a correct accessibility tree for implemented apps, that can use WebBrowser.
@merriemcgaw, @Tanya-Solyanik, your thoughts?

src/System.Windows.Forms/src/System/Windows/Forms/WebBrowser.WebBrowserAccessibleObject.cs
internal override object? GetPropertyValue(UiaCore.UIA propertyID)
=> propertyID switch
{
UiaCore.UIA.AutomationIdPropertyId
Copy link
Contributor

@vladimir-krestov vladimir-krestov Jun 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove, it will be implemented in #7206

Copy link
Contributor Author

@dkazennov dkazennov Jun 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove, it will be implemented in #7206

In #7206 it will be UiaCore.UIA.AutomationIdPropertyId => AutomationId instead. Returning Owner.Name is pre-7206.
Or should we change it pre-emptively?

Copy link
Member

@merriemcgaw merriemcgaw Jun 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're close I don't mind making sure apps with the WebBrowser control have the right UIA tree. But I wouldn't go spending time implementing too many patterns on this particular control.

Copy link
Contributor Author

@dkazennov dkazennov Jun 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove, it will be implemented in #7206

Since #7206 is not yet implemented, I propose we don't change this method until #7206 is implemented (or if this issue is implemented first we will change WebBrowserAccessibleObject in #7206).

Copy link
Contributor

@RussKie RussKie Jun 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a friendly reminder to not lose this

Copy link
Contributor Author

@dkazennov dkazennov Jun 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a friendly reminder to not lose this

Thank you! It will be done now )

src/System.Windows.Forms/src/System/Windows/Forms/WebBrowser.cs
return new WebBrowserSite(this);
}

protected override AccessibleObject CreateAccessibilityInstance() => new WebBrowserAccessibleObject(this);
Copy link
Contributor

@vladimir-krestov vladimir-krestov Jun 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@runfoapp runfoapp bot mentioned this pull request Jun 17, 2022
Closed
@dkazennov
Issue 3421
686642b 
UIAProvider support for WebBrowser control and UnitTests
@dkazennov dkazennov force-pushed the Issue_3421_WebBrowser_ShouldSupport_UIAProvider branch from fc1ea12 to 686642b Compare June 20, 2022 08:53
@RussKie
Copy link
Contributor

RussKie commented Jun 22, 2022

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Jun 22, 2022

Azure Pipelines successfully started running 2 pipeline(s).

RussKie
RussKie reviewed Jun 22, 2022
View reviewed changes
Copy link
Contributor

@RussKie RussKie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dkazennov
Copy link
Contributor Author

dkazennov commented Jun 23, 2022

CTI requested

@dkazennov dkazennov 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
@Olina-Zhang
Copy link
Member

Olina-Zhang commented Jun 24, 2022

Tested this PR with no new issue found.

@Olina-Zhang Olina-Zhang removed 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 24, 2022
@RussKie RussKie merged commit 250c0e1 into dotnet:main Jun 28, 2022
@ghost ghost added this to the 7.0 Preview7 milestone Jun 28, 2022
@dkazennov dkazennov mentioned this pull request Jun 28, 2022
Merged
@Philip-Wang01
Copy link
Contributor

Philip-Wang01 commented Jul 1, 2022

Verified on latest .NET core 7.0 build: .NET 7.0.100-preview.7.22330.8, issue is fixed. Screenshot as below:
image

@vladimir-krestov vladimir-krestov mentioned this pull request Jul 13, 2022
Closed
70 tasks
@Cassie-Li01
Copy link

Cassie-Li01 commented Jul 22, 2022

Verified on .NET 7.0.100-preview.7.22370.3 TP build, issue is fixed, screenshot as below:
image

@dkazennov dkazennov mentioned this pull request Aug 9, 2022
Merged
@ghost ghost locked as resolved and limited conversation to collaborators Aug 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@merriemcgaw merriemcgaw merriemcgaw left review comments

@Tanya-Solyanik Tanya-Solyanik Awaiting requested review from Tanya-Solyanik

@dmitrii-drobotov dmitrii-drobotov Awaiting requested review from dmitrii-drobotov

+2 more reviewers

@RussKie RussKie RussKie approved these changes

@vladimir-krestov vladimir-krestov vladimir-krestov left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@dkazennov dkazennov

Labels

tenet-accessibility MAS violation, UIA issue; problems with accessibility standards

Projects

None yet

Milestone

7.0 Preview7

Development

Successfully merging this pull request may close these issues.

7 participants

@dkazennov @RussKie @Olina-Zhang @Philip-Wang01 @Cassie-Li01 @merriemcgaw @vladimir-krestov