Skip to content

Commit

Permalink
Allow Interactivity to suppress autoscroll
Browse files Browse the repository at this point in the history
This will need to be undone when we move timers and autoscroll down.
  • Loading branch information
DHowett committed Aug 22, 2024
1 parent b3f4162 commit b55fee5
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 16 deletions.
5 changes: 4 additions & 1 deletion src/cascadia/TerminalControl/ControlInteractivity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -334,14 +334,17 @@ namespace winrt::Microsoft::Terminal::Control::implementation
const ::Microsoft::Terminal::Core::ControlKeyStates modifiers,
const bool focused,
const Core::Point pixelPosition,
const bool pointerPressedInBounds)
const bool pointerPressedInBounds,
bool& suppressFurtherHandling)
{
const auto terminalPosition = _getTerminalPosition(til::point{ pixelPosition });
suppressFurtherHandling = false;

// Short-circuit isReadOnly check to avoid warning dialog
if (focused && !_core->IsInReadOnlyMode() && _canSendVTMouseInput(modifiers))
{
_sendMouseEventHelper(terminalPosition, pointerUpdateKind, modifiers, 0, buttonState);
suppressFurtherHandling = true;
}
// GH#4603 - don't modify the selection if the pointer press didn't
// actually start _in_ the control bounds. Case in point - someone drags
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalControl/ControlInteractivity.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
const ::Microsoft::Terminal::Core::ControlKeyStates modifiers,
const bool focused,
const Core::Point pixelPosition,
const bool pointerPressedInBounds);
const bool pointerPressedInBounds,
bool& suppressFurtherHandling);
void TouchMoved(const Core::Point newTouchPoint,
const bool focused);

Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalControl/ControlInteractivity.idl
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ namespace Microsoft.Terminal.Control
Microsoft.Terminal.Core.ControlKeyStates modifiers,
Boolean focused,
Microsoft.Terminal.Core.Point pixelPosition,
Boolean pointerPressedInBounds);
Boolean pointerPressedInBounds,
out Boolean suppressFurtherHandling);

void TouchMoved(Microsoft.Terminal.Core.Point newTouchPoint,
Boolean focused);
Expand Down
6 changes: 4 additions & 2 deletions src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1910,18 +1910,20 @@ namespace winrt::Microsoft::Terminal::Control::implementation
if (type == Windows::Devices::Input::PointerDeviceType::Mouse ||
type == Windows::Devices::Input::PointerDeviceType::Pen)
{
bool suppressFurtherHandling{ false };
_interactivity.PointerMoved(TermControl::GetPressedMouseButtons(point),
TermControl::GetPointerUpdateKind(point),
ControlKeyStates(args.KeyModifiers()),
_focused,
pixelPosition.to_core_point(),
_pointerPressedInBounds);
_pointerPressedInBounds,
/* out */ suppressFurtherHandling);

// GH#9109 - Only start an auto-scroll when the drag actually
// started within our bounds. Otherwise, someone could start a drag
// outside the terminal control, drag into the padding, and trick us
// into starting to scroll.
if (_focused && _pointerPressedInBounds && point.Properties().IsLeftButtonPressed())
if (!suppressFurtherHandling && _focused && _pointerPressedInBounds && point.Properties().IsLeftButtonPressed())
{
// We want to find the distance relative to the bounds of the
// SwapChainPanel, not the entire control. If they drag out of
Expand Down
45 changes: 34 additions & 11 deletions src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,8 @@ namespace ControlUnitTests
TEST_METHOD_PROPERTY(L"IsolationLevel", L"Method")
END_TEST_METHOD_PROPERTIES()

bool unused{};

// This is a test for GH#9725
WEX::TestExecution::DisableVerifyExceptions disableVerifyExceptions{};

Expand Down Expand Up @@ -333,7 +335,8 @@ namespace ControlUnitTests
modifiers,
true, // focused,
cursorPosition1.to_core_point(),
true);
true,
unused);
Log::Comment(L"Verify that there's one selection");
VERIFY_IS_TRUE(core->HasSelection());

Expand All @@ -345,7 +348,8 @@ namespace ControlUnitTests
modifiers,
true, // focused,
cursorPosition2.to_core_point(),
true);
true,
unused);
Log::Comment(L"Verify that there's now two selections (one on each row)");
VERIFY_IS_TRUE(core->HasSelection());

Expand Down Expand Up @@ -376,7 +380,8 @@ namespace ControlUnitTests
modifiers,
true, // focused,
cursorPosition4.to_core_point(),
true);
true,
unused);
Log::Comment(L"Verify that there's now one selection");
VERIFY_IS_TRUE(core->HasSelection());
}
Expand All @@ -390,6 +395,8 @@ namespace ControlUnitTests
// For the sake of this test, scroll one line at a time
interactivity->_rowsToScroll = 1;

bool unused{};

Log::Comment(L"Add some test to the terminal so we can scroll");
for (auto i = 0; i < 40; ++i)
{
Expand Down Expand Up @@ -430,7 +437,8 @@ namespace ControlUnitTests
modifiers,
true, // focused,
cursorPosition1.to_core_point(),
true);
true,
unused);
Log::Comment(L"Verify that there's one selection");
VERIFY_IS_TRUE(core->HasSelection());

Expand Down Expand Up @@ -550,6 +558,8 @@ namespace ControlUnitTests
auto [core, interactivity] = _createCoreAndInteractivity(*settings, *conn);
_standardInit(core, interactivity);

bool unused{};

// For this test, don't use any modifiers
const auto modifiers = ControlKeyStates();
const auto leftMouseDown{ Control::MouseButtonState::IsLeftButtonDown };
Expand Down Expand Up @@ -577,7 +587,8 @@ namespace ControlUnitTests
modifiers,
true, // focused,
cursorPosition1.to_core_point(),
true);
true,
unused);
Log::Comment(L"Verify that there's one selection");
VERIFY_IS_TRUE(core->HasSelection());

Expand All @@ -594,6 +605,8 @@ namespace ControlUnitTests
auto [core, interactivity] = _createCoreAndInteractivity(*settings, *conn);
_standardInit(core, interactivity);

bool unused {}

// For this test, don't use any modifiers
const auto modifiers = ControlKeyStates();
const auto leftMouseDown{ Control::MouseButtonState::IsLeftButtonDown };
Expand Down Expand Up @@ -621,7 +634,8 @@ namespace ControlUnitTests
modifiers,
true, // focused,
cursorPosition1.to_core_point(),
true);
true,
unused);
Log::Comment(L"Verify that there's one selection");
VERIFY_IS_TRUE(core->HasSelection());

Expand All @@ -646,7 +660,8 @@ namespace ControlUnitTests
modifiers,
true, // focused,
cursorPosition2.to_core_point(),
false);
false,
unused);

Log::Comment(L"The selection should be unchanged.");
VERIFY_ARE_EQUAL(expectedAnchor, core->_terminal->GetSelectionAnchor());
Expand All @@ -662,6 +677,8 @@ namespace ControlUnitTests
auto [core, interactivity] = _createCoreAndInteractivity(*settings, *conn);
_standardInit(core, interactivity);

bool unused{};

// For this test, don't use any modifiers
const auto modifiers = ControlKeyStates();
const auto leftMouseDown{ Control::MouseButtonState::IsLeftButtonDown };
Expand Down Expand Up @@ -737,7 +754,8 @@ namespace ControlUnitTests
modifiers,
true, // focused,
cursorPosition1.to_core_point(),
true);
true,
unused);
Log::Comment(L"Verify that there's still no selection");
VERIFY_IS_FALSE(core->HasSelection());
}
Expand All @@ -751,6 +769,8 @@ namespace ControlUnitTests
auto [core, interactivity] = _createCoreAndInteractivity(*settings, *conn);
_standardInit(core, interactivity);

bool unused{};

Log::Comment(L"Fill up the history buffer");
const auto scrollbackLength = settings->HistorySize();
// Output lines equal to history size + viewport height to make sure we're
Expand Down Expand Up @@ -790,7 +810,8 @@ namespace ControlUnitTests
modifiers,
true, // focused,
cursorPosition1.to_core_point(),
true);
true,
unused);
Log::Comment(L"Verify that there's one selection");
VERIFY_IS_TRUE(core->HasSelection());

Expand Down Expand Up @@ -849,7 +870,8 @@ namespace ControlUnitTests
modifiers,
true, // focused,
cursorPosition0.to_core_point(),
true);
true,
unused);
VERIFY_IS_TRUE(core->HasSelection());
{
const auto anchor{ core->_terminal->GetSelectionAnchor() };
Expand All @@ -873,7 +895,8 @@ namespace ControlUnitTests
modifiers,
true, // focused,
cursorPosition1.to_core_point(),
true);
true,
unused);
VERIFY_IS_TRUE(core->HasSelection());
{
const auto anchor{ core->_terminal->GetSelectionAnchor() };
Expand Down

0 comments on commit b55fee5

Please sign in to comment.