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.

Sign up for GitHub

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

Jump to bottom

All forms of UIA navigation appear to skip entries when going back and are unable to go forwards #3976

Closed
ZoeyR opened this issue Dec 16, 2019 · 5 comments · Fixed by #4018
Closed

All forms of UIA navigation appear to skip entries when going back and are unable to go forwards #3976

ZoeyR opened this issue Dec 16, 2019 · 5 comments · Fixed by #4018
Assignees
@carlos-zamora
Labels
Area-Accessibility Issues related to accessibility Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal v0.9

Comments

@ZoeyR
Copy link
Contributor

ZoeyR commented Dec 16, 2019

Steps to reproduce

  • Hook up the WPF control from the dev/zorio/wpf-uia-tree branch into a simple WPF application.
  • Print text to the screen (e.g. "foo bar baz\r\nfoo2 bar2 baz2")
  • Attempt to navigate said text with NVDA

Expected behavior

Word, character and line navigation should work as expected.

Actual behavior

All navigation forms appear to skip entries moving backwards and get stuck moving forwards, for example, I have observed the following:

  • Reading the line "foo bar baz" from right to left as word units ends up reading out "baz, b, foo"
  • Attempting to read the same line left to right reads out "foo" continuously.
  • Reading the characters in that line will exhibit similar behaviour, unable to navigate rightwards and skipping entries when navigating leftwards.
@ZoeyR ZoeyR added the Area-Accessibility Issues related to accessibility label Dec 16, 2019
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Dec 16, 2019
@carlos-zamora carlos-zamora added Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. and removed Needs-Tag-Fix Doesn't match tag requirements labels Dec 16, 2019
@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Dec 16, 2019
@DHowett-MSFT DHowett-MSFT added this to the Terminal-1912 milestone Dec 16, 2019
@carlos-zamora carlos-zamora self-assigned this Dec 17, 2019
@carlos-zamora
Copy link
Member

@ZoeyR I'm having trouble navigating with NVDA (mostly because I'm not too familiar with how to use it well). I've tested this using Narrator and Inspect.exe and the text ranges seem to be accessible. Could you provide some additional guidance on how to navigate to the terminal's text field using NVDA?

I think I could probably figure out how to navigate within the text field using this. I'm just having trouble getting to it (for now).

@ZoeyR
Copy link
Contributor Author

ZoeyR commented Dec 17, 2019

That reference doesn't have the full list of NVDA shortcut keys, what you want to do is use the desktop keyboard configuration, make sure that "caps lock" is set in preferences->keyboard->modifier keys. And then press Caps lock + numpad 5 to focus the current element.

@ZoeyR
Copy link
Contributor Author

ZoeyR commented Dec 17, 2019

NVDA should also be the primary testing method, only 1% of computer users use narrator as their primary screen reader: https://webaim.org/projects/screenreadersurvey8/ I can help more if you're still having issues navigating. Also, you can't use the UWP application since the UIA tree has not yet been hooked up in that.

@carlos-zamora
Copy link
Member

NVDA should also be the primary testing method, only 1% of computer users use narrator as their primary screen reader: https://webaim.org/projects/screenreadersurvey8/ I can help more if you're still having issues navigating. Also, you can't use the UWP application since the UIA tree has not yet been hooked up in that.

I'll definitely use it more then. 😊 Just getting caught up in the learning curve haha.

@carlos-zamora carlos-zamora mentioned this issue Dec 19, 2019
Merged
3 tasks
@ghost ghost added the In-PR This issue has a related PR label Dec 21, 2019
@ghost ghost closed this as completed in #4018 Jan 31, 2020
ghost pushed a commit that referenced this issue Jan 31, 2020
@carlos-zamora
Refactor UiaTextRange For Improved Navigation and Reliability (#4018)
29df540 
## Summary of the Pull Request
This pull request is intended to achieve the following goals...
1) reduce duplicate code
2) remove static functions
3) improve readability
4) improve reliability
5) improve code-coverage for testing
6) establish functioning text buffer navigation in Narrator and NVDA

This also required a change to the wrapper class `XamlUiaTextRange` that has been causing issues with Narrator and NVDA.

See below for additional context.

## References
#3976 - I believe this might have been a result of improperly handling degenerate ranges. Fixed here.
#3895 - reduced the duplicate code. No need to separate into different files
#2160 - same as #3976 above
#1993 - I think just about everything is no longer static

## PR Checklist
* [x] Closes #3895, Closes #1993, Closes #3976, Closes #2160 
* [x] CLA signed
* [x] Tests added/passed

## Detailed Description of the Pull Request / Additional comments

### UiaTextRange
- converted endpoints into the COORD system in the TextBuffer coordinate space
- `start` is inclusive, `end` is exclusive. A degenerate range is when start == end.
- all functions are no longer static
- `MoveByUnit()` functions now rely on `MoveEndpointByUnit()` functions
- removed unnecessary typedefs like `Endpoint`, `ScreenInfoRow`, etc..
- relied more heavily on existing functionality from `TextBuffer` and `Viewport`

### XamlUiaTextRange
- `GetAttributeValue()` must return a special HRESULT that signifies that the requested attribute is not supported. This was the cause of a number of inconsistencies between Narrator and NVDA.
- `FindText()` should return `nullptr` if nothing was found. #4373 properly fixes this functionality now that Search is a shared module

### TextBuffer
- Word navigation functionality is entirely in `TextBuffer` for proper abstraction
- a total of 6 functions are now dedicated to word navigation to get a good understanding of the differences between a "word" in Accessibility and a "word" in selection

As an example, consider a buffer with this text in it:
"  word   other  "
In selection, a "word" is defined as the range between two delimiters, so the words in the example include ["  ", "word", "   ", "other", "  "].
In accessibility , a "word" includes the delimiters after a range of readable characters, so the words in the example include ["word   ", "other  "].

Additionally, accessibility word navigation must be able to detect if it is on the first or last word. This resulted in a slight variant of word navigation functions that return a boolean instead of a COORD.

Ideally, these functions can be consolidated, but that is too risky for a PR of this size as it can have an effect on selection.

### Viewport
- the concept of `EndExclusive` is added. This is used by UiaTextRange's `end` anchor as it is exclusive. To signify that the last character in the buffer is included in this buffer, `end` must be one past the end of the buffer. This is `EndExclusive`
- Since many functions check if the given `COORD` is in bounds, a flag must be set to allow `EndExclusive` as a valid `COORD` that is in bounds.

### Testing
- word navigation testing relies more heavily on TextBuffer tests
- additional testing was created for non-movement focused functions of UiaTextRange
- The results have been compared to Microsoft Word and some have been verified by UiAutomation/Narrator contacts as expected results.

## Validation Steps Performed
Tests pass
Narrator works
NVDA works
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Jan 31, 2020
@carlos-zamora carlos-zamora mentioned this issue Feb 11, 2020
Closed
22 tasks
@ghost
Copy link

ghost commented Feb 13, 2020

🎉This issue was addressed in #4018, which has now been successfully released as Windows Terminal Preview v0.9.433.0.:tada:

Handy links:

@codeofdusk codeofdusk mentioned this issue Apr 5, 2020
Closed
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@carlos-zamora carlos-zamora

Labels
Area-Accessibility Issues related to accessibility Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Terminal v0.9
Development

Successfully merging a pull request may close this issue.

Refactor UiaTextRange For Improved Navigation and Reliability microsoft/terminal
4 participants
@ZoeyR @carlos-zamora @DHowett-MSFT @cinnamon-msft