Skip to content

Keep degenerate UIA text ranges degenerate after movement#7530

Merged
7 commits merged intomicrosoft:masterfrom
codeofdusk:dev/codeofusk/i7342
Sep 4, 2020
Merged

Keep degenerate UIA text ranges degenerate after movement#7530
7 commits merged intomicrosoft:masterfrom
codeofdusk:dev/codeofusk/i7342

Conversation

@codeofdusk
Copy link
Contributor

@codeofdusk codeofdusk commented Sep 4, 2020

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 and nvaccess/nvda#11288

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 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. labels Sep 4, 2020
Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

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

This test is failing:

Not too bad though. Just change this line to:

{4, 0+1}

Bonus points if you add a similar test to CanMoveByLine tests here:

@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Sep 4, 2020
@carlos-zamora
Copy link
Member

Forgot to say, other than that, it looks good! Thanks!

@ghost ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Sep 4, 2020
Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

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

We're close 😊

Also, we have a code formatter. To run it, do this in your terminal:

# in pwsh, go to the repo's directory
Import-Module .\tools\OpenConsole.psm1
Invoke-CodeFormat

@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Sep 4, 2020
@ghost ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Sep 4, 2020
@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Sep 4, 2020
@ghost ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Sep 4, 2020
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

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

This is excellent, and a very good catch. Thank you 😄L

@DHowett DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Sep 4, 2020
@ghost
Copy link

ghost commented Sep 4, 2020

Hello @DHowett!

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.

@DHowett
Copy link
Member

DHowett commented Sep 4, 2020

@carlos-zamora double-tap plz

@ghost ghost merged commit 7a03f75 into microsoft:master Sep 4, 2020
@codeofdusk
Copy link
Contributor Author

Any rough timeline for when this will make it to insider inbox?

@DHowett
Copy link
Member

DHowett commented Sep 16, 2020

This is merging to Windows today; it will be a couple weeks before it goes out to Insiders. 😄 Thanks again for the contribution.

I'll let you know when it's out.

DHowett pushed a commit that referenced this pull request Sep 18, 2020
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

🎉Windows Terminal v1.3.2651.0 has been released which incorporates this pull request.:tada:

Handy links:

@ghost
Copy link

ghost commented Sep 22, 2020

🎉Windows Terminal Preview v1.4.2652.0 has been released which incorporates this pull request.:tada:

Handy links:

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-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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UIA: degenerate text ranges not degenerate after move

3 participants