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

Fix the xterm and SGR mouse encodings for CTRL, ALT, SHIFT (#8379)

Browse source 
We had the xterm and SGR codings for meta/ctrl backwards. Oops.

This commit also fixes an observed issue in Windows Terminal where we
were passing in a console-style modifiers enum when MouseInput is
expecting MK_ constants.

I decided to unify MouseInput around the console-style modifier
constants because they have support for META (which MK_ does not) and
can differentiate between left/right alt/ctrl.

Our tests are fundamentally flawed here: they use a copy of the
modifier key generating logic _themselves_, so we got a bit of "error
carried forward."

I did not fix the tests to use known-good control sequences, I simply
replaced the character generator with another copy of the modifier code.
I did, however, extend them to test ctrl|meta and left/right modifiers.

Fixes #8291
This commit is contained in:
Dustin L. Howett 2020-11-29 19:45:53 -08:00 committed by GitHub
parent b7bebc765e
commit b1e1c7cdf4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 33 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
src/interactivity/win32/windowio.cpp
View file

@ -651,8 +651,12 @@ BOOL HandleMouseEvent(const SCREEN_INFORMATION& ScreenInfo,
sDelta = GET_WHEEL_DELTA_WPARAM(wParam);
}
if (HandleTerminalMouseEvent(MousePosition, Message, GET_KEYSTATE_WPARAM(wParam), sDelta))
if (HandleTerminalMouseEvent(MousePosition, Message, LOWORD(GetControlKeyState(0)), sDelta))
{
// Use GetControlKeyState here to get the control state in console event mode.
// This will ensure that we get ALT and SHIFT, the former of which is not available
// through MK_ constants. We only care about the bottom 16 bits.
// GH#6401: Capturing the mouse ensures that we get drag/release events
// even if the user moves outside the window.
// HandleTerminalMouseEvent returns false if the terminal's not in VT mode,

25
src/terminal/adapter/ut_adapter/MouseInputTest.cpp
View file

@ -186,8 +186,10 @@ public:
wch = L'\x0';
break;
}
// MK_SHIFT is ignored by the translator
wch += (sModifierKeystate & MK_CONTROL) ? 0x08 : 0x00;
// Use Any here with the multi-flag constants -- they capture left/right key state
WI_UpdateFlag(wch, 0x04, WI_IsAnyFlagSet(sModifierKeystate, SHIFT_PRESSED));
WI_UpdateFlag(wch, 0x08, WI_IsAnyFlagSet(sModifierKeystate, ALT_PRESSED));
WI_UpdateFlag(wch, 0x10, WI_IsAnyFlagSet(sModifierKeystate, CTRL_PRESSED));
return wch;
}
@ -222,8 +224,10 @@ public:
result = 0;
break;
}
// MK_SHIFT and MK_ALT is ignored by the translator
result += (sModifierKeystate & MK_CONTROL) ? 0x08 : 0x00;
// Use Any here with the multi-flag constants -- they capture left/right key state
WI_UpdateFlag(result, 0x04, WI_IsAnyFlagSet(sModifierKeystate, SHIFT_PRESSED));
WI_UpdateFlag(result, 0x08, WI_IsAnyFlagSet(sModifierKeystate, ALT_PRESSED));
WI_UpdateFlag(result, 0x10, WI_IsAnyFlagSet(sModifierKeystate, CTRL_PRESSED));
return result;
}
@ -274,8 +278,8 @@ public:
BEGIN_TEST_METHOD_PROPERTIES()
// TEST_METHOD_PROPERTY(L"Data:uiButton", L"{WM_LBUTTONDOWN, WM_LBUTTONUP, WM_MBUTTONDOWN, WM_MBUTTONUP, WM_RBUTTONDOWN, WM_RBUTTONUP}")
TEST_METHOD_PROPERTY(L"Data:uiButton", L"{0x0201, 0x0202, 0x0207, 0x0208, 0x0204, 0x0205}")
// None, MK_SHIFT, MK_CONTROL
TEST_METHOD_PROPERTY(L"Data:uiModifierKeystate", L"{0x0000, 0x0004, 0x0008}")
// None, SHIFT, LEFT_CONTROL, RIGHT_ALT, RIGHT_ALT | LEFT_CONTROL
TEST_METHOD_PROPERTY(L"Data:uiModifierKeystate", L"{0x0000, 0x0010, 0x0008, 0x0001, 0x0009}")
END_TEST_METHOD_PROPERTIES()
Log::Comment(L"Starting test...");
@ -357,7 +361,8 @@ public:
BEGIN_TEST_METHOD_PROPERTIES()
// TEST_METHOD_PROPERTY(L"Data:uiButton", L"{WM_LBUTTONDOWN, WM_LBUTTONUP, WM_MBUTTONDOWN, WM_MBUTTONUP, WM_RBUTTONDOWN, WM_RBUTTONUP}")
TEST_METHOD_PROPERTY(L"Data:uiButton", L"{0x0201, 0x0202, 0x0207, 0x0208, 0x0204, 0x0205}")
TEST_METHOD_PROPERTY(L"Data:uiModifierKeystate", L"{0x0000, 0x0004, 0x0008}")
// None, SHIFT, LEFT_CONTROL, RIGHT_ALT, RIGHT_ALT | LEFT_CONTROL
TEST_METHOD_PROPERTY(L"Data:uiModifierKeystate", L"{0x0000, 0x0010, 0x0008, 0x0001, 0x0009}")
END_TEST_METHOD_PROPERTIES()
Log::Comment(L"Starting test...");
@ -443,7 +448,8 @@ public:
BEGIN_TEST_METHOD_PROPERTIES()
// TEST_METHOD_PROPERTY(L"Data:uiButton", L"{WM_LBUTTONDOWN, WM_LBUTTONUP, WM_MBUTTONDOWN, WM_MBUTTONUP, WM_RBUTTONDOWN, WM_RBUTTONUP, WM_MOUSEMOVE}")
TEST_METHOD_PROPERTY(L"Data:uiButton", L"{0x0201, 0x0202, 0x0207, 0x0208, 0x0204, 0x0205, 0x0200}")
TEST_METHOD_PROPERTY(L"Data:uiModifierKeystate", L"{0x0000, 0x0004, 0x0008}")
// None, SHIFT, LEFT_CONTROL, RIGHT_ALT, RIGHT_ALT | LEFT_CONTROL
TEST_METHOD_PROPERTY(L"Data:uiModifierKeystate", L"{0x0000, 0x0010, 0x0008, 0x0001, 0x0009}")
END_TEST_METHOD_PROPERTIES()
Log::Comment(L"Starting test...");
@ -523,7 +529,8 @@ public:
{
BEGIN_TEST_METHOD_PROPERTIES()
TEST_METHOD_PROPERTY(L"Data:sScrollDelta", L"{-120, 120, -10000, 32736}")
TEST_METHOD_PROPERTY(L"Data:uiModifierKeystate", L"{0x0000, 0x0004, 0x0008}")
// None, SHIFT, LEFT_CONTROL, RIGHT_ALT, RIGHT_ALT | LEFT_CONTROL
TEST_METHOD_PROPERTY(L"Data:uiModifierKeystate", L"{0x0000, 0x0010, 0x0008, 0x0001, 0x0009}")
END_TEST_METHOD_PROPERTIES()
Log::Comment(L"Starting test...");

27
src/terminal/input/mouseInput.cpp
View file

@ -133,8 +133,8 @@ constexpr unsigned int TerminalInput::s_GetPressedButton(const MouseButtonState
// 11 - released (none)
// Next three bits indicate modifier keys:
// 0x04 - shift (This never makes it through, as our emulator is skipped when shift is pressed.)
// 0x08 - ctrl
// 0x10 - meta
// 0x08 - meta
// 0x10 - ctrl
// 32 (x20) is added for "hover" events:
// "For example, motion into cell x,y with button 1 down is reported as `CSI M @ CxCy`.
// ( @ = 32 + 0 (button 1) + 32 (motion indicator) ).
@ -146,7 +146,7 @@ constexpr unsigned int TerminalInput::s_GetPressedButton(const MouseButtonState
// Parameters:
// - button - the message to decode.
// - isHover - whether or not this is a hover event
// - modifierKeyState - alt/ctrl/shift key hold state
// - modifierKeyState - the modifier keys _in console format_
// - delta - scroll wheel delta
// Return value:
// - the int representing the equivalent X button encoding.
@ -188,12 +188,10 @@ static constexpr int _windowsButtonToXEncoding(const unsigned int button,
xvalue += 0x20;
}
// Shift will never pass through to us, because shift is used by the host to skip VT mouse and use the default handler.
// TODO: MSFT:8804719 Add an option to disable/remap shift as a bypass for VT mousemode handling
// xvalue += (modifierKeyState & MK_SHIFT) ? 0x04 : 0x00;
xvalue += (modifierKeyState & MK_CONTROL) ? 0x08 : 0x00;
// Unfortunately, we don't get meta/alt as a part of mouse events. Only Ctrl and Shift.
// xvalue += (modifierKeyState & MK_META) ? 0x10 : 0x00;
// Use Any here with the multi-flag constants -- they capture left/right key state
WI_UpdateFlag(xvalue, 0x04, WI_IsAnyFlagSet(modifierKeyState, SHIFT_PRESSED));
WI_UpdateFlag(xvalue, 0x08, WI_IsAnyFlagSet(modifierKeyState, ALT_PRESSED));
WI_UpdateFlag(xvalue, 0x10, WI_IsAnyFlagSet(modifierKeyState, CTRL_PRESSED));
return xvalue;
}
@ -206,6 +204,7 @@ static constexpr int _windowsButtonToXEncoding(const unsigned int button,
// See MSFT:19461988 and https://github.com/Microsoft/console/issues/296
// Parameters:
// - button - the message to decode.
// - modifierKeyState - the modifier keys _in console format_
// Return value:
// - the int representing the equivalent X button encoding.
static constexpr int _windowsButtonToSGREncoding(const unsigned int button,
@ -247,12 +246,10 @@ static constexpr int _windowsButtonToSGREncoding(const unsigned int button,
xvalue += 0x20;
}
// Shift will never pass through to us, because shift is used by the host to skip VT mouse and use the default handler.
// TODO: MSFT:8804719 Add an option to disable/remap shift as a bypass for VT mousemode handling
// xvalue += (modifierKeyState & MK_SHIFT) ? 0x04 : 0x00;
xvalue += (modifierKeyState & MK_CONTROL) ? 0x08 : 0x00;
// Unfortunately, we don't get meta/alt as a part of mouse events. Only Ctrl and Shift.
// xvalue += (modifierKeyState & MK_META) ? 0x10 : 0x00;
// Use Any here with the multi-flag constants -- they capture left/right key state
WI_UpdateFlag(xvalue, 0x04, WI_IsAnyFlagSet(modifierKeyState, SHIFT_PRESSED));
WI_UpdateFlag(xvalue, 0x08, WI_IsAnyFlagSet(modifierKeyState, ALT_PRESSED));
WI_UpdateFlag(xvalue, 0x10, WI_IsAnyFlagSet(modifierKeyState, CTRL_PRESSED));
return xvalue;
}