Skip to content

Commit 865f5e5

Browse files
authored
Fix SGR mouse movement reports (#18864)
According to the documentation, the final character of an SGR mouse report is meant to be `M` for a button press and `m` for a button release. However it isn't clear what the final character should be for motion events, and we were using an `m` if there weren't any buttons held down at the time, while all other terminals used an `M`, regardless of the button state. This PR updates our implementation to match what everyone else is doing, since our interpretation of the spec was causing problems for some apps. ## Validation Steps Performed I've manually tested the new behavior in Vttest, and confirmed that our mouse reports now match Xterm more closely, and I've tested with v0.42.0 of Zellij, which was previous glitching badly in Windows Terminal, but now works correctly. I've also updated our unit tests for the SGR mouse mode to reflect the correct report format. Closes #18712
1 parent 58092f1 commit 865f5e5

File tree

3 files changed

+22
-32
lines changed

3 files changed

+22
-32
lines changed

src/terminal/adapter/ut_adapter/MouseInputTest.cpp

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ class MouseInputTest
9999

100100
TerminalInput::StringType str;
101101
str.append(std::wstring_view{ buffer });
102-
str[str.size() - 1] = IsButtonDown(uiButton) ? L'M' : L'm';
102+
str[str.size() - 1] = IsButtonUp(uiButton) ? L'm' : L'M';
103103
return str;
104104
}
105105

@@ -182,23 +182,18 @@ class MouseInputTest
182182
return result;
183183
}
184184

185-
bool IsButtonDown(unsigned int uiButton)
185+
bool IsButtonUp(unsigned int uiButton)
186186
{
187-
auto fIsDown = false;
188187
switch (uiButton)
189188
{
190-
case WM_LBUTTONDBLCLK:
191-
case WM_LBUTTONDOWN:
192-
case WM_RBUTTONDOWN:
193-
case WM_RBUTTONDBLCLK:
194-
case WM_MBUTTONDOWN:
195-
case WM_MBUTTONDBLCLK:
196-
case WM_MOUSEWHEEL:
197-
case WM_MOUSEHWHEEL:
198-
fIsDown = true;
199-
break;
189+
case WM_LBUTTONUP:
190+
case WM_RBUTTONUP:
191+
case WM_MBUTTONUP:
192+
case WM_XBUTTONUP:
193+
return true;
194+
default:
195+
return false;
200196
}
201-
return fIsDown;
202197
}
203198

204199
/* From winuser.h - Needed to manually specify the test properties

src/terminal/input/mouseInput.cpp

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -63,24 +63,19 @@ static constexpr bool _isWheelMsg(const unsigned int buttonCode) noexcept
6363
}
6464

6565
// Routine Description:
66-
// - Determines if the input windows message code describes a button press
67-
// (either down or doubleclick)
66+
// - Determines if the input windows message code describes a button release
6867
// Parameters:
6968
// - button - the message to decode.
7069
// Return value:
71-
// - true if button is a button down event
72-
static constexpr bool _isButtonDown(const unsigned int button) noexcept
70+
// - true if button is a button up event
71+
static constexpr bool _isButtonUp(const unsigned int button) noexcept
7372
{
7473
switch (button)
7574
{
76-
case WM_LBUTTONDBLCLK:
77-
case WM_LBUTTONDOWN:
78-
case WM_RBUTTONDOWN:
79-
case WM_RBUTTONDBLCLK:
80-
case WM_MBUTTONDOWN:
81-
case WM_MBUTTONDBLCLK:
82-
case WM_MOUSEWHEEL:
83-
case WM_MOUSEHWHEEL:
75+
case WM_LBUTTONUP:
76+
case WM_RBUTTONUP:
77+
case WM_MBUTTONUP:
78+
case WM_XBUTTONUP:
8479
return true;
8580
default:
8681
return false;
@@ -372,7 +367,7 @@ TerminalInput::OutputType TerminalInput::HandleMouse(const til::point position,
372367
// then we want to handle hovers with WM_MOUSEMOVE.
373368
// However, if we're dragging (WM_MOUSEMOVE with a button pressed),
374369
// then use that pressed button instead.
375-
return _GenerateSGRSequence(position, physicalButtonPressed ? realButton : button, _isButtonDown(realButton), isHover, modifierKeyState, delta);
370+
return _GenerateSGRSequence(position, physicalButtonPressed ? realButton : button, _isButtonUp(button), isHover, modifierKeyState, delta);
376371
}
377372
else
378373
{
@@ -463,18 +458,18 @@ TerminalInput::OutputType TerminalInput::_GenerateUtf8Sequence(const til::point
463458
// Parameters:
464459
// - position - The windows coordinates (top,left = 0,0) of the mouse event
465460
// - button - the message to decode. WM_MOUSEMOVE is used for mouse hovers with no buttons pressed.
466-
// - isDown - true if a mouse button was pressed.
461+
// - isRelease - true if a mouse button was released.
467462
// - isHover - true if the sequence is generated in response to a mouse hover
468463
// - modifierKeyState - the modifier keys pressed with this button
469464
// - delta - the amount that the scroll wheel changed (should be 0 unless button is a WM_MOUSE*WHEEL)
470465
// - ppwchSequence - On success, where to put the pointer to the generated sequence
471466
// - pcchLength - On success, where to put the length of the generated sequence
472-
TerminalInput::OutputType TerminalInput::_GenerateSGRSequence(const til::point position, const unsigned int button, const bool isDown, const bool isHover, const short modifierKeyState, const short delta)
467+
TerminalInput::OutputType TerminalInput::_GenerateSGRSequence(const til::point position, const unsigned int button, const bool isRelease, const bool isHover, const short modifierKeyState, const short delta)
473468
{
474469
// Format for SGR events is:
475-
// "\x1b[<%d;%d;%d;%c", xButton, x+1, y+1, fButtonDown? 'M' : 'm'
470+
// "\x1b[<%d;%d;%d;%c", xButton, x+1, y+1, isRelease? 'm' : 'M'
476471
const auto xbutton = _windowsButtonToSGREncoding(button, isHover, modifierKeyState, delta);
477-
return fmt::format(FMT_COMPILE(L"{}<{};{};{}{}"), _csi, xbutton, position.x + 1, position.y + 1, isDown ? L'M' : L'm');
472+
return fmt::format(FMT_COMPILE(L"{}<{};{};{}{}"), _csi, xbutton, position.x + 1, position.y + 1, isRelease ? L'm' : L'M');
478473
}
479474

480475
// Routine Description:

src/terminal/input/terminalInput.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ namespace Microsoft::Console::VirtualTerminal
109109
#pragma region MouseInput
110110
[[nodiscard]] OutputType _GenerateDefaultSequence(til::point position, unsigned int button, bool isHover, short modifierKeyState, short delta);
111111
[[nodiscard]] OutputType _GenerateUtf8Sequence(til::point position, unsigned int button, bool isHover, short modifierKeyState, short delta);
112-
[[nodiscard]] OutputType _GenerateSGRSequence(til::point position, unsigned int button, bool isDown, bool isHover, short modifierKeyState, short delta);
112+
[[nodiscard]] OutputType _GenerateSGRSequence(til::point position, unsigned int button, bool isRelease, bool isHover, short modifierKeyState, short delta);
113113

114114
[[nodiscard]] OutputType _makeAlternateScrollOutput(short delta) const;
115115

0 commit comments

Comments
 (0)