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

Accessibility: Wide Glyph Detection #1354

Closed
1 task
cinnamon-msft opened this issue Jun 20, 2019 · 1 comment · Fixed by #4946
Closed
1 task

Accessibility: Wide Glyph Detection #1354

cinnamon-msft opened this issue Jun 20, 2019 · 1 comment · Fixed by #4946
Labels
Area-Accessibility Issues related to accessibility Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. 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
Console Backlog

Comments

@cinnamon-msft
Copy link
Contributor

cinnamon-msft commented Jun 20, 2019 •
edited by carlos-zamora
Loading

Summary of the new feature/enhancement

Currently, wide glyphs can be read and accessed properly by a narrator. However, when expanding the text range unit-by-unit, the text range only sees half of the wide glyph, and fails to understand it.

Proposed technical implementation details (optional)

  • Modify existing fundamental accessibility model to detect wide glyphs and read them properly when encountered
@cinnamon-msft cinnamon-msft added Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Area-Accessibility Issues related to accessibility labels Jun 20, 2019
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Jun 20, 2019
@carlos-zamora carlos-zamora added Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal. and removed Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Jun 20, 2019
@carlos-zamora carlos-zamora self-assigned this Jun 20, 2019
This was referenced Feb 11, 2020
Closed
Merged
Merged
DHowett-MSFT pushed a commit that referenced this issue Feb 28, 2020
@carlos-zamora
Move rect expansion to textbuffer; refactor selection code (#4560)
0e672fa 
- When performing chunk selection, the expansion now occurs at the time
  of the selection, not the rendering of the selection
- `GetSelectionRects()` was moved to the `TextBuffer` and is now shared
  between ConHost and Windows Terminal
- Some of the selection variables were renamed for clarity
- Selection COORDs are now in the Text Buffer coordinate space
- Fixes an issue with Shift+Click after performing a Multi-Click
  Selection

## References
This also contributes to...
- #4509: UIA Box Selection
- #2447: UIA Signaling for Selection
- #1354: UIA support for Wide Glyphs

Now that the expansion occurs at before render-time, the selection
anchors are an accurate representation of what is selected. We just need
to move `GetText` to the `TextBuffer`. Then we can have those three
issues just rely on code from the text buffer. This also means ConHost
gets some of this stuff for free 😀

### TextBuffer
- `GetTextRects` is the abstracted form of `GetSelectionRects`
- `_ExpandTextRow` is still needed to handle wide glyphs properly

### Terminal
- Rename...
    - `_boxSelection` --> `_blockSelection` for consistency with ConHost
    - `_selectionAnchor` --> `_selectionStart` for consistency with UIA
    - `_endSelectionPosition` --> `_selectionEnd` for consistency with
      UIA
- Selection anchors are in Text Buffer coordinates now
- Really rely on `SetSelectionEnd` to accomplish appropriate chunk
  selection and shift+click actions

## Validation Steps Performed
- Shift+Click
- Multi-Click --> Shift+Click
- Chunk Selection at...
    - top of buffer
    - bottom of buffer
    - random region in scrollback

Closes #4465
Closes #4547
carlos-zamora added a commit that referenced this issue Mar 9, 2020
@carlos-zamora
Abstract GetTextForClipboard() for UIA (#4578)
e79a421 
## Summary of the Pull Request
`GetTextForClipboard` already exists in the TextBuffer. It makes sense to use that for UIA as well. This changes the behavior or `GetText()` such that it does not remove leading/trailing whitespace anymore. That is more of an expected behavior.

## References
This also contributes to...
- #4509: UIA Box Selection
- #2447: UIA Signaling for Selection
- #1354: UIA support for Wide Glyphs
Now that the expansion occurs at before render-time, the selection anchors are an accurate representation of what is selected. We just need to move GetText to the TextBuffer. Then we can have those three issues just rely on code from the text buffer. This also means ConHost gets some of this stuff for free 😀

## Detailed Description of the Pull Request / Additional comments
- `TextBuffer::GetTextForClipboard()` --> `GetText()`
- `TextBuffer::GetText()` no longer requires GetForegroundColor/GetBackgroundColor. If either of these are not defined, we return a `TextAndColor` with only the `text` field populated.
- renamed a few parameters for copying text to the clipboard for clarity
- Updated `UiaTextRange::GetText()` to use `TextBuffer::GetText()`

## Validation Steps Performed
Manual tests for UIA using accessibility insights and Windows Terminal's copy action (w/ and w/out shift)

Added tests as well.
@carlos-zamora carlos-zamora mentioned this issue Mar 16, 2020
Merged
1 task
@ghost ghost added the In-PR This issue has a related PR label Mar 16, 2020
@ghost ghost closed this as completed in #4946 Mar 23, 2020
ghost pushed a commit that referenced this issue Mar 23, 2020
@carlos-zamora
Improve wide glyph support in UIA (#4946)
a338227 
## Summary of the Pull Request
- Added better wide glyph support for UIA. We used to move one _cell_ at a time, so wide glyphs would be read twice.
- Converted a few things to use til::point since I'm already here.
- fixed telemetry for UIA

## PR Checklist
* [x] Closes #1354

## Detailed Description of the Pull Request / Additional comments
The text buffer has a concept of word boundaries, so it makes sense to have a concept of glyph boundaries too.

_start and _end in UiaTextRange are now til::point

## Validation Steps Performed
Verified using Narrator
@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 Mar 23, 2020
DHowett-MSFT pushed a commit that referenced this issue Apr 14, 2020
@carlos-zamora @DHowett
Improve wide glyph support in UIA (#4946)
c97f336 
## Summary of the Pull Request
- Added better wide glyph support for UIA. We used to move one _cell_ at a time, so wide glyphs would be read twice.
- Converted a few things to use til::point since I'm already here.
- fixed telemetry for UIA

## PR Checklist
* [x] Closes #1354

## Detailed Description of the Pull Request / Additional comments
The text buffer has a concept of word boundaries, so it makes sense to have a concept of glyph boundaries too.

_start and _end in UiaTextRange are now til::point

## Validation Steps Performed
Verified using Narrator
@ghost
Copy link

ghost commented Apr 22, 2020

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

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-Accessibility Issues related to accessibility Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. 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
Console Backlog
Development

Successfully merging a pull request may close this issue.

Improve wide glyph support in UIA microsoft/terminal
2 participants
@carlos-zamora @cinnamon-msft