From ca658ba331ef641830dabe57f99198e2857197b1 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 5 Jan 2023 14:19:28 -0600 Subject: [PATCH] Fix the bug --- .../TerminalControl/ControlInteractivity.cpp | 17 ++++++++++++++--- .../ControlInteractivityTests.cpp | 6 ++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/cascadia/TerminalControl/ControlInteractivity.cpp b/src/cascadia/TerminalControl/ControlInteractivity.cpp index 7eaf4275535..8521d6e835c 100644 --- a/src/cascadia/TerminalControl/ControlInteractivity.cpp +++ b/src/cascadia/TerminalControl/ControlInteractivity.cpp @@ -255,10 +255,21 @@ namespace winrt::Microsoft::Terminal::Control::implementation } else if (WI_IsFlagSet(buttonState, MouseButtonState::IsRightButtonDown)) { - // Try to copy the text and clear the selection - const auto successfulCopy = CopySelectionToClipboard(shiftEnabled, nullptr); + const auto copyOnSelect{ _core->CopyOnSelect() }; + bool successfulCopy = false; + + // Don't try to copy if we're in copyOnSelect mode and have already + // copied this selection. GH#14464 demonstrates a scenario where the + // buffer contents might have changed since the selection was made, + // and copying here would cause weirdness. + if (_selectionNeedsToBeCopied || !copyOnSelect) + { + // Try to copy the text and clear the selection + successfulCopy = CopySelectionToClipboard(shiftEnabled, nullptr); + } _core->ClearSelection(); - if (_core->CopyOnSelect() || !successfulCopy) + + if (copyOnSelect || !successfulCopy) { // CopyOnSelect: right click always pastes! // Otherwise: no selection --> paste diff --git a/src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp b/src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp index abd2ca30837..f9b018f4d62 100644 --- a/src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp +++ b/src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp @@ -1041,6 +1041,9 @@ namespace ControlUnitTests VERIFY_IS_TRUE(core->HasSelection()); Log::Comment(L" --- Right-click to paste --- "); + // Note from GH#14464: we don't want to copy _again_ at this point. The + // copy occured when the selection was made, we shouldn't stealth-update + // the clipboard again. expectedCopyContents = std::nullopt; expectedPaste = true; interactivity->PointerPressed(rightMouseDown, @@ -1053,6 +1056,9 @@ namespace ControlUnitTests void ControlInteractivityTests::CopyOnSelectAltBuffer() { + // This test was inspired by GH#14464. Ultimately, it's similar to the + // CopyOnSelectSimple, just with an alt buffer, and outputting text + // after the selection was made. auto [settings, conn] = _createSettingsAndConnection(); settings->CopyOnSelect(true); auto [core, interactivity] = _createCoreAndInteractivity(*settings, *conn);