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

narrator scan mode [ctrl+end] crashes conhost #7839

Closed
DHowett opened this issue Oct 6, 2020 · 5 comments · Fixed by #7868
Closed

narrator scan mode [ctrl+end] crashes conhost #7839

DHowett opened this issue Oct 6, 2020 · 5 comments · Fixed by #7868
Assignees
@carlos-zamora
Labels
Area-Accessibility Issues related to accessibility Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-0 Bugs that we consider release-blocking/recall-class (P0) Product-Conhost For issues in the Console codebase Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-Crash Crashes are real bad news.
Milestone
Terminal v1.5

Comments

@DHowett
Copy link
Member

DHowett commented Oct 6, 2020

ut supra
@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 Oct 6, 2020
@DHowett DHowett added Area-Accessibility Issues related to accessibility Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-0 Bugs that we consider release-blocking/recall-class (P0) Product-Conhost For issues in the Console codebase Severity-Crash Crashes are real bad news. 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 Oct 6, 2020
@DHowett
Copy link
Member Author

DHowett commented Oct 6, 2020 

09 000000ae`3f4fe7a0 00007ffc`6495acd0     conhost!Microsoft::Console::Types::UiaTextRangeBase::ScrollIntoView+0x2a1 [onecore\windows\core\console\open\src\types\uiatextrangebase.cpp @ 813] 


    FAIL_FAST_IF(!(newViewport.Top >= topRow));

0:009> dv newViewport
    newViewport = {LT(0, 8962) RB(119, 9001) In:[120 x 40] Ex:[119 x 39]}

top=8962

in Vb conhost, ctrl+end moves to the ... top of the buffer :|

@DHowett
Copy link
Member Author

DHowett commented Oct 6, 2020

Oh, it's bottom that's wrong here

@DHowett
Copy link
Member Author

DHowett commented Oct 6, 2020 

    [+0x048] _start           : {0,9000} [Type: _COORD]
    [+0x04c] _end             : {0,9001} [Type: _COORD]

@zadjii-msft zadjii-msft added this to the Terminal v1.5 milestone Oct 7, 2020
@carlos-zamora carlos-zamora mentioned this issue Oct 9, 2020
Merged
@ghost ghost added the In-PR This issue has a related PR label Oct 9, 2020
@ghost ghost closed this as completed in #7868 Oct 9, 2020
ghost pushed a commit that referenced this issue Oct 9, 2020
@carlos-zamora
Fix UIA ScrollIntoView at EndExclusive (#7868)
7a1932c 
`ScrollIntoView` is responsible for scrolling the viewport to include
the UTR's start endpoint. The crash was caused by `start` being at the
exclusive end, and attempting to scroll to it. This is now fixed by
clamping the result to the bottom of the buffer.

Most of the work here is to allow a test for this. `ScrollIntoView`
relied on a virtual `ChangeViewport` function. By making that
non-virtual, the `DummyElementProvider` in the tests can now be a
`ScreenInfoUiaProviderBase`. This opens up the possibility of more
UiaTextRange tests in the future too.

Closes #7839
@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 Oct 9, 2020
skyline75489 pushed a commit to skyline75489/terminal that referenced this issue Oct 10, 2020
@carlos-zamora @skyline75489
Fix UIA ScrollIntoView at EndExclusive (microsoft#7868)
80a72dc 
`ScrollIntoView` is responsible for scrolling the viewport to include
the UTR's start endpoint. The crash was caused by `start` being at the
exclusive end, and attempting to scroll to it. This is now fixed by
clamping the result to the bottom of the buffer.

Most of the work here is to allow a test for this. `ScrollIntoView`
relied on a virtual `ChangeViewport` function. By making that
non-virtual, the `DummyElementProvider` in the tests can now be a
`ScreenInfoUiaProviderBase`. This opens up the possibility of more
UiaTextRange tests in the future too.

Closes microsoft#7839
DHowett pushed a commit that referenced this issue Oct 19, 2020
@carlos-zamora @DHowett
Fix UIA ScrollIntoView at EndExclusive (#7868)
62d22c2 
`ScrollIntoView` is responsible for scrolling the viewport to include
the UTR's start endpoint. The crash was caused by `start` being at the
exclusive end, and attempting to scroll to it. This is now fixed by
clamping the result to the bottom of the buffer.

Most of the work here is to allow a test for this. `ScrollIntoView`
relied on a virtual `ChangeViewport` function. By making that
non-virtual, the `DummyElementProvider` in the tests can now be a
`ScreenInfoUiaProviderBase`. This opens up the possibility of more
UiaTextRange tests in the future too.

Closes #7839

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

ghost commented Nov 11, 2020

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

Handy links:

@ghost
Copy link

ghost commented Nov 11, 2020

🎉This issue was addressed in #7868, which has now been successfully released as Windows Terminal Preview v1.5.3142.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-0 Bugs that we consider release-blocking/recall-class (P0) Product-Conhost For issues in the Console codebase Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-Crash Crashes are real bad news.
Projects
None yet
Milestone
Terminal v1.5
Development

Successfully merging a pull request may close this issue.

Fix UIA ScrollIntoView at EndExclusive microsoft/terminal
3 participants
@DHowett @carlos-zamora @zadjii-msft