Skip to content

Fix copyOnSelect right-click overwriting clipboard with stale selection#19943

Open
sagarbhure-msft wants to merge 2 commits intomicrosoft:mainfrom
sagarbhure-msft:fix/copyOnSelect-right-click
Open

Fix copyOnSelect right-click overwriting clipboard with stale selection#19943
sagarbhure-msft wants to merge 2 commits intomicrosoft:mainfrom
sagarbhure-msft:fix/copyOnSelect-right-click

Conversation

@sagarbhure-msft
Copy link
Contributor

Summary of the Pull Request

Fix copyOnSelect right-click paste overwriting the clipboard with a stale selection instead of pasting the current clipboard contents.

References and Relevant Issues

Closes #19942

Detailed Description of the Pull Request / Additional comments

When copyOnSelect is enabled, the selected text is already copied to the clipboard on left mouse button release in PointerReleased. The selection is intentionally left visible as a visual reference (the _selectionNeedsToBeCopied flag is set to false to track that the copy already happened).

However, the right-click handler in PointerPressed unconditionally called CopySelectionToClipboard() before pasting, ignoring both the copyOnSelect setting and the _selectionNeedsToBeCopied flag. This caused any still-visible selection to be re-copied to the clipboard, overwriting whatever the user may have copied from another application (or another terminal tab) in the meantime.

This change splits the right-click behavior based on the copyOnSelect setting:

  • copyOnSelect: true — Skip the redundant copy, clear the selection, and paste directly. The text was already copied on mouse-up.
  • copyOnSelect: false — Preserve existing behavior: copy the selection (if any), clear it, and paste only if there was no selection to copy.

Validation Steps Performed

  1. Set "copyOnSelect": true in settings.json.
  2. Selected text in a terminal pane - verified it was copied to clipboard on mouse-up.
  3. Switched to Notepad, copied different text ("NEW TEXT").
  4. Switched back to Terminal (selection still visible), right-clicked — verified "NEW TEXT" was pasted, not the old selection.
  5. Verified right-click with no active selection still pastes clipboard contents.
  6. Verified right-click immediately after selecting (no app switch) pastes the just-selected text.
  7. Set "copyOnSelect": false — verified right-click still copies selection first, then pastes only when no selection exists (original behavior unchanged).
  8. Verified tab-switching with an active selection does not cause stale clipboard overwrites on right-click.

PR Checklist

@sagarbhure-msft
Copy link
Contributor Author

@microsoft-github-policy-service agree

// no selection --> paste
RequestPasteTextFromClipboard();
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be shortened IMO:

// GH#19942: With copyOnSelect the selection was already copied when the
// user released the left mouse button. Don't re-copy it on right-click.
// Otherwise, we copy the selection if any.
const auto paste = _core->CopyOnSelect() || !CopySelectionToClipboard(shiftEnabled, false, _core->Settings().CopyFormatting());

_core->ClearSelection();

// With copyOnSelect any right click turns into a paste.
// Otherwise, only if there was no selection to copy, it's a paste.
if (paste)
{
    RequestPasteTextFromClipboard();
}

@zadjii-msft
Copy link
Member

Reviewers may want to x-ref this much much older PR of mine:
#14635

Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the x-ref Mike!

Yeah, the same problem from #14635 happens. Blocking over that.

Bug:

  1. ensure copy on select is enabled
  2. enter mark mode
  3. make a selection with mark mode
  4. right-click to copy/paste

expected: selection is copied and pasted immediately

NOTE: similar thing happens when you make a mouse selection, then modify it using quick-edit keys (i.e. shift+right). The selected text is not copied and pasted immediately.

from #14635 (review)

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Mar 11, 2026
@carlos-zamora
Copy link
Member

Root cause from a chat with GH Copilot:

The root cause: _selectionNeedsToBeCopied is only set to true in SetEndSelectionPoint() (mouse drag path, line 699). Mark mode selections go through TryMarkModeKeybinding →
UpdateSelection() → _updateSelectionUI(), which never sets that flag. So for mark mode, the right-click copy was the only opportunity to copy the selection.

@sagarbhure-msft
Copy link
Contributor Author

Thanks for the x-ref Mike!

Yeah, the same problem from #14635 happens. Blocking over that.

Bug:

  1. ensure copy on select is enabled
  2. enter mark mode
  3. make a selection with mark mode
  4. right-click to copy/paste

expected: selection is copied and pasted immediately
NOTE: similar thing happens when you make a mouse selection, then modify it using quick-edit keys (i.e. shift+right). The selected text is not copied and pasted immediately.

from #14635 (review)

ack, looking into this...

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Mar 12, 2026
Fixes the mark-mode and quick-edit (shift+arrow) bug identified in
PR microsoft#14635's review: _selectionNeedsToBeCopied was only set in
SetEndSelectionPoint() (mouse drag), so keyboard-driven selections
were incorrectly treated as already-copied on right-click.

Fix: subscribe to ControlCore::UpdateSelectionMarkers in the
ControlInteractivity constructor so the flag is set for ALL
selection types (mouse, mark mode, quick-edit, select-all).

The right-click handler now uses the same gating logic from PR microsoft#14635
(_selectionNeedsToBeCopied || !copyOnSelect) to decide whether to
copy, and always pastes in copyOnSelect mode.

Also collapses the if/else into a compact conditional per lhecker's
review suggestion.

Closes microsoft#14464
Fixes microsoft#19942
@sagarbhure-msft
Copy link
Contributor Author

Pushed a follow-up commit addressing all review feedback:

  • @lhecker : collapsed the if/else into a compact conditional as suggested.
  • @carlos-zamora : fixed the mark-mode and quick-edit (shift+arrow) bug from Don't copy before pasting in copyOnSelect mode #14635. Root cause was _selectionNeedsToBeCopied only being set in SetEndSelectionPoint() (mouse drag path). Fix: ControlInteractivity now subscribes to UpdateSelectionMarkers so the flag is set for all selection types. The right-click handler uses _selectionNeedsToBeCopied || !copyOnSelect to gate the copy, so mark-mode selections are correctly copied and pasted on right-click

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs-Attention The core contributors need to come back around and look at this ASAP.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Right click paste also does a copy with copyOnSelect

4 participants