maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Don't allow a linefeed to scroll when outside DECSTBM margins (#3704)

Browse source 
## Summary of the Pull Request

When the `DECSTBM` margins are set, scrolling should only allowed within
those margins. So if the cursor is below the bottom margin, it should
just be clamped when it tries to move down from the bottom line of the
viewport, instead of scrolling the screen up. This PR implements that
restriction, i.e. it prevents scrolling taking place outside the
`DECSTBM` margins, which was previously allowed.

## PR Checklist
* [x] Closes #2657
* [x] CLA signed.
* [x] Tests added/passed

## Detailed Description of the Pull Request / Additional comments

This simply adds a condition in the `AdjustCursorPosition` function to
check if the cursor is moving below the bottom of the viewport when the
margins are set. If so, it clamps the y coordinate to the bottom line of
the viewport.

The only time it's acceptable for scrolling to happen when margins are
set, is if the scrolling is taking place within those margins. But those
cases would already have been handled earlier in the function (in the
`fScrollDown` or `scrollDownAtTop` conditions), and thus the y
coordinate would have already been prevented from moving out of the
viewport.

## Validation Steps Performed

I've added some screen buffer tests to confirm this new behaviour, and
I've also checked the test case from the initial bug report (#2657) and
made sure it now matches the results in XTerm.
This commit is contained in:
James Holderness 2019-12-11 22:59:27 +00:00 committed by msftbot[bot]
parent 1e2f203395
commit 6f21f30951
2 changed files with 41 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
src/host/_stream.cpp
View file

@ -203,6 +203,15 @@ constexpr unsigned int LOCAL_BUFFER_SIZE = 100;
coordCursor.Y -= diff;
}
// If the margins are set, then it shouldn't be possible for the cursor to
// move below the bottom of the viewport. Either it should be constrained
// inside the margins by one of the scrollDown cases handled above, or
// we'll need to clamp it inside the viewport here.
if (fMarginsSet && coordCursor.Y > viewport.BottomInclusive())
{
coordCursor.Y = viewport.BottomInclusive();
}
NTSTATUS Status = STATUS_SUCCESS;
if (coordCursor.Y >= bufferSize.Y)

32
src/host/ut_host/ScreenBufferTests.cpp
View file

@ -117,6 +117,7 @@ class ScreenBufferTests
TEST_METHOD(VtNewlinePastViewport);
TEST_METHOD(VtNewlinePastEndOfBuffer);
TEST_METHOD(VtNewlineOutsideMargins);
TEST_METHOD(VtSetColorTable);
@ -1475,6 +1476,37 @@ void ScreenBufferTests::VtNewlinePastEndOfBuffer()
}
}
void ScreenBufferTests::VtNewlineOutsideMargins()
{
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
auto& si = gci.GetActiveOutputBuffer();
auto& stateMachine = si.GetStateMachine();
auto& cursor = si.GetTextBuffer().GetCursor();
const auto viewportTop = si.GetViewport().Top();
const auto viewportBottom = si.GetViewport().BottomInclusive();
// Make sure the bottom margin will fit inside the viewport.
VERIFY_IS_TRUE(si.GetViewport().Height() > 5);
Log::Comment(L"LF at bottom of viewport scrolls the viewport");
cursor.SetPosition({ 0, viewportBottom });
stateMachine.ProcessString(L"\n");
VERIFY_ARE_EQUAL(COORD({ 0, viewportBottom + 1 }), cursor.GetPosition());
VERIFY_ARE_EQUAL(COORD({ 0, viewportTop + 1 }), si.GetViewport().Origin());
Log::Comment(L"Reset viewport and apply DECSTBM margins");
VERIFY_SUCCEEDED(si.SetViewportOrigin(true, COORD({ 0, viewportTop }), true));
stateMachine.ProcessString(L"\x1b[1;5r");
// Make sure we clear the margins on exit so they can't break other tests.
auto clearMargins = wil::scope_exit([&] { stateMachine.ProcessString(L"\x1b[r"); });
Log::Comment(L"LF no longer scrolls the viewport when below bottom margin");
cursor.SetPosition({ 0, viewportBottom });
stateMachine.ProcessString(L"\n");
VERIFY_ARE_EQUAL(COORD({ 0, viewportBottom }), cursor.GetPosition());
VERIFY_ARE_EQUAL(COORD({ 0, viewportTop }), si.GetViewport().Origin());
}
void ScreenBufferTests::VtSetColorTable()
{
CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation();