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

UIA: degenerate text ranges not degenerate after move #7342

Closed
codeofdusk opened this issue Aug 19, 2020 · 5 comments · Fixed by #7530
Closed

UIA: degenerate text ranges not degenerate after move #7342

codeofdusk opened this issue Aug 19, 2020 · 5 comments · Fixed by #7530
Assignees
@carlos-zamora
Labels
Area-Accessibility Issues related to accessibility Issue-Bug It either shouldn't be doing this or needs an investigation. 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.4

Comments

@codeofdusk
Copy link
Contributor

Environment

Windows build number: [run `[Environment]::OSVersion` for powershell, or `ver` for cmd] Latest master of OpenConsole
Windows Terminal version (if applicable):

Any other software? NVDA

Steps to reproduce

>>> # In the NVDA Python console (NVDA+control+z)
>>> # In a Microsoft Word document (using UIA for access, enable in advanced settings), caret positioned on the first "C" in "Cactus".
>>> start = nav.makeTextInfo("caret")
>>> end = start.copy()
>>> end.move("character", 2) # move to the second "c".
2
>>> marker=start.copy()
>>> marker.setEndPoint(start, "startToStart") # Calls MoveEndpointByRange for UIA controls.
>>> marker.setEndPoint(end, "endToEnd")
>>> marker.text
'Ca' # GOOD

>>> # Now, in conhost with UIA enabled
>>> start = nav.makeTextInfo("caret")
>>> end = start.copy()
>>> end.move("character", 2)
2
>>> marker=start.copy()
>>> marker.setEndPoint(start, "startToStart")
>>> marker.setEndPoint(end, "endToEnd")
>>> marker.text
'Cac' # BAD

Expected behaviour

Conhost matches Microsoft Word when calling MoveEndpointByRange.

Actual behaviour

See above.
@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 Aug 19, 2020
@codeofdusk
Copy link
Contributor Author

Cc @carlos-zamora.

@codeofdusk codeofdusk mentioned this issue Aug 19, 2020
Merged
@carlos-zamora carlos-zamora added Area-Accessibility Issues related to accessibility Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal. and removed Needs-Tag-Fix Doesn't match tag requirements labels Aug 19, 2020
@zadjii-msft zadjii-msft added this to the Terminal v2.0 milestone Aug 19, 2020
@zadjii-msft zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Aug 19, 2020
@codeofdusk
Copy link
Contributor Author

Could this please be reconsidered for 21H1? I'd really like to start using UIA console by default. Thanks.

@zadjii-msft zadjii-msft added the Priority-1 A description (P1) label Aug 20, 2020
@codeofdusk codeofdusk mentioned this issue Aug 24, 2020
Closed
@codeofdusk
Copy link
Contributor Author

The issue seems not to be with MoveEndpointByRange after all, but rather with move. It calls ExpandToEnclosingUnit to determine _end, but I don't think that's quite right. See, for example:

>>> # In word
>>> ti=nav.makeTextInfo("caret")
>>> ti.compareEndPoints(ti, "startToEnd")
0
>>> ti.move("character", 2)
2
>>> ti.compareEndPoints(ti, "startToEnd")
0
>>> # Now, in conhost
>>> ti=nav.makeTextInfo("caret")
>>> ti.compareEndPoints(ti, "startToEnd")
0
>>> ti.move("character", 2)
2
>>> ti.compareEndPoints(ti, "startToEnd")
-1

Based on my testing in Word, the key seems to be: if a degenerate range is moved, it should remain degenerate after the move.

@codeofdusk codeofdusk changed the title UIA: MoveEndpointByRange does not move to the exclusive end of the target range UIA: degenerate text ranges not degenerate after move Sep 3, 2020
@codeofdusk codeofdusk mentioned this issue Sep 4, 2020
Merged
@ghost ghost added the In-PR This issue has a related PR label Sep 4, 2020
@ghost ghost closed this as completed in #7530 Sep 4, 2020
ghost pushed a commit that referenced this issue Sep 4, 2020
@codeofdusk
Keep degenerate UIA text ranges degenerate after movement (#7530)
7a03f75 
Conhost expands UIA text ranges when moved. This means that degenerate
ranges become non-degenerate after movement, leading to odd behaviour
from UIA clients. This PR doesn't expand degenerate ranges, but rather
keeps them degenerate by moving `_end` to the newly-changed `_start`.

Tested in the NVDA Python console (cases with `setEndPoint` and
`compareEndPoints` described in #7342). Also ran the logic by
@michaelDCurran.

Closes #7342

Almost definitely addresses nvaccess/nvda#11288 (although I'll need to
test with my Braille display). Also fixes an issue privately reported to
me by @Simon818 with copy/paste from review cursor which originally lead
me to believe the issue was with `moveEndPointByRange`.
@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 Sep 4, 2020
DHowett pushed a commit that referenced this issue Sep 18, 2020
@codeofdusk @DHowett
Keep degenerate UIA text ranges degenerate after movement (#7530)
420d714 
Conhost expands UIA text ranges when moved. This means that degenerate
ranges become non-degenerate after movement, leading to odd behaviour
from UIA clients. This PR doesn't expand degenerate ranges, but rather
keeps them degenerate by moving `_end` to the newly-changed `_start`.

Tested in the NVDA Python console (cases with `setEndPoint` and
`compareEndPoints` described in #7342). Also ran the logic by
@michaelDCurran.

Closes #7342

Almost definitely addresses nvaccess/nvda#11288 (although I'll need to
test with my Braille display). Also fixes an issue privately reported to
me by @Simon818 with copy/paste from review cursor which originally lead
me to believe the issue was with `moveEndPointByRange`.

(cherry picked from commit 7a03f75)
@ghost
Copy link

ghost commented Sep 22, 2020

🎉This issue was addressed in #7530, which has now been successfully released as Windows Terminal v1.3.2651.0.:tada:

Handy links:

@ghost
Copy link

ghost commented Sep 22, 2020

🎉This issue was addressed in #7530, which has now been successfully released as Windows Terminal Preview v1.4.2652.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

@carlos-zamora carlos-zamora

Labels
Area-Accessibility Issues related to accessibility Issue-Bug It either shouldn't be doing this or needs an investigation. 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.4
Development

Successfully merging a pull request may close this issue.

Keep degenerate UIA text ranges degenerate after movement codeofdusk/terminal
4 participants
@codeofdusk @carlos-zamora @zadjii-msft @cinnamon-msft