Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make selected text copyable instead of copying password (fix for #7059) #7209

Merged
merged 1 commit into from
Dec 9, 2021

Conversation

mckeema
Copy link
Contributor

@mckeema mckeema commented Dec 7, 2021

Fixes #7059. Checks whether the fingerprint, comment, or public key widgets of the SSH Agent tab in the Edit Entry screen are selected when a copy event occurs. If they are, the relevant selected text is copied, otherwise falls back to the current behavior of copying the password. The clipboard is cleared after the timeout if the usual setting is enabled. Below is a short video demonstration of the new behavior. Previously, none of the text mentioned above could be copied using the copy shortcut. The password would be copied instead.

Screenshots

demo.mp4

Testing strategy

I tested the change manually, as you can see in the video included above. I also ran make test locally and got the same results on my branch as the develop branch.

Type of change

  • ✅ Bug fix (non-breaking change that fixes an issue)

@codecov-commenter
Copy link

codecov-commenter commented Dec 7, 2021

Codecov Report

Attention: Patch coverage is 68.75000% with 5 lines in your changes missing coverage. Please review.

Project coverage is 64.08%. Comparing base (a0a063b) to head (3776784).
Report is 549 commits behind head on develop.

Files with missing lines Patch % Lines
src/gui/DatabaseWidget.cpp 68.75% 5 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #7209      +/-   ##
===========================================
+ Coverage    64.07%   64.08%   +0.01%     
===========================================
  Files          335      335              
  Lines        42309    42322      +13     
===========================================
+ Hits         27108    27119      +11     
- Misses       15201    15203       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@droidmonkey
Copy link
Member

This is not the desired approach to this and is extremely fragile to name changes in the UI file. Additionally, all text input fields and labels suffer from this bug, even on the preview widget. Remove the checks for the widget names and add a QTextEdit* check as well.

@mckeema
Copy link
Contributor Author

mckeema commented Dec 8, 2021

Thanks for the pointers. No screenshots because the behavior hasn't changed from the original video. I removed the object name checks and replaced them with checks for QLabel, QTextEdit, and QPlainTextEdit. If any text is highlighted in any of those three widget types, it is copied instead of the password. Is this closer to the desired behavior/approach?

@droidmonkey
Copy link
Member

Yah that is perfect!

@droidmonkey droidmonkey added this to the v2.7.0 milestone Dec 8, 2021
@droidmonkey droidmonkey changed the title Make SSH Agent fingerprint, comment, and public key text selectable and copyable (fix for #7059) Make selected text copyable instead of copying password (fix for #7059) Dec 8, 2021
@droidmonkey droidmonkey merged commit 6c4a82b into keepassxreboot:develop Dec 9, 2021
@phoerious phoerious added pr: bugfix Pull request that fixes a bug and removed bug labels Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: bugfix Pull request that fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Selecting the Public key text, and ctrl-c leads to the password being copied
4 participants