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

Refactor DatabaseOpenWidget/Dialog and Auto-Type Database unlocking #2506

Merged
merged 1 commit into from
Nov 24, 2018

Conversation

phoerious
Copy link
Member

@phoerious phoerious commented Nov 24, 2018

Description

This patch removes redundant lock widget members of the DatabaseWidget and consolidates all unlocking functionality into a single DatabaseOpenWidget (with the exception of KeePass1OpenWidget).
Distinction between different unlock actions is now done via a dedicated Intent enum class instead of using individual widgets.

Further, the DatabaseUnlockDialog has been generalized so that it is usable for unlock intents other than just Auto-Type and is now also used for merging databases which is less confusing to the user.

The KeePassXC main window is no longer a parent of the DatabaseUnlockDialog and has the Qt::ForeignWindow flag set, which should cause fewer issues with Auto-Type trying to type into KeePassXC after unlock instead of the intended target window.

In addition, its instance has been moved into the DatabaseTabWidget class so that it is no longer bound to individual DatabaseWidgets, potentially allowing for database selection during Auto-Type. The actual
selection has not yet been implemented, but Auto-Type has been adjusted to use the currently selected tab instead of the first one as an intermediary improvement.

How has this been tested?

Tests pass. Auto-Type works as expected (and better than before even). Automatic relocking after Auto-Type works as well and so does merging in another KDBX and importing CSV or KDB.

Types of changes

  • ✅ Refactor

Checklist:

  • ✅ I have read the CONTRIBUTING document. [REQUIRED]
  • ✅ My code follows the code style of this project. [REQUIRED]
  • ✅ All new and existing tests passed. [REQUIRED]
  • ✅ I have compiled and verified my code with -DWITH_ASAN=ON. [REQUIRED]

@phoerious phoerious added the pr: refactoring Pull request that refactors code label Nov 24, 2018
@phoerious phoerious added this to the v2.4.0 milestone Nov 24, 2018
@phoerious phoerious force-pushed the feature/refactor-databaseopenwidget branch 5 times, most recently from 507447c to 39ffcee Compare November 24, 2018 01:08
This patch removes redundant lock widget members of the DatabaseWidget
and consolidates all unlocking functionality into a single
DatabaseOpenWidget (with the exception of KeePass1OpenWidget).
Distinction between different unlock actions is now done via a dedicated
Intent enum class instead of using individual widgets.

Further, the DatabaseUnlockDialog has been generalized so that it is
usable for unlock intents other than just Auto-Type and is now also
used for merging databases which is less confusing to the user.

The KeePassXC main window is no longer a parent of the
DatabaseUnlockDialog and has the Qt::ForeignWindow flag set, which
should cause fewer issues with Auto-Type trying to type into KeePassXC
after unlock instead of the intended target window.

In addition, its instance has been moved into the DatabaseTabWidget
class so that it is no longer bound to individual DatabaseWidgets,
potentially allowing for database selection during Auto-Type. The actual
selection has not yet been implemented, but Auto-Type has been adjusted
to use the currently selected tab instead of the first one as an
intermediary improvement.
@phoerious phoerious force-pushed the feature/refactor-databaseopenwidget branch from 39ffcee to d88f769 Compare November 24, 2018 10:57
Copy link
Member

@droidmonkey droidmonkey left a comment

Choose a reason for hiding this comment

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

It works, but I think we are complicating things again.

src/gui/DatabaseWidget.h Show resolved Hide resolved
src/gui/DatabaseWidget.cpp Show resolved Hide resolved
@phoerious phoerious merged commit 3c362ac into develop Nov 24, 2018
@phoerious phoerious deleted the feature/refactor-databaseopenwidget branch November 24, 2018 14:51
droidmonkey added a commit that referenced this pull request Mar 19, 2019
- New Database Wizard [#1952]
- Advanced Search [#1797]
- Automatic update checker [#2648]
- KeeShare database synchronization [#2109, #1992, #2738, #2742, #2746, #2739]
- Improve favicon fetching; transition to Duck-Duck-Go [#2795, #2011, #2439]
- Remove KeePassHttp support [#1752]
- CLI: output info to stderr for easier scripting [#2558]
- CLI: Add --quiet option [#2507]
- CLI: Add create command [#2540]
- CLI: Add recursive listing of entries [#2345]
- CLI: Fix stdin/stdout encoding on Windows [#2425]
- SSH Agent: Support OpenSSH for Windows [#1994]
- macOS: TouchID Quick Unlock [#1851]
- macOS: Multiple improvements; include CLI in DMG [#2165, #2331, #2583]
- Linux: Prevent Klipper from storing secrets in clipboard [#1969]
- Linux: Use polling based file watching for NFS [#2171]
- Linux: Enable use of browser plugin in Snap build [#2802]
- TOTP QR Code Generator [#1167]
- High-DPI Scaling for 4k screens [#2404]
- Make keyboard shortcuts more consistent [#2431]
- Warn user if deleting referenced entries [#1744]
- Allow toolbar to be hidden and repositioned [#1819, #2357]
- Increase max allowed database timeout to 12 hours [#2173]
- Password generator uses existing password length by default [#2318]
- Improve alert message box button labels [#2376]
- Show message when a database merge makes no changes [#2551]
- Browser Integration Enhancements [#1497, #2253, #1904, #2232, #1850, #2218, #2391, #2396, #2542, #2622, #2637, #2790]
- Overall Code Improvements [#2316, #2284, #2351, #2402, #2410, #2419, #2422, #2443, #2491, #2506, #2610, #2667, #2709, #2731]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: refactoring Pull request that refactors code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants