Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UIA: Fix GetVisibleRanges() and add Tracing #4495

Merged
11 commits merged into from
Feb 20, 2020
Merged

Conversation

carlos-zamora
Copy link
Member

@carlos-zamora carlos-zamora commented Feb 6, 2020

Summary of the Pull Request

Debugging our custom UIA providers has been a painful experience because outputting content to VS may result in UIA Clients getting impatient and giving up on extracting data.

Adding tracing allows us to debug these providers without getting in the way of reproducing a bug. This will help immensely with developing accessibility features on Windows Terminal and Console.

This pull request additionally contains payload from #4526:

  • Make GetVisibleRanges() return one range (and add tracing for it).
    ScreenInfoUiaProvider::GetVisibleRanges() used to return one range per line of visible text. The documentation for this function says that we should return one per contiguous span of text. Since all of the text in the TermControl will always be contiguous (at least by our standards), we should only ever be returning one range.

PR Checklist

Detailed Description of the Pull Request / Additional comments

UiaTracing is a singleton class that is in charge of registration for trace logging. TextRange is used to trace UiaTextRange, whereas TextProvider is used to trace ScreenInfoUiaProviderBase.

_getValue() is overloaded to transform complex objects and enums into a string for logging.

_getTextValue() had to be added to be able to trace the text a UiaTextRange included. This makes following UiaTextRanges much simpler.

Validation Steps Performed

Performed a few operations when under NVDA/Narrator and manually checked the results.

@carlos-zamora carlos-zamora added Area-Accessibility Issues related to accessibility Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal. labels Feb 6, 2020
@carlos-zamora carlos-zamora added this to the Terminal v0.10 milestone Feb 7, 2020
DHowett-MSFT
DHowett-MSFT previously approved these changes Feb 7, 2020
Copy link
Contributor

@DHowett-MSFT DHowett-MSFT 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 totally understand: it makes the tracing compile, but it doesn't actually enable many of the commented-out traces?

src/types/UiaTextRangeBase.cpp Show resolved Hide resolved
src/types/UiaTextRangeBase.cpp Outdated Show resolved Hide resolved
src/types/UiaTracing.cpp Outdated Show resolved Hide resolved
@DHowett-MSFT
Copy link
Contributor

sorry, i was just looking at one commit. re-reviewing.

Copy link
Contributor

@DHowett-MSFT DHowett-MSFT left a comment

Choose a reason for hiding this comment

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

okay, minor bits.

src/types/ScreenInfoUiaProviderBase.cpp Show resolved Hide resolved
src/types/UiaTextRangeBase.cpp Outdated Show resolved Hide resolved
src/types/UiaTextRangeBase.cpp Outdated Show resolved Hide resolved
@ghost ghost added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Feb 7, 2020
carlos-zamora and others added 3 commits February 7, 2020 17:53
…4526)

## Summary of the Pull Request
`ScreenInfoUiaProvider::GetVisibleRanges()` used to return one range per line of visible text. The documentation for this function says that we should return one per contiguous span of text. Since all of the text in the TermControl will always be contiguous (at least by our standards), we should only ever be returning one range.

## PR Checklist
* [X] Closes #4507 
* [X] CLA signed.

## Validation Steps Performed
Verified using Accessibility Insights.
@DHowett-MSFT
Copy link
Contributor

oop this one got another PR merged into it

@carlos-zamora carlos-zamora changed the title Add Tracing for UiaTextRange and ScreenInfoUiaProvider UIA: Fix GetVisibleRanges() and add Tracing Feb 11, 2020
src/types/UiaTracing.cpp Outdated Show resolved Hide resolved
@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Feb 11, 2020
@ghost ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Feb 13, 2020
@DHowett-MSFT
Copy link
Contributor

I've updated the PR description to include the GetVisibleRanges change.

src/types/UiaTextRangeBase.hpp Show resolved Hide resolved
src/types/UiaTracing.h Show resolved Hide resolved
@carlos-zamora carlos-zamora added the AutoMerge Marked for automatic merge by the bot when requirements are met label Feb 20, 2020
@ghost
Copy link

ghost commented Feb 20, 2020

Hello @carlos-zamora!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 360c655 into master Feb 20, 2020
@ghost ghost deleted the dev/cazamor/uia-tracing branch February 20, 2020 23:50
codeofdusk added a commit to codeofdusk/nvda that referenced this pull request Feb 23, 2020
codeofdusk added a commit to codeofdusk/nvda that referenced this pull request Apr 1, 2020
michaelDCurran pushed a commit to nvaccess/nvda that referenced this pull request Apr 6, 2020
* Bring over ConsoleUIATextInfo refactoring from #10716.

* Add WinTerminalUIA

* Style.

* Review actions.

* microsoft/terminal#4495: support fixed GetVisibleRanges.

* KeyboardHandlerBasedTypedCharSupport -> EnhancedTermTypedCharSupport, add _shouldUseToUnicodeEx flag and set False for WinTerminalUIA.

* Update doc comments.

* Update copyrights.

* Review actions.

* Factor out bounding into separate method.

* Re-introduce KeyboardHandlerBasedTypedCharSupport.

* Fix UIA console support for newer Windows Console builds.

* Revert copyright header changes.

* Revert another no longer necessary copyright header change.

* Fix comments.

* Update docstring.

* Review actions.

* Update source/NVDAObjects/behaviors.py

Co-Authored-By: Leonard de Ruijter <[email protected]>

* Add class docstring.

* Optimization: clear character buffers when reporting new lines.

This saves us from scanning the old and new lines twice in _calculateNewText (once for diffing and once for finding nonblank indices), and fixes typed word reporting for 21H1 console.

This new approach does not resolve #10942.

* Update inaccurate information in user guide.

Co-authored-by: Leonard de Ruijter <[email protected]>
@codeofdusk codeofdusk mentioned this pull request Nov 6, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Accessibility Issues related to accessibility AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ScreenInfoUiaProvider::GetVisibleRanges() returns an improper result Task: Re-attach Tracing to UIA Tree
3 participants