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.

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

Make sure RIS re-enables win32 input and focus events #15476

Merged
merged 4 commits into from
Jun 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -3055,11 +3055,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