Improve confirm access dialog for browser plugin #2143
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
varjolintu
force-pushed
the
confirm_dialog_improvement
branch
from
dc20748 to
d0be8cd
Compare
droidmonkey
previously requested changes
Aug 20, 2018
There was a problem hiding this comment.
The whole dialog needs to be rethinked. In fact, with this change the meaning of "Approve" and "Deny" change depending on your selections. It is not entirely obvious to the user what will happen. When you "Deny" selected entries you are implicitly approving non-selected entries. That is a little weird to me.
I think we should discuss this function, and move this to 2.4
src/browser/BrowserService.cpp
Outdated
| } | ||
|
|
||
| return false; | ||
| return (res == QDialog::Accepted) ? selectedEntriesList : nonSelectedEntriesList;; |
There was a problem hiding this comment.
Remove extra semi-colon. Why would you return the non-selected entries??
edit: answered my own question in the general comment, non selected entries are implicitly approved when "Deny" is selected.
There was a problem hiding this comment.
Approved yes, but the setting is not remembered. What would you suggest that will happen when user has e.g. three entries and denies only one of them?
varjolintu
force-pushed
the
confirm_dialog_improvement
branch
from
d0be8cd to
8b96f5c
Compare
droidmonkey
force-pushed
the
release/2.3.4
branch
from
4264ed6 to
d6cae74
Compare
|
Don't know why this was closed, reopened. |
droidmonkey
changed the title
phoerious
previously requested changes
Sep 24, 2018
varjolintu
force-pushed
the
confirm_dialog_improvement
branch
from
8b96f5c to
5286795
Compare
varjolintu
force-pushed
the
confirm_dialog_improvement
branch
from
5286795 to
81c1f34
Compare
varjolintu
force-pushed
the
confirm_dialog_improvement
branch
from
81c1f34 to
52ab8bc
Compare
|
@varjolintu do you want to keep this PR or start over from scratch? I believe this needs to be addressed along with the UX updates on the extension side. |
|
@droidmonkey I'd like to keep this PR. I'm going to rebase it soon and see if everything still works. |
varjolintu
force-pushed
the
confirm_dialog_improvement
branch
from
52ab8bc to
19b0f38
Compare
|
Rebased. |
droidmonkey
force-pushed
the
confirm_dialog_improvement
branch
from
19b0f38 to
0e2ab0a
Compare
droidmonkey
force-pushed
the
confirm_dialog_improvement
branch
from
0e2ab0a to
85fd6e5
Compare
droidmonkey
force-pushed
the
confirm_dialog_improvement
branch
from
85fd6e5 to
ab6daf2
Compare
|
Made significant edits to the workflow (as a new commit):
|
droidmonkey
force-pushed
the
confirm_dialog_improvement
branch
2 times, most recently
from
68164d0 to
586f097
Compare
droidmonkey
approved these changes
Jan 5, 2020
droidmonkey
force-pushed
the
confirm_dialog_improvement
branch
from
586f097 to
1e1a444
Compare
* Disable access to entries immediately within the dialog * Use checkboxes instead of row selection * Add button to deny all access immediately
droidmonkey
force-pushed
the
confirm_dialog_improvement
branch
from
1e1a444 to
1ebdf13
Compare
droidmonkey
added a commit
that referenced
this pull request
Jul 7, 2020
Added - Custom Light and Dark themes [#4110, #4769, #4791, #4796, #4892, #4915] - Compact mode to use classic Group and Entry line height [#4910] - View menu to quickly switch themes, compact mode, and toggle UI elements [#4910] - Search for groups and scope search to matched groups [#4705] - Save Database Backup feature [#4550] - Sort entries by "natural order" and move lines up/down [#4357] - Option to launch KeePassXC on system startup/login [#4675] - Caps Lock warning on password input fields [#3646] - Add "Size" column to entry view [#4588] - Browser-like tab experience using Ctrl+[Num] (Alt+[Num] on Linux) [#4063, #4305] - Password Generator: Define additional characters to choose from [#3876] - Reports: Database password health check (offline) [#3993] - Reports: HIBP online service to check for breached passwords [#4438] - Auto-Type: DateTime placeholders [#4409] - Browser: Show group name in results sent to browser extension [#4111] - Browser: Ability to define a custom browser location (macOS and Linux only) [#4148] - Browser: Ability to change root group UUID and inline edit connection ID [#4315, #4591] - CLI: `db-info` command [#4231] - CLI: Use wl-clipboard if xclip is not available (Linux) [#4323] - CLI: Incorporate xclip into snap builds [#4697] - SSH Agent: Key file path env substitution, SSH_AUTH_SOCK override, and connection test [#3769, #3801, #4545] - SSH Agent: Context menu actions to add/remove keys [#4290] Changed - Complete replacement of default database icons [#4699] - Complete replacement of application icons [#4066, #4161, #4203, #4411] - Complete rewrite of documentation and manpages using Asciidoctor [#4937] - Complete refactor of config files; separate between local and roaming [#4665] - Complete refactor of browser integration and proxy code [#4680] - Complete refactor of hardware key integration (YubiKey and OnlyKey) [#4584, #4843] - Significantly improve performance when saving and opening databases [#4309, #4833] - Remove read-only detection for database files [#4508] - Overhaul of password fields and password generator [#4367] - Replace instances of "Master Key" with "Database Credentials" [#4929] - Change settings checkboxes to positive phrasing for consistency [#4715] - Improve UX of using entry actions (focus fix) [#3893] - Set expiration time to Now when enabling entry expiration [#4406] - Always show "New Entry" in context menu [#4617] - Issue warning before adding large attachments [#4651] - Improve importing OPVault [#4630] - Improve AutoOpen capability [#3901, #4752] - Check for updates every 7 days even while still running [#4752] - Improve Windows installer UI/UX [#4675] - Improve config file handling of portable distribution [#4131, #4752] - macOS: Hide dock icon when application is hidden to tray [#4782] - Browser: Use unlock dialog to improve UX of opening a locked database [#3698] - Browser: Improve database and entry settings experience [#4392, #4591] - Browser: Improve confirm access dialog [#2143, #4660] - KeeShare: Improve monitoring file changes of shares [#4720] - CLI: Rename `create` command to `db-create` [#4231] - CLI: Cleanup `db-create` options (`--set-key-file` and `--set-password`) [#4313] - CLI: Use stderr for help text and password prompts [#4086, #4623] - FdoSecrets: Display existing secret service process [#4128] Fixed - Fix changing focus around the main window using tab key [#4641] - Fix search field clearing while still using the application [#4368] - Improve search help widget displaying on macOS and Linux [#4236] - Return keyboard focus after editing an entry [#4287] - Reset database path after failed "Save As" [#4526] - Use SHA256 Digest for Windows code signing [#4129] - Improve handling of ccache when building [#4104, #4335] - macOS: Properly re-hide application window after browser integration and Auto-Type usage [#4909] - Auto-Type: Fix crash when performing on new entry [#4132] - Browser: Send legacy HTTP settings to recycle bin [#4589] - Browser: Fix merging browser keys [#4685] - CLI: Fix encoding when exporting database [#3921] - SSH Agent: Improve reliability and underlying code [#3833, #4256, #4549, #4595] - FdoSecrets: Fix crash when editing settings before service is enabled [#4332]
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.
Improves the confirm access dialog when requesting credentials from KeePassXC-Browser.
Description
When confirm access dialog is displayed it's possible to select one or more credentials which are allowed or denied. All credentials are selected by default. Only the selected credentials are affected with the remember checkbox. Permissions to non-selected credentials will remain the same.
Motivation and context
Previously you could select one credential from the popup but the selection didn't actually do anything. All credentials were still allowed or denied.
How has this been tested?
Manually.
Types of changes
Checklist:
-DWITH_ASAN=ON. [REQUIRED]