Fix a pair of TermControl dragging bugs

src/cascadia/TerminalControl/ControlInteractivity.cpp

@@ -279,7 +279,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                                             const unsigned int pointerUpdateKind,
                                             const ::Microsoft::Terminal::Core::ControlKeyStates modifiers,
                                             const bool focused,
-                                            const til::point pixelPosition)
+                                            const til::point pixelPosition,
+                                            const bool pointerPressedInBounds)
     {
         const til::point terminalPosition = _getTerminalPosition(pixelPosition);
 
@@ -288,7 +289,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         {
             _core->SendMouseEvent(terminalPosition, pointerUpdateKind, modifiers, 0, toInternalMouseState(buttonState));
         }
-        else if (focused && WI_IsFlagSet(buttonState, MouseButtonState::IsLeftButtonDown))
+        // GH#4603 - don't modify the selection if the pointer press didn't
+        // actually start _in_ the control bounds. Case in point - someone drags
+        // a file into the bounds of the control. That shouldn't send the
+        // selection into space.
+        else if (focused && pointerPressedInBounds && WI_IsFlagSet(buttonState, MouseButtonState::IsLeftButtonDown))
         {
             if (_singleClickTouchdownPos)
             {

src/cascadia/TerminalControl/ControlInteractivity.h

@@ -58,7 +58,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                           const unsigned int pointerUpdateKind,
                           const ::Microsoft::Terminal::Core::ControlKeyStates modifiers,
                           const bool focused,
-                          const til::point pixelPosition);
+                          const til::point pixelPosition,
+                          const bool pointerPressedInBounds);
         void TouchMoved(const til::point newTouchPoint,
                         const bool focused);
 

src/cascadia/TerminalControl/ControlInteractivity.idl

@@ -39,7 +39,9 @@ namespace Microsoft.Terminal.Control
                           UInt32 pointerUpdateKind,
                           Microsoft.Terminal.Core.ControlKeyStates modifiers,
                           Boolean focused,
-                          Microsoft.Terminal.Core.Point pixelPosition);
+                          Microsoft.Terminal.Core.Point pixelPosition,
+                          Boolean pointerPressedInBounds);
+
         void TouchMoved(Microsoft.Terminal.Core.Point newTouchPoint,
                         Boolean focused);
 

src/cascadia/TerminalControl/TermControl.cpp

@@ -1052,6 +1052,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             Focus(FocusState::Pointer);
         }
 
+        // Mark that this pointer event actually started within our bounds.
+        // We'll need this later, for PointerMoved events.
+        _pointerPressedInBounds = true;
+
         if (type == Windows::Devices::Input::PointerDeviceType::Touch)
         {
             const auto contactRect = point.Properties().ContactRect();
@@ -1104,10 +1108,19 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                                         TermControl::GetPointerUpdateKind(point),
                                         ControlKeyStates(args.KeyModifiers()),
                                         _focused,
-                                        pixelPosition);
-
-            if (_focused && point.Properties().IsLeftButtonPressed())
+                                        pixelPosition,
+                                        _pointerPressedInBounds);
+
+            // 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())
             {
+                // We want to find the distance relative to the bounds of the
+                // SwapChainPanel, not the entire control. If they drag out of
+                // the bounds of the text, into the padding, we still what that
+                // to auto-scroll
                 const double cursorBelowBottomDist = cursorPosition.Y - SwapChainPanel().Margin().Top - SwapChainPanel().ActualHeight();
                 const double cursorAboveTopDist = -1 * cursorPosition.Y + SwapChainPanel().Margin().Top;
 
@@ -1157,6 +1170,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             return;
         }
 
+        _pointerPressedInBounds = false;
+
         const auto ptr = args.Pointer();
         const auto point = args.GetCurrentPoint(*this);
         const auto cursorPosition = point.Position();

src/cascadia/TerminalControl/TermControl.h

@@ -167,11 +167,13 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         std::shared_ptr<ThrottledFuncTrailing<ScrollBarUpdate>> _updateScrollBar;
         bool _isInternalScrollBarUpdate;
 
-        // Auto scroll occurs when user, while selecting, drags cursor outside viewport. View is then scrolled to 'follow' the cursor.
+        // Auto scroll occurs when user, while selecting, drags cursor outside
+        // viewport. View is then scrolled to 'follow' the cursor.
         double _autoScrollVelocity;
         std::optional<Windows::UI::Input::PointerPoint> _autoScrollingPointerPoint;
         Windows::UI::Xaml::DispatcherTimer _autoScrollTimer;
         std::optional<std::chrono::high_resolution_clock::time_point> _lastAutoScrollUpdateTime;
+        bool _pointerPressedInBounds{ false };
 
         winrt::Windows::UI::Composition::ScalarKeyFrameAnimation _bellLightAnimation{ nullptr };
         Windows::UI::Xaml::DispatcherTimer _bellLightTimer{ nullptr };

src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp

@@ -32,6 +32,9 @@ namespace ControlUnitTests
         TEST_METHOD(ScrollWithSelection);
         TEST_METHOD(TestScrollWithTrackpad);
         TEST_METHOD(TestQuickDragOnSelect);
+
+        TEST_METHOD(TestDragSelectOutsideBounds);
+
         TEST_METHOD(PointerClickOutsideActiveRegion);
 
         TEST_CLASS_SETUP(ClassSetup)
@@ -288,7 +291,8 @@ namespace ControlUnitTests
                                     WM_LBUTTONDOWN, //pointerUpdateKind
                                     modifiers,
                                     true, // focused,
-                                    cursorPosition1);
+                                    cursorPosition1,
+                                    true);
         Log::Comment(L"Verify that there's one selection");
         VERIFY_IS_TRUE(core->HasSelection());
         VERIFY_ARE_EQUAL(1u, core->_terminal->GetSelectionRects().size());
@@ -300,7 +304,8 @@ namespace ControlUnitTests
                                     WM_LBUTTONDOWN, //pointerUpdateKind
                                     modifiers,
                                     true, // focused,
-                                    cursorPosition2);
+                                    cursorPosition2,
+                                    true);
         Log::Comment(L"Verify that there's now two selections (one on each row)");
         VERIFY_IS_TRUE(core->HasSelection());
         VERIFY_ARE_EQUAL(2u, core->_terminal->GetSelectionRects().size());
@@ -333,7 +338,8 @@ namespace ControlUnitTests
                                     WM_LBUTTONDOWN, //pointerUpdateKind
                                     modifiers,
                                     true, // focused,
-                                    cursorPosition4);
+                                    cursorPosition4,
+                                    true);
         Log::Comment(L"Verify that there's now one selection");
         VERIFY_IS_TRUE(core->HasSelection());
         VERIFY_ARE_EQUAL(1u, core->_terminal->GetSelectionRects().size());
@@ -388,7 +394,8 @@ namespace ControlUnitTests
                                     WM_LBUTTONDOWN, //pointerUpdateKind
                                     modifiers,
                                     true, // focused,
-                                    cursorPosition1);
+                                    cursorPosition1,
+                                    true);
         Log::Comment(L"Verify that there's one selection");
         VERIFY_IS_TRUE(core->HasSelection());
         VERIFY_ARE_EQUAL(1u, core->_terminal->GetSelectionRects().size());
@@ -536,14 +543,83 @@ namespace ControlUnitTests
                                     WM_LBUTTONDOWN, //pointerUpdateKind
                                     modifiers,
                                     true, // focused,
-                                    cursorPosition1);
+                                    cursorPosition1,
+                                    true);
+        Log::Comment(L"Verify that there's one selection");
+        VERIFY_IS_TRUE(core->HasSelection());
+        VERIFY_ARE_EQUAL(1u, core->_terminal->GetSelectionRects().size());
+
+        Log::Comment(L"Verify that it started on the first cell we clicked on, not the one we dragged to");
+        COORD expectedAnchor{ 0, 0 };
+        VERIFY_ARE_EQUAL(expectedAnchor, core->_terminal->GetSelectionAnchor());
+    }
+
+    void ControlInteractivityTests::TestDragSelectOutsideBounds()
+    {
+        // This is a test for GH#4603
+
+        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 };
+        Log::Comment(L"Click on the terminal");
+        const til::point cursorPosition0{ 6, 0 };
+        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());
+
+        VERIFY_IS_TRUE(interactivity->_singleClickTouchdownPos.has_value());
+        VERIFY_ARE_EQUAL(cursorPosition0, interactivity->_singleClickTouchdownPos.value());
+
+        Log::Comment(L"Drag the mouse a lot. This simulates dragging the mouse real fast.");
+        const til::point cursorPosition1{ 6 + fontSize.width<int>() * 2, 0 };
+        interactivity->PointerMoved(leftMouseDown,
+                                    WM_LBUTTONDOWN, //pointerUpdateKind
+                                    modifiers,
+                                    true, // focused,
+                                    cursorPosition1,
+                                    true);
         Log::Comment(L"Verify that there's one selection");
         VERIFY_IS_TRUE(core->HasSelection());
         VERIFY_ARE_EQUAL(1u, core->_terminal->GetSelectionRects().size());
 
         Log::Comment(L"Verify that it started on the first cell we clicked on, not the one we dragged to");
         COORD expectedAnchor{ 0, 0 };
         VERIFY_ARE_EQUAL(expectedAnchor, core->_terminal->GetSelectionAnchor());
+        COORD expectedEnd{ 2, 0 };
+        VERIFY_ARE_EQUAL(expectedEnd, core->_terminal->GetSelectionEnd());
+
+        interactivity->PointerReleased(noMouseDown,
+                                       WM_LBUTTONUP,
+                                       modifiers,
+                                       cursorPosition1);
+
+        VERIFY_ARE_EQUAL(expectedAnchor, core->_terminal->GetSelectionAnchor());
+        VERIFY_ARE_EQUAL(expectedEnd, core->_terminal->GetSelectionEnd());
+
+        Log::Comment(L"Simulate dragging the mouse into the control, without first clicking into the control");
+        const til::point cursorPosition2{ fontSize.width<int>() * 10, 0 };
+        interactivity->PointerMoved(leftMouseDown,
+                                    WM_LBUTTONDOWN, //pointerUpdateKind
+                                    modifiers,
+                                    true, // focused,
+                                    cursorPosition2,
+                                    false);
+
+        Log::Comment(L"The selection should be unchanged.");
+        VERIFY_ARE_EQUAL(expectedAnchor, core->_terminal->GetSelectionAnchor());
+        VERIFY_ARE_EQUAL(expectedEnd, core->_terminal->GetSelectionEnd());
     }
 
     void ControlInteractivityTests::PointerClickOutsideActiveRegion()
@@ -561,7 +637,6 @@ namespace ControlUnitTests
         const Control::MouseButtonState noMouseDown{};
 
         const til::size fontSize{ 9, 21 };
-
         interactivity->_rowsToScroll = 1;
         int expectedTop = 0;
         int expectedViewHeight = 20;
@@ -630,7 +705,8 @@ namespace ControlUnitTests
                                     WM_LBUTTONDOWN, //pointerUpdateKind
                                     modifiers,
                                     true, // focused,
-                                    cursorPosition1);
+                                    cursorPosition1,
+                                    true);
         Log::Comment(L"Verify that there's still no selection");
         VERIFY_IS_FALSE(core->HasSelection());
     }