Add support for UIA with chromium based browsers #12025

feerrenrut · 2021-02-02T14:03:15Z

This is based off of PR "Add advanced setting to allow UIA in chromium based browsers #11079"

However, due to the large amount of moved code, NV Access has decided not to squash this PR. Instead the commits have been cleaned up to make following the history as easy as possible. The changes from #11079 are as follows:

Code is moved between files verbatim, in commits independent from any other changes. It should be noted that these commits will not run, but serve to make diffs / blame easier to follow.
Linting is done in independent commits. This allows logic changes to be differentiated from "clean up"
UIA/edge becomes UIA/spartan_edge and a placeholder is added for UIA/anaheim_edge

This PR supersedes and thus closes #11079, however the history there is still relevant and useful.

Link to issue number:

Related to #10555

Summary of the issue:

Chromium based browsers now contain a UIA implementation which may be exposed (E.G. in Edge Anaheim). Allowing this to be used will allow developers to work to stabilize and mature this technology. For users on systems where process injection is not possible, access to UIA in Edge can act as a fallback option.

Description of how this pull request fixes the issue:

Provides an advanced setting to use UIA when available. Moves significant portions of the UIA code for reuse:

Web
Legacy Edge (Spartan)
Chromium based browsers such as Edge Anaheim

Testing performed:

Manual testing.

Known issues with pull request:

None

Change log entry:

New features

- Early support for UIA with Chromium based browsers (such as Edge).

AppVeyorBot · 2021-02-02T14:10:23Z

Build (for testing PR)

See test results for failed build of commit 7f60febae5

leonardder · 2021-02-02T14:13:45Z

I'm slightly worried about introducing underscores in the names here, as it is not following the naming conventions we usually use.

michaelDCurran · 2021-02-02T22:11:53Z

True we have not used underscores before, however, I think this is a suitable way to very clearly denote very different implementations of Edge. Especially when they are living in the same directory.

michaelDCurran

I have done the same manual testing I did on the original pr and all was good for modern Edge.
I hit an issue in classic edge, but this was actually introduced by pr #11969 so I have separately opened pr #12026 to address that. But other than that things seemed to be working well.

feerrenrut · 2021-02-03T00:47:19Z

very clearly denote very different implementations of Edge. Especially when they are living in the same directory.

Hmm perhaps there should be more structure here eg "namespaces". Consider if we moved the files:

- UIA
  |- New Package: Web
  |   |- New Package: Chromium
  |   |   |- EdgeAnaheim.py
  |   |- EdgeSpartan.py

michaelDCurran · 2021-02-03T00:52:46Z

I'm okay with either way. Don't think the further directory structure is necessary work, but fine if it gets done.

lukaszgo1 · 2021-02-03T02:11:51Z

source/NVDAObjects/__init__.py

@@ -417,6 +417,9 @@ def _get_name(self):
 		"""
 		return ""

+	#: Type definition for auto prop '_get_role'
+	role: str


I don't think this annotation is correct since roles are represented by ROLE_ constants in controlTypes and they are integers.

Yes you are right @lukaszgo1, good catch.

feerrenrut · 2021-02-03T05:08:42Z

I've been looking at the layout of this code and reconsidering whether a nested structure is better. I think it is better for this code to stick with a flatter structure, otherwise there are dependencies going in the wrong direction:

chromium depends on Web
Web depends on UIA
spartan_edge depends on UIA
Nesting these as packages requires requires multiple level relative imports or absolute imports.

I will however rename spartan_edge and anaheim _edge to spartanEdge and anaheimEdge respectively.

UIA.Web is a new base for browser specific implementations: - spartan-edge - Chromium based browsers: anaheim-edge, chrome This commit is a direct move (no alterations) of code to preserve history. - Move function splitUIAElementAttribs - Move EdgeTextInfo - Move shareable aspects of class EdgeNode: * _get_role * _get_states * _get_ariaProperties * _get_isCurrent * _get_roleText * _get_placeholder * _get_landmark - Move class EdgeList - Move class EdgeHeadingQuickNavItem - Move class EdgeHeadingQuicknavIterator - Move shareable aspects of class EdgeHTMLTreeInterceptor: * makeTextInfo * shouldPassThrough * _iterNodesByType

This is a module containing common classes above the browser specific classes (edge.py, chromium.py). The browser specific classes inherit from these web module classes. - Move splitUIAElementAttribs(attribsString) from edge module to web module - Move common implementation of edge.EdgeNode and rename to web.UIAWeb(UIA), edge.EdgeNode now inherits from web.UIAWeb. - Move implementation of edge.EdgeList and rename to web.List(UIAWeb), edge.EdgeList now inherits from web.List - Move implementation of edge.EdgeHTMLTreeInterceptor and rename to web.UIAWebTreeInterceptor, edge.EdgeHTMLTreeInterceptor now inherits from web.UIAWebTreeInterceptor

The concrete implementation relied on knowing that self is actually a subclass and providing varied behaviour. Instead, abstract this test so it can be altered via polymorphism.

Use chromium treeinterceptor Use Web module for chromium module Use chromium.ChromiumUIADocument in findOverlayClasses

Trade off: slight degradation of performance. UIATextInfo._getTextWithFieldsForUIARange: Test each child to see if it is clipped by or lives completely outside the parent text range, and if so appropriately clip the child range and break out of the for loop. Previously these checks only happened on the final child to try and avoid extra calls, but Edge sometimes returns many more children then it should when on the end of a list, and a trivial slow down is much preferred over a 20 second freeze.

Trade-off: only the first line is fetched. speech.getObjectSpeech: If the given object should have its text presented (E.g. a document or edit field) and the line at the selection cannot be fetched, then only present the first line. Previously the text for the entire object would be fetched, which in the case of Edgium, was extremely costly and could cause a freeze of 10 seconds or more.

As headings are exposed in the element tree, don't expose them in format fields.

UIATextInfo._getTextwithFieldsForUIARange: skip children that appear completely before the start of the parent textRange. Stops a freeze on some checkboxes in Edgium and is generally safer.

Make sure interactive / presentational lists are presented correctly in browseMode in Edgium.

… with UIA Embedded object characters in Edgium should not stop overriding labels from being used.

Allows ignoring of layout tables when configured to do so.

Ensure the value of comboboxes are used as content.

Names of controls (that do not use their name as content) are always reported: Currently no way to tell if author has explicitly set name. Therefore always report the name if the control is not of a type that by definition uses its name for content. This may cause some duplicate speaking, but that is currently better than nothing at all.

PR #12025 was not squash merged (which is the usual practice for this project). Instead, to preserve the history of moved then modified code, the commits have been rebased directly onto master.

…12319) Fixes #12114 PR #12025 started catching only very specific exceptions when getting selection of edit fields. However in Firefox attempting to get caret for non focused edit fields results in RuntimeError which made it impossible to speak these controls. Description of how this pull request fixes the issue: When getting content of edit fields in speech, we're now catching RuntimeError as well and treating this situation like no selection.

feerrenrut requested a review from michaelDCurran February 2, 2021 14:03

feerrenrut mentioned this pull request Feb 2, 2021

Add advanced setting to allow UIA in chromium based browsers #11079

Closed

4 tasks

michaelDCurran previously approved these changes Feb 3, 2021

View reviewed changes

lukaszgo1 reviewed Feb 3, 2021

View reviewed changes

feerrenrut dismissed michaelDCurran’s stale review via 89d2d34 February 3, 2021 05:59

feerrenrut force-pushed the UiaWithChromium branch 2 times, most recently from 89d2d34 to d667d35 Compare February 3, 2021 08:55

feerrenrut and others added 16 commits February 3, 2021 16:56

Rename edge -> spartanEdge

6f7cebf

Depend upon abstract _isIframe rather than concrete impl

d3af571

The concrete implementation relied on knowing that self is actually a subclass and providing varied behaviour. Instead, abstract this test so it can be altered via polymorphism.

Fix lint for moved code

e13e128

Add chromium module

61d1e46

Use chromium treeinterceptor Use Web module for chromium module Use chromium.ChromiumUIADocument in findOverlayClasses

Fix duplicate heading announcement Chromium with UIA

ad73947

As headings are exposed in the element tree, don't expose them in format fields.

Fix freeze on checkboxes for chromium with UIA

b202125

UIATextInfo._getTextwithFieldsForUIARange: skip children that appear completely before the start of the parent textRange. Stops a freeze on some checkboxes in Edgium and is generally safer.

Fix list presentation while in browse mode for Chromium with UIA

29d74cd

Make sure interactive / presentational lists are presented correctly in browseMode in Edgium.

Fix overriding labels when embedded object chars present for Chromium…

5c8c91b

… with UIA Embedded object characters in Edgium should not stop overriding labels from being used.

Fix identification of layout tables for Chromium with UIA

a188534

Allows ignoring of layout tables when configured to do so.

Fix missing value for comboboxes for Chromium with UIA

5275600

Ensure the value of comboboxes are used as content.

Add user config for UIA with Chromium

49c04df

feerrenrut force-pushed the UiaWithChromium branch from d667d35 to 49c04df Compare February 3, 2021 08:56

Update changes file for PR #12025

e8200bc

PR #12025 was not squash merged (which is the usual practice for this project). Instead, to preserve the history of moved then modified code, the commits have been rebased directly onto master.

feerrenrut merged commit 11ba984 into master Feb 3, 2021

feerrenrut deleted the UiaWithChromium branch February 3, 2021 09:38

nvaccessAuto added this to the 2021.1 milestone Feb 3, 2021

CyrilleB79 mentioned this pull request Feb 4, 2021

Fixes somes context help issues in advanced settings panel. #12034

Merged

feerrenrut mentioned this pull request Feb 9, 2021

Support UI Automation in Microsoft Edge Chromium and Google Chrome #10555

Closed

lukaszgo1 mentioned this pull request Apr 21, 2021

Fix-up of 12025: No longer fail to report content of nonfocused text fields in Firefox #12319

Merged

7 tasks

Add support for UIA with chromium based browsers #12025

Add support for UIA with chromium based browsers #12025

feerrenrut commented Feb 2, 2021 •

edited

AppVeyorBot commented Feb 2, 2021

leonardder commented Feb 2, 2021

michaelDCurran commented Feb 2, 2021 via email

michaelDCurran left a comment

feerrenrut commented Feb 3, 2021

michaelDCurran commented Feb 3, 2021 via email

lukaszgo1 Feb 3, 2021

feerrenrut Feb 3, 2021

feerrenrut commented Feb 3, 2021

Add support for UIA with chromium based browsers #12025

Add support for UIA with chromium based browsers #12025

Conversation

feerrenrut commented Feb 2, 2021 • edited

Link to issue number:

Summary of the issue:

Description of how this pull request fixes the issue:

Testing performed:

Known issues with pull request:

Change log entry:

AppVeyorBot commented Feb 2, 2021

leonardder commented Feb 2, 2021

michaelDCurran commented Feb 2, 2021 via email

michaelDCurran left a comment

Choose a reason for hiding this comment

feerrenrut commented Feb 3, 2021

michaelDCurran commented Feb 3, 2021 via email

lukaszgo1 Feb 3, 2021

Choose a reason for hiding this comment

feerrenrut Feb 3, 2021

Choose a reason for hiding this comment

feerrenrut commented Feb 3, 2021

feerrenrut commented Feb 2, 2021 •

edited