nvda icon indicating copy to clipboard operation
nvda copied to clipboard

Implement support for writing UI Automation Remote Operations natively in NVDA using Python

Open michaelDCurran opened this issue 1 year ago • 32 comments

Link to issue number:

None

Background

Several years ago Microsoft added a low-level API in Windows to allow the caller to execute multiple actions in a UI Automation remote provider via only one cross-process call. Limiting the amount of cross-process calls is necessary for providing a responsive experience, especially for interacting with applications hosted in Windows Defender Application Guard (WDAG) or as a Remote Application Integrated Locally (RAIL). There are also scenarios such as searching for / listing headings in large documents, which can bennifit from limiting cross-process calls, even when the application is running on the local machine. the Windows.UI.UIAutomation.Core.CoreAutomationRemoteOperation is a winRT interface that allows the caller to specify the actions as a string of byte code representing a specific set of instructions for a conceptual virtual machine which is executed within the remote provider. The instructions allow you to manipulate UI Automation elements, patterns and text ranges, as well as perform basic logic and collect information using ints, floats, strings, GUIDs and arrays created within the conceptual virtual machine. Although these instructions are well-defined, public documentation is limited and programming with these raw instructions can be slow and complicated to read. Microsoft released the Microsoft-UI-UIAutomation Remote Operations Library which provides some higher level ways of building and executing these instructions, including intermediate winRT interfaces, and high-level pure c++/winRT interfaces. NV Access has made many prototypes with this library, and has made significant contributions to that project over the years. Today, NVDA includes this library as a depedency, and currently uses it to access the new UI Automation Custom Extensions feature, which is required to access certain information in Microsoft Word. In theory, NVDA could make a much greater uuse of UI automation Remote Operations to improve performance across many parts of Windows and other applications. However, there are several issues with the Microsoft UI Automation Remote Operations Library which slow down or limit greater usage in NVDA, including:

  • All remote operations logic must be written in C++ and not Python. This creates a large learning curve, if not a quite uncomfortable context switch at very least.
  • the Remote Operations Library seems to be no longer maintained by Microsoft.
  • The Remote Operations Library currently contributes an extra massive 16 minutes to NvDA's build time, as it must compile the whole of the C++/winRT API set.

Solution

This PR introduces a new remote operations library written in pure Python, which wraps the low-level CoreAutomationRemoteOperation winRT interfaces in Windows. This new library replaces the need for the older Microsoft Remote Operations Library, and therefore is no longer a dependency.

I had several initial goals in mind with this new library:

  1. To support our current usage of remote operations (namely UI Automation custom extensions: isExtension and CallExtension). If I could not achieve our current requirements, there would be no point continuing.
  2. To make a significant performance improvement to at least one extra action or scenario in NvDA (E.g. quick navigation by heading in Microsoft Word). This would prove that implementation of a fix / feature was easier with the new library, and would make it worth spending the initial time on it.

I was able to achieve both of these goals.

Description of user facing changes

  • In browse mode in Microsoft Word with UIA enabled, quick nav to headings, and listing headings in Elements list, is now up to 30 times faster. E.g. Jumping to a heading at the bottom of a 160 page document all the way from the top previously took ~4 seconds, now it takes ~0.13 seconds.

Description of development approach

  • A new UIAHandler._remoteOps package has been added, which interfaces with the low-level Windows Remote Operations API, providing higher-level APIs and types to aide in building and executing easy to read remote algorithms. See The NVDA remote operations readme for explanations and example code for all features.
  • UIAHandler.remote's msWord_getCustomAttributeValue has been re-written to use the new NVDA remote operations library.
  • Some extra functions were added to UIAHandler.remote, including findFirstHeadingInTextRange and collectAllHeadingsInTextRange, built on the NVDA remote ops library. UIA browse mode makes use of these functions on Windows 11, to majorly speed up jumping to / listing headings.

Testing strategy:

  • ~40 unit tests were added for the NVDA remote operations library, see tests/unit/test_UIARemoteOps/test_highLevel.py
  • In browse mode in Microsoft Word with UIA enabled, tested pressing h to jump to each heading in this document, also tested opening the Elements list and listing headings.

Known issues with pull request:

  • Documentation could be improved.

Code Review Checklist:

  • [x] Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • [x] Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • [x] UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • [x] API is compatible with existing add-ons.
  • [x] Security precautions taken.

Summary by CodeRabbit

  • Chores

    • Removed the microsoft-ui-uiautomation submodule and its references from the project.
  • New Features

    • Introduced functionality for creating, manipulating, and retrieving results from remote operations in UI Automation.
    • Added support for building and managing remote operations within NVDA.
    • Enhanced local execution of operations with new classes and functions.
    • Implemented a function for iterating over text units in a remote text range.
  • Documentation

    • Updated readme files to reflect the removal of microsoft-ui-uiautomation and added guidelines for new remote operations features.

michaelDCurran avatar Feb 22 '24 22:02 michaelDCurran

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • FAIL: Lint check. See test results for more information.
  • FAIL: System tests (tags: installer NVDA). See test results for more information.
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/jn05uj4xm025rmgw/artifacts/output/nvda_snapshot_pr16214-31153,fbf340e0.exe
  • CI timing (mins): INIT 0.0, INSTALL_START 1.0, INSTALL_END 0.9, BUILD_START 0.0, BUILD_END 12.8, TESTSETUP_START 0.0, TESTSETUP_END 0.2, TEST_START 0.0, TEST_END 0.7, FINISH_END 0.1

See test results for failed build of commit fbf340e091

AppVeyorBot avatar Feb 22 '24 22:02 AppVeyorBot

CC @mltony, maybe this PR could interest you :)

ABuffEr avatar Feb 24 '24 17:02 ABuffEr

Wow what an amazing work, thank you very much Mick this is really great decision to make NVDA more independent. I guess this will open up many possibilities to solve also other issues we have with UIA in MS Office products at least.

Adriani90 avatar Feb 24 '24 17:02 Adriani90

So I've been reading and trying to understand this PR today. It must be the most awesome computer science project in years. Great incentive to further optimize the use of UIA in MS Office!

dkager avatar Feb 24 '24 20:02 dkager

ForkIfTrue has not been implemented as none of the higher-level APIs or types depend upon it currently. Thus, the unit tests as currently written would not exercise it either.

But of course if there is a clear need, then someone is welcome to add it.

michaelDCurran avatar Feb 24 '24 21:02 michaelDCurran

It has certainly been a very fun personal project that up to now has been outside of work time, as I was really not sure just how successful it would be.

michaelDCurran avatar Feb 24 '24 21:02 michaelDCurran

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • FAIL: Lint check. See test results for more information.
  • FAIL: System tests (tags: installer NVDA). See test results for more information.
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/b3k6611mkgoo0m6k/artifacts/output/nvda_snapshot_pr16214-31175,4d27efae.exe
  • CI timing (mins): INIT 0.0, INSTALL_START 1.0, INSTALL_END 0.9, BUILD_START 0.0, BUILD_END 12.4, TESTSETUP_START 0.0, TESTSETUP_END 0.2, TEST_START 0.0, TEST_END 0.7, FINISH_END 0.2

See test results for failed build of commit 4d27efae37

AppVeyorBot avatar Feb 24 '24 21:02 AppVeyorBot

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • PASS: Lint check.
  • FAIL: System tests (tags: installer NVDA). See test results for more information.
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/cnfbyr0il1ebgelo/artifacts/output/nvda_snapshot_pr16214-31182,2e598324.exe
  • CI timing (mins): INIT 0.0, INSTALL_START 1.0, INSTALL_END 0.9, BUILD_START 0.0, BUILD_END 11.8, TESTSETUP_START 0.0, TESTSETUP_END 0.2, TEST_START 0.0, TEST_END 0.7, FINISH_END 0.1

See test results for failed build of commit 2e59832472

AppVeyorBot avatar Feb 25 '24 01:02 AppVeyorBot

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • PASS: Lint check.
  • FAIL: System tests (tags: installer NVDA). See test results for more information.
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/g673t291ohev2wra/artifacts/output/nvda_snapshot_pr16214-31183,e6bcaa01.exe
  • CI timing (mins): INIT 0.0, INSTALL_START 1.0, INSTALL_END 0.9, BUILD_START 0.0, BUILD_END 12.0, TESTSETUP_START 0.0, TESTSETUP_END 0.2, TEST_START 0.0, TEST_END 0.6, FINISH_END 0.2

See test results for failed build of commit e6bcaa017e

AppVeyorBot avatar Feb 25 '24 06:02 AppVeyorBot

Has it been considered to add the library documentation to the developer guide rather than into a separate read me?

@lukaszgo1 Perhaps eventually, but as this is so new, I would like to keep it contained to just UIAHandler, and keep the _remoteOps package private, until it has been road tested a bit internally by NVDA. it can definitely provide extraordinary speedups if used appropriately, but may actually be much slower for very small scenarios with only 2 or 3 cross-process calls - more real-world testing is required.

michaelDCurran avatar Feb 25 '24 21:02 michaelDCurran

Wow, haven't looked at the code yet but this is fantastic!

Another good test candidate for this might be #13517.

codeofdusk avatar Feb 26 '24 00:02 codeofdusk

I did some testing in a document that was notoriously slow. I navigated from one h1 to the next h1. They are about four pages apart.

  • Without this PR (release build 2023.3.3): 592 ms
  • With this PR (running from source): 157 ms

Interestingly, the elements list (headings selected) did not show significant speed improvements. In any case, this should greatly speed up quicknav. I think it could also be very effective for navigating hyperlinks in Word.

dkager avatar Mar 04 '24 19:03 dkager

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • FAIL: Lint check. See test results for more information.
  • PASS: System tests (tags: installer NVDA).
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/6j7vb3297bt84aqa/artifacts/output/nvda_snapshot_pr16214-31905,0da09743.exe
  • CI timing (mins): INIT 0.0, INSTALL_START 1.0, INSTALL_END 0.9, BUILD_START 0.0, BUILD_END 11.7, TESTSETUP_START 0.0, TESTSETUP_END 0.4, TEST_START 0.0, TEST_END 2.3, FINISH_END 0.2

See test results for failed build of commit 0da09743cc

AppVeyorBot avatar May 13 '24 23:05 AppVeyorBot

@coderabbitai review

Testing code rabbit on this large PR

seanbudd avatar May 14 '24 23:05 seanbudd

@seanbudd, I have reviewed the PR and gathered the following information:

Summary of Changes

  1. New _remoteOps Package: This package interfaces with the low-level Windows Remote Operations API, providing higher-level APIs and types to aid in building and executing remote algorithms.
  2. Changes in UIAHandler.remote: Functions like msWord_getCustomAttributeValue have been rewritten to use the new NVDA remote operations library. Additional functions such as findFirstHeadingInTextRange and collectAllHeadingsInTextRange have been added.
  3. Unit Tests: Approximately 40 unit tests have been added for the NVDA remote operations library, located in tests/unit/test_UIARemoteOps/test_highLevel.py.

CI Analysis

  • Lint Check: The lint check failed due to unused imports and trailing whitespace in several test files.
  • System Tests: All system tests passed except for one linting error.

Linting Errors

Here are the specific linting errors:

  • Unused imports in multiple test files (e.g., unittest.mock.Mock, ctypes.POINTER, UIAHandler.UIA, UIAHandler._remoteOps.lowLevel.PropertyId).
  • Trailing whitespace in test_numeric.py.

Next Steps

To resolve the linting errors, you can remove the unused imports and trailing whitespace. Here are the specific changes needed:

  1. Remove Unused Imports:

    • tests/unit/test_UIARemoteOps/test_highLevel/test_bool.py
    • tests/unit/test_UIARemoteOps/test_highLevel/test_errorHandling.py
    • tests/unit/test_UIARemoteOps/test_highLevel/test_if.py
    • tests/unit/test_UIARemoteOps/test_highLevel/test_instructionLimit.py
    • tests/unit/test_UIARemoteOps/test_highLevel/test_iterable.py
    • tests/unit/test_UIARemoteOps/test_highLevel/test_numeric.py
    • tests/unit/test_UIARemoteOps/test_highLevel/test_string.py
  2. Remove Trailing Whitespace:

    • tests/unit/test_UIARemoteOps/test_highLevel/test_numeric.py

Would you like me to provide the specific code changes to address these linting issues?

Analysis chain
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Gather information about the new `_remoteOps` package implementation.
fd '_remoteOps' --exec cat

# Gather information about the changes in `UIAHandler.remote`.
fd 'UIAHandler/remote' --exec cat

# Gather information about the unit tests added in `tests/unit/test_UIARemoteOps/test_highLevel.py`.
fd 'test_highLevel.py' --exec cat

# Analyze the CI failures, especially the lint and system tests.
curl -s https://ci.appveyor.com/api/buildjobs/6j7vb3297bt84aqa/artifacts/output/nvda_snapshot_pr16214-31905,0da09743.exe | jq '.'

Length of output: 276



Script:

#!/bin/bash
# Gather information about the new `_remoteOps` package implementation.
fd '_remoteOps' -e py --exec cat

# Gather information about the changes in `UIAHandler.remote`.
fd 'UIAHandler/remote' -e py --exec cat

# Gather information about the unit tests added in `tests/unit/test_UIARemoteOps/test_highLevel.py`.
fd 'test_highLevel.py' --exec cat

# Analyze the CI failures, especially the lint and system tests.
curl -s https://ci.appveyor.com/api/buildjobs/6j7vb3297bt84aqa/tests | jq '.'

Length of output: 250888

Actions Performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot] avatar May 14 '24 23:05 coderabbitai[bot]

Walkthrough

The recent changes involve the removal of the microsoft-ui-uiautomation submodule and its references, updates to nvdaHelper and UIAHandler, and the introduction of new functions for remote operation handling in UI Automation. These changes enhance functionality and robustness across the project components.

Changes

File Path Change Summary
.gitmodules, include/readme.md,
nvdaHelper/readme.md,
projectDocs/dev/createDevEnvironment.md
Removed references to microsoft-ui-uiautomation submodule and dependencies.
nvdaHelper/UIARemote/lowLevel.cpp,
nvdaHelper/UIARemote/sconscript,
nvdaHelper/archBuild_sconscript
Introduced and adjusted functions for remote operation handling, updated build scripts post submodule removal.
source/UIAHandler/_remoteOps/...,
tests/unit/test_UIARemoteOps/test_highLevel/...
Added modules/functions for remote operation management, instruction handling, and extensive testing.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to full the review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar May 14 '24 23:05 coderabbitai[bot]

@coderabbitai resolve

seanbudd avatar May 15 '24 00:05 seanbudd

Actions Performed

Comments resolved.

coderabbitai[bot] avatar May 15 '24 00:05 coderabbitai[bot]

@coderabbitai review

michaelDCurran avatar May 15 '24 02:05 michaelDCurran

Actions Performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot] avatar May 15 '24 02:05 coderabbitai[bot]

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • FAIL: Lint check. See test results for more information.
  • PASS: System tests (tags: installer NVDA).
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/6qxsrgktem20i7at/artifacts/output/nvda_snapshot_pr16214-31935,b043a073.exe
  • CI timing (mins): INIT 0.0, INSTALL_START 1.2, INSTALL_END 1.0, BUILD_START 0.0, BUILD_END 12.9, TESTSETUP_START 0.0, TESTSETUP_END 0.4, TEST_START 0.0, TEST_END 2.5, FINISH_END 0.2

See test results for failed build of commit b043a073f0

AppVeyorBot avatar May 15 '24 03:05 AppVeyorBot

Note this might also close out https://github.com/nvaccess/nvda/issues/13995

seanbudd avatar May 16 '24 04:05 seanbudd

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • FAIL: Lint check. See test results for more information.
  • PASS: System tests (tags: installer NVDA).
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/j9i8ylomxmovbh7d/artifacts/output/nvda_snapshot_pr16214-31950,fdf095ce.exe
  • CI timing (mins): INIT 0.0, INSTALL_START 1.1, INSTALL_END 1.0, BUILD_START 0.0, BUILD_END 13.0, TESTSETUP_START 0.0, TESTSETUP_END 0.4, TEST_START 0.0, TEST_END 2.5, FINISH_END 0.3

See test results for failed build of commit fdf095ced0

AppVeyorBot avatar May 16 '24 07:05 AppVeyorBot

  • PASS: Translation comments check.
  • FAIL: Unit tests. See test results for more information.
  • PASS: Lint check.
  • FAIL: System tests (tags: installer NVDA). See test results for more information.
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/wxw3kv21wu1342y2/artifacts/output/nvda_snapshot_pr16214-31983,b4d0291b.exe
  • CI timing (mins): INIT 0.0, INSTALL_START 1.1, INSTALL_END 0.9, BUILD_START 0.0, BUILD_END 12.1, TESTSETUP_START 0.0, TESTSETUP_END 0.3, TEST_START 0.0, TEST_END 0.7, FINISH_END 0.2

See test results for failed build of commit b4d0291b21

AppVeyorBot avatar May 19 '24 22:05 AppVeyorBot

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • FAIL: Lint check. See test results for more information.
  • PASS: System tests (tags: installer NVDA).
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/wohf883kds04o7w8/artifacts/output/nvda_snapshot_pr16214-31985,de983ba9.exe
  • CI timing (mins): INIT 0.0, INSTALL_START 1.1, INSTALL_END 0.9, BUILD_START 0.0, BUILD_END 12.7, TESTSETUP_START 0.0, TESTSETUP_END 0.4, TEST_START 0.0, TEST_END 2.3, FINISH_END 0.2

See test results for failed build of commit de983ba96c

AppVeyorBot avatar May 19 '24 22:05 AppVeyorBot

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • FAIL: Lint check. See test results for more information.
  • PASS: System tests (tags: installer NVDA).
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/h79qdwfsdcclgeyq/artifacts/output/nvda_snapshot_pr16214-31999,a914c036.exe
  • CI timing (mins): INIT 0.0, INSTALL_START 1.1, INSTALL_END 0.9, BUILD_START 0.0, BUILD_END 11.9, TESTSETUP_START 0.0, TESTSETUP_END 0.4, TEST_START 0.0, TEST_END 2.4, FINISH_END 0.2

See test results for failed build of commit a914c036ec

AppVeyorBot avatar May 20 '24 02:05 AppVeyorBot

@coderabbitai review

michaelDCurran avatar May 20 '24 09:05 michaelDCurran

Actions Performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot] avatar May 20 '24 09:05 coderabbitai[bot]

@coderabbitai resolve

michaelDCurran avatar May 20 '24 23:05 michaelDCurran

Actions Performed

Comments resolved.

coderabbitai[bot] avatar May 20 '24 23:05 coderabbitai[bot]