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

Fix mouse coordinates when viewport is scrolled #10642

Merged
11 commits merged into from
Jul 20, 2021
7 changes: 6 additions & 1 deletion src/cascadia/TerminalControl/ControlInteractivity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
}
else if (_canSendVTMouseInput(modifiers))
{
_core->SendMouseEvent(terminalPosition, pointerUpdateKind, modifiers, 0, toInternalMouseState(buttonState));
const auto adjustment = _core->ScrollOffset() > 0 ? _core->BufferHeight() - _core->ScrollOffset() - _core->ViewHeight() : 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised we don't already have a thing that calculates "how far back from the top line are we scrolled"... hmm.

// If the click happened outside the active region, just don't send any mouse event
if (const auto adjustedY = terminalPosition.y() - adjustment; adjustedY >= 0)
{
_core->SendMouseEvent({ terminalPosition.x(), adjustedY }, pointerUpdateKind, modifiers, 0, toInternalMouseState(buttonState));
}
}
else if (WI_IsFlagSet(buttonState, MouseButtonState::IsLeftButtonDown))
{
Expand Down
90 changes: 90 additions & 0 deletions src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ namespace ControlUnitTests
TEST_METHOD(ScrollWithSelection);
TEST_METHOD(TestScrollWithTrackpad);
TEST_METHOD(TestQuickDragOnSelect);
TEST_METHOD(PointerClickOutsideActiveRegion);

TEST_CLASS_SETUP(ClassSetup)
{
Expand Down Expand Up @@ -544,4 +545,93 @@ namespace ControlUnitTests
COORD expectedAnchor{ 0, 0 };
VERIFY_ARE_EQUAL(expectedAnchor, core->_terminal->GetSelectionAnchor());
}

void ControlInteractivityTests::PointerClickOutsideActiveRegion()
{
// This is a test for GH#10642
WEX::TestExecution::DisableVerifyExceptions disableVerifyExceptions{};

auto [settings, conn] = _createSettingsAndConnection();
auto [core, interactivity] = _createCoreAndInteractivity(*settings, *conn);
_standardInit(core, interactivity);

// For this test, don't use any modifiers
const auto modifiers = ControlKeyStates();
const Control::MouseButtonState leftMouseDown{ Control::MouseButtonState::IsLeftButtonDown };
const Control::MouseButtonState noMouseDown{};

const til::size fontSize{ 9, 21 };

interactivity->_rowsToScroll = 1;
int expectedTop = 0;
int expectedViewHeight = 20;
int expectedBufferHeight = 20;

auto scrollChangedHandler = [&](auto&&, const Control::ScrollPositionChangedArgs& args) mutable {
VERIFY_ARE_EQUAL(expectedTop, args.ViewTop());
VERIFY_ARE_EQUAL(expectedViewHeight, args.ViewHeight());
VERIFY_ARE_EQUAL(expectedBufferHeight, args.BufferSize());
};
core->ScrollPositionChanged(scrollChangedHandler);
interactivity->ScrollPositionChanged(scrollChangedHandler);

for (int i = 0; i < 40; ++i)
{
Log::Comment(NoThrowString().Format(L"Writing line #%d", i));
// The \r\n in the 19th loop will cause the view to start moving
if (i >= 19)
{
expectedTop++;
expectedBufferHeight++;
}

conn->WriteInput(L"Foo\r\n");
}
// We printed that 40 times, but the final \r\n bumped the view down one MORE row.
VERIFY_ARE_EQUAL(20, core->_terminal->GetViewport().Height());
VERIFY_ARE_EQUAL(21, core->ScrollOffset());
VERIFY_ARE_EQUAL(20, core->ViewHeight());
VERIFY_ARE_EQUAL(41, core->BufferHeight());

expectedBufferHeight = 41;
expectedTop = 21;

Log::Comment(L"Scroll up 10 times");
for (int i = 0; i < 11; ++i)
{
expectedTop--;
interactivity->MouseWheel(modifiers,
WHEEL_DELTA,
til::point{ 0, 0 },
noMouseDown);
}

// Enable VT mouse event tracking
conn->WriteInput(L"\x1b[?1003;1006h");

// Mouse clicks in the inactive region (i.e. the top 10 rows in this case) should not register
Log::Comment(L"Click on the terminal");
const til::point terminalPosition0{ 4, 4 };
const til::point cursorPosition0 = terminalPosition0 * fontSize;
interactivity->PointerPressed(leftMouseDown,
WM_LBUTTONDOWN, //pointerUpdateKind
0, // timestamp
modifiers,
cursorPosition0);
Log::Comment(L"Verify that there's not yet a selection");

VERIFY_IS_FALSE(core->HasSelection());

Log::Comment(L"Drag the mouse");
// move the mouse as if to make a selection
const til::point terminalPosition1{ 10, 4 };
const til::point cursorPosition1 = terminalPosition1 * fontSize;
interactivity->PointerMoved(leftMouseDown,
WM_LBUTTONDOWN, //pointerUpdateKind
modifiers,
true, // focused,
cursorPosition1);
Log::Comment(L"Verify that there's still no selection");
VERIFY_IS_FALSE(core->HasSelection());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we may not have a way to test that it actually doesn't generate mouse events. oh well 😄

@zadjii-msft any ideas? would we need the mock connection to tell us? that sounds tedious.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ew yea that would probably require a lot more plumbing than it's worth.

}
}