Fix Copy Password button when text is selected #10853
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Love it, glad to use the new action collection too! I really couldn't figure out a fix to this, too focused on the toolbar action and not the keyboard shortcut. |
c4rlo
force-pushed
the
fix-copy-password-sel
branch
6 times, most recently
from
03f7a5a
to
3830953
Compare
c4rlo
changed the title
c4rlo
force-pushed
the
fix-copy-password-sel
branch
from
3830953
to
bb4b216
Compare
c4rlo
commented
Jun 7, 2024
|
I actually have a slightly simpler approach now, based on |
c4rlo
force-pushed
the
fix-copy-password-sel
branch
from
bb4b216
to
73edded
Compare
c4rlo
changed the title
c4rlo
force-pushed
the
fix-copy-password-sel
branch
4 times, most recently
from
c602efc
to
07a279b
Compare
|
Ok, this is ready for review now. I've undone all the refactoring and kept this more focused. |
|
The macOS CI failure seems like some kind of test flakeyness, unrelated to this change. I have created #10901 to address that. |
When the user chooses to copy the password for an entry to the clipboard, previously there was logic to check if text was selected, and if so, that text was instead copied to the clipboard. That made sense if (a) the user invoked the Copy Password action via its keyboard shortcut, and (b) that keyboard shortcut was configured (as per default) to be Ctrl-C, i.e. the same as the system action for copy-to-clipboard. However, it made no sense if the user invoked that action in some other way, for example by clicking the corresponding toolbar button. It also made no sense in the case that the Copy Password action had some other keyboard shortcut assigned. Also, if some other action had Ctrl-C assigned, the logic would not kick in then. Fix all of the above by modifying the keyboard shortcut logic to intervene precisely in the case where a shortcut is pressed that matches the system copy-to-clipboard shortcut; only in that case do we now check if text is selected and if so copy that to the clipboard instead of the action we would otherwise take. Add a test case to TestGui.cpp. Fixes keepassxreboot#10734.
c4rlo
force-pushed
the
fix-copy-password-sel
branch
from
07a279b
to
f1079a8
Compare
droidmonkey
approved these changes
Jun 19, 2024
libf-de
pushed a commit
to libf-de/keepassxc-secretservice-dbus
that referenced
this pull request
Jun 20, 2024
Release 2.7.9 * Passkeys: Ability to easily remove a passkey from an entry [keepassxreboot#10777] * Snap: Use new desktop portal for native messaging integration [keepassxreboot#10906] * Improve entry placeholder/reference feature [keepassxreboot#10846] * Improve CSV importing when title field isn't specified [keepassxreboot#10843] * Improve encrypted Bitwarden importing [keepassxreboot#10800] * Improve database settings UX [keepassxreboot#10821] * Improve handling of clipboard actions from entry preview [keepassxreboot#10810] * Improve group/entry view resize behavior and set sensible defaults [keepassxreboot#10641] * Passkeys: Fix incorrect username fill [keepassxreboot#10874] * Passkeys: Return additional data to the extension [keepassxreboot#10857] * Fix password clear timer inconsistency on unlock view [keepassxreboot#10708] * Fix portability check [keepassxreboot#10760] * Fix page overflow on HTML exports [keepassxreboot#10735] * Fix broken builds when using system provided zxcvbn [keepassxreboot#10717] * Fix copy password button when text is selected [keepassxreboot#10853] * Fix tab ordering on application settings pages [keepassxreboot#10907] * SSH Agent: Fix broken decrypt button [keepassxreboot#10638] * Windows: Fix ALT Auto-Type modifier [keepassxreboot#10795] * Windows: Fix wrong DACL memory size allocation [keepassxreboot#10712] * macOS: Fix monospace font sizing [keepassxreboot#10739] * Flatpak: Fix configuration settings off-by-one error [keepassxreboot#10688] * BSD: Fix compiling with libusb implementation [keepassxreboot#10736] # -----BEGIN PGP SIGNATURE----- # # iQEzBAABCAAdFiEENIkEDB8MPuq41ValRA/GXy4MbgEFAmZzTogACgkQRA/GXy4M # bgHahggAg+hzMTiM0uDaw5yfxhv6GEfQQBPHMhX3JDyHEC+i7Pq6OjlxQkdUrRdu # f4w74od5jSul0Al/ehu9L2eZwNPMnU87FWDn16o1btYHsG9n24v5S0DuQoLXUjde # Y9nJNKeRNoWAlVKWbUG2YGvy9hF9YbtrFaiBksaQ+g3w8Xz82PzLY0VaUu4Xa/LO # RXAhryJC+8T3T479dXpHxJcUmEWkoY4bqj1i6R8tEK5Kz9y1c0kqzqwWysKMj+rD # WxTb2V4y9s57pO35zt9yxMLg66xx9bdcQHbSULa2vZNMFd9qdqk8WJmWFle112yG # UCBXv2ZIjd3lghPt0IrD+WKcuL85Aw== # =rbfs # -----END PGP SIGNATURE----- # gpg: directory '/home/runner/.gnupg' created # gpg: keybox '/home/runner/.gnupg/pubring.kbx' created # gpg: Signature made Wed Jun 19 21:32:56 2024 UTC # gpg: using RSA key 3489040C1F0C3EEAB8D556A5440FC65F2E0C6E01 # gpg: Can't check signature: No public key
Perlover
added a commit
to Perlover/keepassxc
that referenced
this pull request
Jul 24, 2024
Release 2.7.9 * Passkeys: Ability to easily remove a passkey from an entry [keepassxreboot#10777] * Snap: Use new desktop portal for native messaging integration [keepassxreboot#10906] * Improve entry placeholder/reference feature [keepassxreboot#10846] * Improve CSV importing when title field isn't specified [keepassxreboot#10843] * Improve encrypted Bitwarden importing [keepassxreboot#10800] * Improve database settings UX [keepassxreboot#10821] * Improve handling of clipboard actions from entry preview [keepassxreboot#10810] * Improve group/entry view resize behavior and set sensible defaults [keepassxreboot#10641] * Passkeys: Fix incorrect username fill [keepassxreboot#10874] * Passkeys: Return additional data to the extension [keepassxreboot#10857] * Fix password clear timer inconsistency on unlock view [keepassxreboot#10708] * Fix portability check [keepassxreboot#10760] * Fix page overflow on HTML exports [keepassxreboot#10735] * Fix broken builds when using system provided zxcvbn [keepassxreboot#10717] * Fix copy password button when text is selected [keepassxreboot#10853] * Fix tab ordering on application settings pages [keepassxreboot#10907] * SSH Agent: Fix broken decrypt button [keepassxreboot#10638] * Windows: Fix ALT Auto-Type modifier [keepassxreboot#10795] * Windows: Fix wrong DACL memory size allocation [keepassxreboot#10712] * macOS: Fix monospace font sizing [keepassxreboot#10739] * Flatpak: Fix configuration settings off-by-one error [keepassxreboot#10688] * BSD: Fix compiling with libusb implementation [keepassxreboot#10736]
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When the user chooses to copy the password for an entry to the clipboard, previously there was logic to check if text was selected, and if so, that text was instead copied to the clipboard. That made sense if (a) the user invoked the Copy Password action via its keyboard shortcut, and (b) that keyboard shortcut was configured (as per default) to be Ctrl-C, i.e. the same as the system action for copy-to-clipboard.
However, it made no sense if the user invoked that action in some other way, for example by clicking the corresponding toolbar button.
It also made no sense in the case that the Copy Password action had some other keyboard shortcut assigned. Also, if some other action had Ctrl-C assigned, the logic would not kick in then.
Fix all of the above by modifying the keyboard shortcut logic to intervene precisely in the case where a shortcut is pressed that matches the system copy-to-clipboard shortcut; only in that case do we now check if text is selected and if so copy that to the clipboard instead of the action we would otherwise take.
Fixes #10734.
Testing strategy
Added test to
TestGui.cpp. Also confirmed via manual testing.Type of change