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

ScreenInfoUiaProvider::GetSelection() returns an improper result #4452

Closed
carlos-zamora opened this issue Feb 3, 2020 · 2 comments · Fixed by #4466
Closed

ScreenInfoUiaProvider::GetSelection() returns an improper result #4452

carlos-zamora opened this issue Feb 3, 2020 · 2 comments · Fixed by #4466
Assignees
@carlos-zamora
Labels
Area-Accessibility Issues related to accessibility Issue-Task It's a feature request, but it doesn't really need a major design. Priority-1 A description (P1) Product-Conhost For issues in the Console codebase 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 v1.0

Comments

@carlos-zamora
Copy link
Member

Summary

Our TermControlUiaProvider (aka ScreenInfoUiaProvider) returns a list of UiaTextRange for our selection (one per row in the buffer).

The docs for GetSelection state we should only return one range.

Technical Details

For SupportedTextSelection, we state that we only support a single selection.

The documentation for GetSelection states as follows:

For UI Automation providers that support text selection, the provider should implement this method and also return a ITextProvider::SupportedTextSelection value.

If the control contains only a single span of selected text, the pRetVal array should contain a single text range.

If the control contains a text insertion point but no text is selected, the pRetVal array should contain a degenerate (empty) text range at the position of the text insertion point.
@carlos-zamora carlos-zamora added Product-Conhost For issues in the Console codebase Area-Accessibility Issues related to accessibility Product-Terminal The new Windows Terminal. Issue-Task It's a feature request, but it doesn't really need a major design. labels Feb 3, 2020
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Feb 3, 2020
@carlos-zamora
Copy link
Member Author

This impacts signaling for selection (#2447).

If a selection is created, we return the wrong range.

@carlos-zamora carlos-zamora mentioned this issue Feb 4, 2020
Merged
1 task
@ghost ghost added the In-PR This issue has a related PR label Feb 4, 2020
@carlos-zamora carlos-zamora added this to the Terminal v0.9 milestone Feb 4, 2020
@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 Feb 6, 2020
@DHowett-MSFT
Copy link
Contributor

-triage since this is in PR and we chatted about it offline

@carlos-zamora carlos-zamora mentioned this issue Feb 11, 2020
Closed
22 tasks
@ghost ghost closed this as completed in #4466 Feb 20, 2020
ghost pushed a commit that referenced this issue Feb 20, 2020
@carlos-zamora
Make ScreenInfoUiaProvider::GetSelection() Return One Selection (#4466)
d0c8221 
## Summary of the Pull Request
We used to return multiple text ranges to represent one selection. We only support one selection at a time, so we should only return one range.

Additionally, I moved all TriggerSelection() calls to the renderer from Terminal to TermControl for consistency. This ensures we only call it _once_ when we make a change to our selection state.

## References
#2447 - helps polish Signaling for Selection
#4465 - This is more apparent as the problem holding back Signaling for Selection

## PR Checklist
* [x] Closes #4452 

Tested using Accessibility Insights.
@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 Feb 20, 2020
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-Task It's a feature request, but it doesn't really need a major design. Priority-1 A description (P1) Product-Conhost For issues in the Console codebase 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 v1.0
Development

Successfully merging a pull request may close this issue.

Make ScreenInfoUiaProvider::GetSelection() Return One Selection microsoft/terminal
3 participants
@carlos-zamora @DHowett-MSFT @cinnamon-msft