Skip to content

Commit

Permalink
Ignore right-click copy when copy on select is enabled (microsoft#4819)
Browse files Browse the repository at this point in the history
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
Right clicking on a focused tab while Copy On Select is active currently copies any active selection. This is because `PointerReleasedHandler` doesn't check for the mouse button that was released. 
During a mouse button release, only the left mouse button release should be doing anything.

<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes microsoft#4740
* [x] CLA signed.
* [x] Tests added/passed

<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
These are the scenarios I've tested. They're a combination of in focus/out of focus, Copy On Select on/off, left/right click pressed and their move and release variants.

From Out of Focus:
- Left Click = Focus
- Left Click Move = Focus + Selection
- Left Click Release
  - CoS on = Copy
  - CoS off = Nothing
- Shift Left Click = Focus
- Right Click 
  - Focus 
  - CoS on = Paste
  - CoS off = Copy if Active Selection, Paste if not.
- Right Click Move = Nothing
- Right Click Release = Nothing

From In Focus
- Left Click = Selection if CoS off
- Left Click Move = Selection
- Left Click Release
  - CoS on = Copy
  - CoS off = Nothing
- Shift Left Click = Set Selection Anchor
- Right Click 
  - CoS on = Paste
  - CoS off = Copy if Active Selection, Paste if not.
- Right Click Move = Nothing
- Right Click Release = Nothing
  • Loading branch information
leonMSFT authored and abhijeetviswam committed Mar 12, 2020
1 parent 88460c9 commit 1c52585
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 42 deletions.
72 changes: 33 additions & 39 deletions src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,11 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
_actualFont{ DEFAULT_FONT_FACE, 0, 10, { 0, DEFAULT_FONT_SIZE }, CP_UTF8, false },
_touchAnchor{ std::nullopt },
_cursorTimer{},
_lastMouseClick{},
_lastMouseClickTimestamp{},
_lastMouseClickPos{},
_searchBox{ nullptr },
_unfocusedClickPos{ std::nullopt },
_isClickDragSelection{ false }
_focusRaisedClickPos{ std::nullopt },
_clickDrag{ false }
{
_EnsureStaticInitialization();
InitializeComponent();
Expand Down Expand Up @@ -827,13 +827,9 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
if (!_focused)
{
Focus(FocusState::Pointer);

// Save the click position here when the terminal does not have focus
// because they might be performing a click-drag selection. Since we
// only want to start the selection when the user moves the pointer with
// the left mouse button held down, the PointerMovedHandler will use
// this saved position to set the SelectionAnchor.
_unfocusedClickPos = point.Position();
// Save the click position that brought this control into focus
// in case the user wants to perform a click-drag selection.
_focusRaisedClickPos = point.Position();
}

if (ptr.PointerDeviceType() == Windows::Devices::Input::PointerDeviceType::Mouse || ptr.PointerDeviceType() == Windows::Devices::Input::PointerDeviceType::Pen)
Expand All @@ -846,11 +842,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation

if (point.Properties().IsLeftButtonPressed())
{
// _unfocusedClickPos having a value signifies to us that
// the user clicked on an unfocused terminal. We don't want
// a single left click from out of focus to start a selection,
// so we return fast here.
if (_unfocusedClickPos)
// A single left click from out of focus should only focus.
if (_focusRaisedClickPos)
{
args.Handled(true);
return;
Expand Down Expand Up @@ -885,18 +878,17 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
}
else
{
// save location before rendering
_terminal->SetSelectionAnchor(terminalPosition);
}

_lastMouseClick = point.Timestamp();
_lastMouseClickTimestamp = point.Timestamp();
_lastMouseClickPos = cursorPosition;
}
_renderer->TriggerSelection();
}
else if (point.Properties().IsRightButtonPressed())
{
// copyOnSelect causes right-click to always paste
// CopyOnSelect right click always pastes
if (_terminal->IsCopyOnSelectActive() || !_terminal->IsSelectionActive())
{
PasteTextFromClipboard();
Expand Down Expand Up @@ -932,15 +924,13 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
{
if (point.Properties().IsLeftButtonPressed())
{
_isClickDragSelection = true;
_clickDrag = true;

// If this does not have a value, it means that PointerPressedHandler already
// set the SelectionAnchor. If it does have a value, that means the user is
// performing a click-drag selection on an unfocused terminal, so
// a SelectionAnchor isn't set yet. We'll have to set it here.
if (_unfocusedClickPos)
// PointerPressedHandler doesn't set the SelectionAnchor when the click was
// from out of focus, so PointerMoved has to set it.
if (_focusRaisedClickPos)
{
_terminal->SetSelectionAnchor(_GetTerminalPosition(*_unfocusedClickPos));
_terminal->SetSelectionAnchor(_GetTerminalPosition(*_focusRaisedClickPos));
}

const auto cursorPosition = point.Position();
Expand Down Expand Up @@ -1011,22 +1001,26 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
void TermControl::_PointerReleasedHandler(Windows::Foundation::IInspectable const& sender,
Input::PointerRoutedEventArgs const& args)
{
_ReleasePointerCapture(sender, args);

const auto ptr = args.Pointer();
const auto point = args.GetCurrentPoint(*this);

_ReleasePointerCapture(sender, args);

if (ptr.PointerDeviceType() == Windows::Devices::Input::PointerDeviceType::Mouse || ptr.PointerDeviceType() == Windows::Devices::Input::PointerDeviceType::Pen)
{
const auto modifiers = static_cast<uint32_t>(args.KeyModifiers());
// static_cast to a uint32_t because we can't use the WI_IsFlagSet
// macro directly with a VirtualKeyModifiers
const auto shiftEnabled = WI_IsFlagSet(modifiers, static_cast<uint32_t>(VirtualKeyModifiers::Shift));

if (_terminal->IsCopyOnSelectActive())
// Only a left click release when copy on select is active should perform a copy.
// Right clicks and middle clicks should not need to do anything when released.
if (_terminal->IsCopyOnSelectActive() && point.Properties().PointerUpdateKind() == Windows::UI::Input::PointerUpdateKind::LeftButtonReleased)
{
// If the terminal was in focus, copy to clipboard.
// If the terminal was unfocused AND a click-drag selection happened, copy to clipboard.
if (!_unfocusedClickPos || (_unfocusedClickPos && _isClickDragSelection))
const auto modifiers = static_cast<uint32_t>(args.KeyModifiers());
// static_cast to a uint32_t because we can't use the WI_IsFlagSet
// macro directly with a VirtualKeyModifiers
const auto shiftEnabled = WI_IsFlagSet(modifiers, static_cast<uint32_t>(VirtualKeyModifiers::Shift));

// In a Copy on Select scenario,
// All left click drags should copy,
// All left clicks on a focused control should copy if a selection is active.
if (_clickDrag || !_focusRaisedClickPos)
{
CopySelectionToClipboard(shiftEnabled);
}
Expand All @@ -1037,8 +1031,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
_touchAnchor = std::nullopt;
}

_unfocusedClickPos = std::nullopt;
_isClickDragSelection = false;
_focusRaisedClickPos = std::nullopt;
_clickDrag = false;

_TryStopAutoScroll(ptr.PointerId());

Expand Down Expand Up @@ -2110,7 +2104,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
{
// if click occurred at a different location or past the multiClickTimer...
Timestamp delta;
THROW_IF_FAILED(UInt64Sub(clickTime, _lastMouseClick, &delta));
THROW_IF_FAILED(UInt64Sub(clickTime, _lastMouseClickTimestamp, &delta));
if (clickPos != _lastMouseClickPos || delta > _multiClickTimer)
{
// exit early. This is a single click.
Expand Down
7 changes: 4 additions & 3 deletions src/cascadia/TerminalControl/TermControl.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,11 +155,12 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
// imported from WinUser
// Used for PointerPoint.Timestamp Property (https://docs.microsoft.com/en-us/uwp/api/windows.ui.input.pointerpoint.timestamp#Windows_UI_Input_PointerPoint_Timestamp)
Timestamp _multiClickTimer;
Timestamp _lastMouseClick;
Timestamp _lastMouseClickTimestamp;
unsigned int _multiClickCounter;
std::optional<winrt::Windows::Foundation::Point> _lastMouseClickPos;
std::optional<winrt::Windows::Foundation::Point> _unfocusedClickPos;
bool _isClickDragSelection;

std::optional<winrt::Windows::Foundation::Point> _focusRaisedClickPos;
bool _clickDrag;

winrt::Windows::UI::Xaml::Controls::SwapChainPanel::LayoutUpdated_revoker _layoutUpdatedRevoker;

Expand Down

0 comments on commit 1c52585

Please sign in to comment.