A pair of fixes related to cursor movement in conpty (#4372)

## Summary of the Pull Request

This is a pair of related fixes to conpty. For both of these bugs, the root cause was that the cursor was getting set to Off in conpty. Without the `CursorBlinkerTimer`, the cursor would remain off, and frames that only had cursor movements would not update the cursor position in the terminal.

## References

## PR Checklist
* [x] Closes #4102 
* [x] Closes #2642
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated

## Detailed Description of the Pull Request / Additional comments

Recall that there's a bunch of cursor state that's hard to parse without looking up:
* `Visibility` This controls whether the cursor is visible _at all_, regardless if it's been blinked on or off
* `Blinking` controls whether the blinker timer should do something, or leave the cursor alone.
* `IsOn`: When the cursor is blinking, this alternates between true and false. 

The trick here is that we only `TriggerCursorMoved` when the cursor is `On`, and there are some scenarios where the cursor is manually set to off. 

Fundamentally, these two bugs are similar cases, but they are triggered by different things:
* #2642 was caused by `DoSrvPrivateAllowCursorBlinking(false)` (`^[[?12l`) also manually turning the cursor off.
* #4102 was caused by the client calling `SetConsoleScreenBuffer` to change the active buffer. `win-curses` actually uses that API instead of the alt buffer.
This commit is contained in:
Mike Griese 2020-01-30 14:14:16 -06:00 committed by GitHub
parent 4ad77d9ff7
commit 7e2f51face
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 75 additions and 3 deletions

View file

@ -1341,7 +1341,14 @@ void DoSrvPrivateShowCursor(SCREEN_INFORMATION& screenInfo, const bool show) noe
void DoSrvPrivateAllowCursorBlinking(SCREEN_INFORMATION& screenInfo, const bool fEnable)
{
screenInfo.GetActiveBuffer().GetTextBuffer().GetCursor().SetBlinkingAllowed(fEnable);
screenInfo.GetActiveBuffer().GetTextBuffer().GetCursor().SetIsOn(!fEnable);
// GH#2642 - From what we've gathered from other terminals, when blinking is
// disabled, the cursor should remain On always, and have the visibility
// controlled by the IsVisible property. So when you do a printf "\e[?12l"
// to disable blinking, the cursor stays stuck On. At this point, only the
// cursor visibility property controls whether the user can see it or not.
// (Yes, the cursor can be On and NOT Visible)
screenInfo.GetActiveBuffer().GetTextBuffer().GetCursor().SetIsOn(true);
}
// Routine Description:

View file

@ -457,8 +457,20 @@ void SetActiveScreenBuffer(SCREEN_INFORMATION& screenInfo)
CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
gci.pCurrentScreenBuffer = &screenInfo;
// initialize cursor
screenInfo.GetTextBuffer().GetCursor().SetIsOn(false);
// initialize cursor GH#4102 - Typically, the cursor is set to on by the
// cursor blinker. Unfortunately, in conpty mode, there is no cursor
// blinker. So, in conpty mode, we need to leave the cursor on always. The
// cursor can still be set to hidden, and whether the cursor should be
// blinking will still be passed through to the terminal, but internally,
// the cursor should always be on.
//
// In particular, some applications make use of a calling
// `SetConsoleScreenBuffer` and `SetCursorPosition` without printing any
// text in between these calls. If we initialize the cursor to Off in conpty
// mode, then the cursor will remain off until they print text. This can
// lead to alignment problems in the terminal, because we won't move the
// terminal's cursor in this _exact_ scenario.
screenInfo.GetTextBuffer().GetCursor().SetIsOn(gci.IsInVtIoMode());
// set font
screenInfo.RefreshFontWithRenderer();

View file

@ -208,6 +208,8 @@ class ScreenBufferTests
TEST_METHOD(CursorSaveRestore);
TEST_METHOD(ScreenAlignmentPattern);
TEST_METHOD(TestCursorIsOn);
};
void ScreenBufferTests::SingleAlternateBufferCreationTest()
@ -5730,3 +5732,54 @@ void ScreenBufferTests::ScreenAlignmentPattern()
expectedAttr.SetMetaAttributes(0);
VERIFY_ARE_EQUAL(expectedAttr, si.GetAttributes());
}
void ScreenBufferTests::TestCursorIsOn()
{
auto& g = ServiceLocator::LocateGlobals();
auto& gci = g.getConsoleInformation();
auto& si = gci.GetActiveOutputBuffer();
auto& tbi = si.GetTextBuffer();
auto& stateMachine = si.GetStateMachine();
auto& cursor = tbi.GetCursor();
stateMachine.ProcessString(L"Hello World");
VERIFY_IS_TRUE(cursor.IsOn());
VERIFY_IS_TRUE(cursor.IsBlinkingAllowed());
VERIFY_IS_TRUE(cursor.IsVisible());
stateMachine.ProcessString(L"\x1b[?12l");
VERIFY_IS_TRUE(cursor.IsOn());
VERIFY_IS_FALSE(cursor.IsBlinkingAllowed());
VERIFY_IS_TRUE(cursor.IsVisible());
stateMachine.ProcessString(L"\x1b[?12h");
VERIFY_IS_TRUE(cursor.IsOn());
VERIFY_IS_TRUE(cursor.IsBlinkingAllowed());
VERIFY_IS_TRUE(cursor.IsVisible());
cursor.SetIsOn(false);
stateMachine.ProcessString(L"\x1b[?12l");
VERIFY_IS_TRUE(cursor.IsOn());
VERIFY_IS_FALSE(cursor.IsBlinkingAllowed());
VERIFY_IS_TRUE(cursor.IsVisible());
stateMachine.ProcessString(L"\x1b[?12h");
VERIFY_IS_TRUE(cursor.IsOn());
VERIFY_IS_TRUE(cursor.IsBlinkingAllowed());
VERIFY_IS_TRUE(cursor.IsVisible());
stateMachine.ProcessString(L"\x1b[?25l");
VERIFY_IS_TRUE(cursor.IsOn());
VERIFY_IS_TRUE(cursor.IsBlinkingAllowed());
VERIFY_IS_FALSE(cursor.IsVisible());
stateMachine.ProcessString(L"\x1b[?25h");
VERIFY_IS_TRUE(cursor.IsOn());
VERIFY_IS_TRUE(cursor.IsBlinkingAllowed());
VERIFY_IS_TRUE(cursor.IsVisible());
stateMachine.ProcessString(L"\x1b[?12;25l");
VERIFY_IS_TRUE(cursor.IsOn());
VERIFY_IS_FALSE(cursor.IsBlinkingAllowed());
VERIFY_IS_FALSE(cursor.IsVisible());
}