Skip to content

Commit

Permalink
Make sure RIS re-enables win32 input and focus events (#15476)
Browse files Browse the repository at this point in the history
When an `RIS` (hard reset) sequence is executed, ConHost will pass that
through to the connected conpty terminal, which results in all modes
being reset to their initial state. To work best, though, conpty needs
to have the win32 input mode enabled, as well as the focus event mode.
This PR addresses that by explicitly requesting the required modes after
an `RIS` is passed through.

Originally these modes were only set if the `--win32input` option was
given, but there is really no need for that, since terminals that don't
support them should simply ignore the request. To avoid that additional
complication, I've now removed the option (i.e. ConHost will now always
attempt to set the modes it needs).

I've manually confirmed that keypresses are still passed through with
win32 input sequences after a hard reset, and that focus events are
still being generated. I've also updated the existing conpty round-trip
test for `RIS` to confirm that it's now also passing through the mode
requests that it needs.

Closes #15461

(cherry picked from commit 8aefc7a)
Service-Card-Id: 89384907
Service-Version: 1.18
  • Loading branch information
j4james authored and DHowett committed Jul 27, 2023
1 parent 14fb08a commit d11e4dd
Show file tree
Hide file tree
Showing 11 changed files with 26 additions and 35 deletions.
2 changes: 1 addition & 1 deletion src/cascadia/TerminalConnection/ConptyConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
// handoff from an already-started PTY process.
if (!_inPipe)
{
DWORD flags = PSEUDOCONSOLE_RESIZE_QUIRK | PSEUDOCONSOLE_WIN32_INPUT_MODE;
DWORD flags = PSEUDOCONSOLE_RESIZE_QUIRK;

// If we're using an existing buffer, we want the new connection
// to reuse the existing cursor. When not setting this flag, the
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1200,7 +1200,9 @@ void ConptyRoundtripTests::PassthroughHardReset()
}

// Write a Hard Reset VT sequence to the host, it should come through to the Terminal
// along with a DECSET sequence to re-enable win32 input and focus events.
expectedOutput.push_back("\033c");
expectedOutput.push_back("\033[?9001;1004h");
hostSm.ProcessString(L"\033c");

const auto termSecondView = term->GetViewport();
Expand Down
11 changes: 0 additions & 11 deletions src/host/ConsoleArguments.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ const std::wstring_view ConsoleArguments::WIDTH_ARG = L"--width";
const std::wstring_view ConsoleArguments::HEIGHT_ARG = L"--height";
const std::wstring_view ConsoleArguments::INHERIT_CURSOR_ARG = L"--inheritcursor";
const std::wstring_view ConsoleArguments::RESIZE_QUIRK = L"--resizeQuirk";
const std::wstring_view ConsoleArguments::WIN32_INPUT_MODE = L"--win32input";
const std::wstring_view ConsoleArguments::FEATURE_ARG = L"--feature";
const std::wstring_view ConsoleArguments::FEATURE_PTY_ARG = L"pty";
const std::wstring_view ConsoleArguments::COM_SERVER_ARG = L"-Embedding";
Expand Down Expand Up @@ -515,12 +514,6 @@ void ConsoleArguments::s_ConsumeArg(_Inout_ std::vector<std::wstring>& args, _In
s_ConsumeArg(args, i);
hr = S_OK;
}
else if (arg == WIN32_INPUT_MODE)
{
_win32InputMode = true;
s_ConsumeArg(args, i);
hr = S_OK;
}
else if (arg == CLIENT_COMMANDLINE_ARG)
{
// Everything after this is the explicit commandline
Expand Down Expand Up @@ -677,10 +670,6 @@ bool ConsoleArguments::IsResizeQuirkEnabled() const
{
return _resizeQuirk;
}
bool ConsoleArguments::IsWin32InputModeEnabled() const
{
return _win32InputMode;
}

#ifdef UNIT_TESTING
// Method Description:
Expand Down
3 changes: 0 additions & 3 deletions src/host/ConsoleArguments.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ class ConsoleArguments
short GetHeight() const;
bool GetInheritCursor() const;
bool IsResizeQuirkEnabled() const;
bool IsWin32InputModeEnabled() const;

#ifdef UNIT_TESTING
void EnableConptyModeForTests();
Expand All @@ -74,7 +73,6 @@ class ConsoleArguments
static const std::wstring_view HEIGHT_ARG;
static const std::wstring_view INHERIT_CURSOR_ARG;
static const std::wstring_view RESIZE_QUIRK;
static const std::wstring_view WIN32_INPUT_MODE;
static const std::wstring_view FEATURE_ARG;
static const std::wstring_view FEATURE_PTY_ARG;
static const std::wstring_view COM_SERVER_ARG;
Expand Down Expand Up @@ -144,7 +142,6 @@ class ConsoleArguments
DWORD _signalHandle;
bool _inheritCursor;
bool _resizeQuirk{ false };
bool _win32InputMode{ false };

[[nodiscard]] HRESULT _GetClientCommandline(_Inout_ std::vector<std::wstring>& args,
const size_t index,
Expand Down
6 changes: 1 addition & 5 deletions src/host/VtIo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ VtIo::VtIo() :
{
_lookingForCursorPosition = pArgs->GetInheritCursor();
_resizeQuirk = pArgs->IsResizeQuirkEnabled();
_win32InputMode = pArgs->IsWin32InputModeEnabled();
_passthroughMode = pArgs->IsPassthroughMode();

// If we were already given VT handles, set up the VT IO engine to use those.
Expand Down Expand Up @@ -269,10 +268,7 @@ bool VtIo::IsUsingVt() const
// win32-input-mode from them. This will enable the connected terminal to
// send us full INPUT_RECORDs as input. If the terminal doesn't understand
// this sequence, it'll just ignore it.
if (_win32InputMode)
{
LOG_IF_FAILED(_pVtRenderEngine->RequestWin32Input());
}
LOG_IF_FAILED(_pVtRenderEngine->RequestWin32Input());

// MSFT: 15813316
// If the terminal application wants us to inherit the cursor position,
Expand Down
1 change: 0 additions & 1 deletion src/host/VtIo.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ namespace Microsoft::Console::VirtualTerminal
bool _lookingForCursorPosition;

bool _resizeQuirk{ false };
bool _win32InputMode{ false };
bool _passthroughMode{ false };
bool _closeEventSent{ false };

Expand Down
8 changes: 3 additions & 5 deletions src/host/srvinit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -603,11 +603,9 @@ try
outPipeTheirSide.reset();
signalPipeTheirSide.reset();

// GH#13211 - Make sure we request win32input mode and that the terminal
// obeys the resizing quirk. Otherwise, defterm connections to the Terminal
// are going to have weird resizing, and aren't going to send full fidelity
// input messages.
const auto commandLine = fmt::format(FMT_COMPILE(L" --headless --resizeQuirk --win32input --signal {:#x}"),
// GH#13211 - Make sure the terminal obeys the resizing quirk. Otherwise,
// defterm connections to the Terminal are going to have weird resizing.
const auto commandLine = fmt::format(FMT_COMPILE(L" --headless --resizeQuirk --signal {:#x}"),
(int64_t)signalPipeOurSide.release());

ConsoleArguments consoleArgs(commandLine, inPipeOurSide.release(), outPipeOurSide.release());
Expand Down
1 change: 0 additions & 1 deletion src/inc/conpty-static.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
#endif

#define PSEUDOCONSOLE_RESIZE_QUIRK (2u)
#define PSEUDOCONSOLE_WIN32_INPUT_MODE (4u)
#define PSEUDOCONSOLE_PASSTHROUGH_MODE (8u)

CONPTY_EXPORT HRESULT WINAPI ConptyCreatePseudoConsole(COORD size, HANDLE hInput, HANDLE hOutput, DWORD dwFlags, HPCON* phPC);
Expand Down
3 changes: 3 additions & 0 deletions src/renderer/vt/state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,9 @@ void VtEngine::SetTerminalCursorTextPosition(const til::point cursor) noexcept
// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write.
HRESULT VtEngine::RequestWin32Input() noexcept
{
// It's important that any additional modes set here are also mirrored in
// the AdaptDispatch::HardReset method, since that needs to re-enable them
// in the connected terminal after passing through an RIS sequence.
RETURN_IF_FAILED(_RequestWin32Input());
RETURN_IF_FAILED(_RequestFocusEventMode());
RETURN_IF_FAILED(_Flush());
Expand Down
20 changes: 15 additions & 5 deletions src/terminal/adapter/adaptDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2941,11 +2941,21 @@ bool AdaptDispatch::HardReset()
_macroBuffer = nullptr;
}

// GH#2715 - If all this succeeded, but we're in a conpty, return `false` to
// make the state machine propagate this RIS sequence to the connected
// terminal application. We've reset our state, but the connected terminal
// might need to do more.
return !_api.IsConsolePty();
// If we're in a conpty, we need flush this RIS sequence to the connected
// terminal application, but we also need to follow that up with a DECSET
// sequence to re-enable the modes that we require (namely win32 input mode
// and focus event mode). It's important that this is kept in sync with the
// VtEngine::RequestWin32Input method which requests the modes on startup.
if (_api.IsConsolePty())
{
auto& stateMachine = _api.GetStateMachine();
if (stateMachine.FlushToTerminal())
{
auto& engine = stateMachine.Engine();
engine.ActionPassThroughString(L"\033[?9001;1004h");
}
}
return true;
}

// Routine Description:
Expand Down
4 changes: 1 addition & 3 deletions src/winconpty/winconpty.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,19 +133,17 @@ HRESULT _CreatePseudoConsole(const HANDLE hToken,
RETURN_IF_WIN32_BOOL_FALSE(SetHandleInformation(signalPipeConhostSide.get(), HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT));

// GH4061: Ensure that the path to executable in the format is escaped so C:\Program.exe cannot collide with C:\Program Files
auto pwszFormat = L"\"%s\" --headless %s%s%s%s--width %hu --height %hu --signal 0x%x --server 0x%x";
auto pwszFormat = L"\"%s\" --headless %s%s%s--width %hu --height %hu --signal 0x%x --server 0x%x";
// This is plenty of space to hold the formatted string
wchar_t cmd[MAX_PATH]{};
const BOOL bInheritCursor = (dwFlags & PSEUDOCONSOLE_INHERIT_CURSOR) == PSEUDOCONSOLE_INHERIT_CURSOR;
const BOOL bResizeQuirk = (dwFlags & PSEUDOCONSOLE_RESIZE_QUIRK) == PSEUDOCONSOLE_RESIZE_QUIRK;
const BOOL bWin32InputMode = (dwFlags & PSEUDOCONSOLE_WIN32_INPUT_MODE) == PSEUDOCONSOLE_WIN32_INPUT_MODE;
const BOOL bPassthroughMode = (dwFlags & PSEUDOCONSOLE_PASSTHROUGH_MODE) == PSEUDOCONSOLE_PASSTHROUGH_MODE;
swprintf_s(cmd,
MAX_PATH,
pwszFormat,
_ConsoleHostPath(),
bInheritCursor ? L"--inheritcursor " : L"",
bWin32InputMode ? L"--win32input " : L"",
bResizeQuirk ? L"--resizeQuirk " : L"",
bPassthroughMode ? L"--passthrough " : L"",
size.X,
Expand Down

0 comments on commit d11e4dd

Please sign in to comment.