Fixes #4603

Kinda the same thing. Don't modify the selection if the pointer down drag didn't even start in the control to begin with.
microsoft · Jul 13, 2021 · 2aaae9e · 2aaae9e
1 parent 3d30295
commit 2aaae9e
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 10 deletions.
diff --git a/src/cascadia/TerminalControl/ControlInteractivity.cpp b/src/cascadia/TerminalControl/ControlInteractivity.cpp
@@ -274,7 +274,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);
 
@@ -283,7 +284,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)
             {

diff --git a/src/cascadia/TerminalControl/ControlInteractivity.h b/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);
 

diff --git a/src/cascadia/TerminalControl/ControlInteractivity.idl b/src/cascadia/TerminalControl/ControlInteractivity.idl
@@ -39,7 +39,8 @@ 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);
 

diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp
@@ -1105,7 +1105,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                                         TermControl::GetPointerUpdateKind(point),
                                         ControlKeyStates(args.KeyModifiers()),
                                         _focused,
-                                        pixelPosition);
+                                        pixelPosition,
+                                        _pointerPressedInBounds);
 
             // GH#9109 - Only start an auto-scroll when the drag actually
             // started within our bounds. Otherwise, someone could start a drag

diff --git a/src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp b/src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp
@@ -287,7 +287,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());
@@ -299,7 +300,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());
@@ -332,7 +334,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());
@@ -387,7 +390,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());
@@ -535,7 +539,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());