From ea50b958b82c76ed554b2e474f3da9b2ab3dd391 Mon Sep 17 00:00:00 2001 From: Allen Wild Date: Tue, 22 Sep 2020 19:48:20 -0400 Subject: [PATCH] Allow selecting any open database in unlock dialog When showing the database unlock dialog from browser or auto-type and there's more than one open database, add tabs to the unlock dialog so that the user can choose which database to unlock. This implements part of [1] and improves the UX when working with multiple databases after version 2.6 switched to using the dialog for browser unlock. Make the DatabaseOpenDialog window Application-Modal so that it blocks input to the main UI when the dialog is open. This reduces corner cases by avoiding the possibility of databases getting closed or unlocked behind the open dialog. [1] https://github.com/keepassxreboot/keepassxc/issues/2322 --- src/gui/DatabaseOpenDialog.cpp | 125 ++++++++++++++++++++++++++++----- src/gui/DatabaseOpenDialog.h | 16 +++-- src/gui/DatabaseTabWidget.cpp | 40 +++++++++-- src/gui/DatabaseTabWidget.h | 2 + 4 files changed, 158 insertions(+), 25 deletions(-) diff --git a/src/gui/DatabaseOpenDialog.cpp b/src/gui/DatabaseOpenDialog.cpp index e7194b7e24..c113884135 100644 --- a/src/gui/DatabaseOpenDialog.cpp +++ b/src/gui/DatabaseOpenDialog.cpp @@ -17,40 +17,116 @@ #include "DatabaseOpenDialog.h" #include "DatabaseOpenWidget.h" +#include "DatabaseTabWidget.h" #include "DatabaseWidget.h" #include "core/Database.h" +#include +#include + DatabaseOpenDialog::DatabaseOpenDialog(QWidget* parent) : QDialog(parent) , m_view(new DatabaseOpenWidget(this)) + , m_tabBar(new QTabBar(this)) { setWindowTitle(tr("Unlock Database - KeePassXC")); setWindowFlags(Qt::Dialog | Qt::WindowStaysOnTopHint); - connect(m_view, SIGNAL(dialogFinished(bool)), this, SLOT(complete(bool))); + // block input to the main window/application while the dialog is open + setWindowModality(Qt::ApplicationModal); + connect(m_view, &DatabaseOpenWidget::dialogFinished, this, &DatabaseOpenDialog::complete); + + m_tabBar->setAutoHide(true); + m_tabBar->setExpanding(false); + connect(m_tabBar, &QTabBar::currentChanged, this, &DatabaseOpenDialog::tabChanged); + auto* layout = new QVBoxLayout(); - layout->setMargin(0); - setLayout(layout); + layout->setContentsMargins(0, 0, 0, 0); + layout->setSpacing(0); + layout->addWidget(m_tabBar); layout->addWidget(m_view); + setLayout(layout); setMinimumWidth(700); + + // set up Ctrl+PageUp and Ctrl+PageDown shortcuts to cycle tabs + auto* shortcut = new QShortcut(Qt::CTRL + Qt::Key_PageUp, this); + shortcut->setContext(Qt::WidgetWithChildrenShortcut); + connect(shortcut, &QShortcut::activated, this, [this]() { selectTabOffset(-1); }); + shortcut = new QShortcut(Qt::CTRL + Qt::Key_PageDown, this); + shortcut->setContext(Qt::WidgetWithChildrenShortcut); + connect(shortcut, &QShortcut::activated, this, [this]() { selectTabOffset(1); }); } -void DatabaseOpenDialog::setFilePath(const QString& filePath) +void DatabaseOpenDialog::selectTabOffset(int offset) { - m_view->load(filePath); + if (offset == 0 || m_tabBar->count() <= 1) { + return; + } + int tab = m_tabBar->currentIndex() + offset; + int last = m_tabBar->count() - 1; + if (tab < 0) { + tab = last; + } else if (tab > last) { + tab = 0; + } + m_tabBar->setCurrentIndex(tab); +} + +void DatabaseOpenDialog::addDatabaseTab(DatabaseWidget* dbWidget) +{ + Q_ASSERT(dbWidget); + if (!dbWidget) { + return; + } + + // important - we must add the DB widget first, because addTab will fire + // tabChanged immediately which will look for a dbWidget in the list + m_tabDbWidgets.append(dbWidget); + QFileInfo fileInfo(dbWidget->database()->filePath()); + m_tabBar->addTab(fileInfo.fileName()); + Q_ASSERT(m_tabDbWidgets.count() == m_tabBar->count()); +} + +void DatabaseOpenDialog::setActiveDatabaseTab(DatabaseWidget* dbWidget) +{ + if (!dbWidget) { + return; + } + int index = m_tabDbWidgets.indexOf(dbWidget); + if (index != -1) { + m_tabBar->setCurrentIndex(index); + } +} + +void DatabaseOpenDialog::tabChanged(int index) +{ + if (index < 0 || index >= m_tabDbWidgets.count()) { + return; + } + + if (m_tabDbWidgets.count() == m_tabBar->count()) { + DatabaseWidget* dbWidget = m_tabDbWidgets[index]; + setTarget(dbWidget, dbWidget->database()->filePath()); + } else { + // if these list sizes don't match, there's a bug somewhere nearby + qWarning("DatabaseOpenDialog: mismatch between tab count %d and DB count %d", + m_tabBar->count(), + m_tabDbWidgets.count()); + } } /** - * Set target DatabaseWidget to which signals are connected. - * - * @param dbWidget database widget + * Sets the target DB and reloads the UI. */ -void DatabaseOpenDialog::setTargetDatabaseWidget(DatabaseWidget* dbWidget) +void DatabaseOpenDialog::setTarget(DatabaseWidget* dbWidget, const QString& filePath) { - if (m_dbWidget) { - disconnect(this, nullptr, m_dbWidget, nullptr); + // reconnect finished signal to new dbWidget, then reload the UI + if (m_currentDbWidget) { + disconnect(this, &DatabaseOpenDialog::dialogFinished, m_currentDbWidget, nullptr); } - m_dbWidget = dbWidget; connect(this, &DatabaseOpenDialog::dialogFinished, dbWidget, &DatabaseWidget::unlockDatabase); + + m_currentDbWidget = dbWidget; + m_view->load(filePath); } void DatabaseOpenDialog::setIntent(DatabaseOpenDialog::Intent intent) @@ -68,13 +144,21 @@ void DatabaseOpenDialog::clearForms() m_view->clearForms(); m_db.reset(); m_intent = Intent::None; - if (m_dbWidget) { - disconnect(this, nullptr, m_dbWidget, nullptr); - m_dbWidget = nullptr; + if (m_currentDbWidget) { + disconnect(this, &DatabaseOpenDialog::dialogFinished, m_currentDbWidget, nullptr); + } + m_currentDbWidget.clear(); + m_tabDbWidgets.clear(); + + // block signals while removing tabs so that tabChanged doesn't get called + m_tabBar->blockSignals(true); + while (m_tabBar->count() > 0) { + m_tabBar->removeTab(0); } + m_tabBar->blockSignals(false); } -QSharedPointer DatabaseOpenDialog::database() +QSharedPointer DatabaseOpenDialog::database() const { return m_db; } @@ -89,6 +173,13 @@ void DatabaseOpenDialog::complete(bool accepted) } else { reject(); } - emit dialogFinished(accepted, m_dbWidget); + + if (m_intent != Intent::Merge) { + // Update the current database in the main UI to match what we just unlocked + auto* tabWidget = qobject_cast(parentWidget()); + tabWidget->setCurrentIndex(tabWidget->indexOf(m_currentDbWidget)); + } + + emit dialogFinished(accepted, m_currentDbWidget); clearForms(); } diff --git a/src/gui/DatabaseOpenDialog.h b/src/gui/DatabaseOpenDialog.h index 30ac4c762e..4a15efb5f3 100644 --- a/src/gui/DatabaseOpenDialog.h +++ b/src/gui/DatabaseOpenDialog.h @@ -21,8 +21,10 @@ #include "core/Global.h" #include +#include #include #include +#include class Database; class DatabaseWidget; @@ -42,11 +44,12 @@ class DatabaseOpenDialog : public QDialog }; explicit DatabaseOpenDialog(QWidget* parent = nullptr); - void setFilePath(const QString& filePath); - void setTargetDatabaseWidget(DatabaseWidget* dbWidget); + void setTarget(DatabaseWidget* dbWidget, const QString& filePath); + void addDatabaseTab(DatabaseWidget* dbWidget); + void setActiveDatabaseTab(DatabaseWidget* dbWidget); void setIntent(Intent intent); Intent intent() const; - QSharedPointer database(); + QSharedPointer database() const; void clearForms(); signals: @@ -54,11 +57,16 @@ class DatabaseOpenDialog : public QDialog public slots: void complete(bool accepted); + void tabChanged(int index); private: + void selectTabOffset(int offset); + QPointer m_view; + QPointer m_tabBar; QSharedPointer m_db; - QPointer m_dbWidget; + QList> m_tabDbWidgets; + QPointer m_currentDbWidget; Intent m_intent = Intent::None; }; diff --git a/src/gui/DatabaseTabWidget.cpp b/src/gui/DatabaseTabWidget.cpp index 2683cecec1..178ffb862f 100644 --- a/src/gui/DatabaseTabWidget.cpp +++ b/src/gui/DatabaseTabWidget.cpp @@ -664,11 +664,43 @@ void DatabaseTabWidget::unlockDatabaseInDialog(DatabaseWidget* dbWidget, DatabaseOpenDialog::Intent intent, const QString& filePath) { - m_databaseOpenDialog->setTargetDatabaseWidget(dbWidget); + m_databaseOpenDialog->clearForms(); + m_databaseOpenDialog->setIntent(intent); + m_databaseOpenDialog->setTarget(dbWidget, filePath); + displayUnlockDialog(); +} + +/** + * Unlock a database with an unlock popup dialog. + * The dialog allows the user to select any open & unlocked database. + * + * @param intent intent for unlocking + */ +void DatabaseTabWidget::unlockAnyDatabaseInDialog(DatabaseOpenDialog::Intent intent) +{ + m_databaseOpenDialog->clearForms(); m_databaseOpenDialog->setIntent(intent); - m_databaseOpenDialog->setFilePath(filePath); + // add a tab to the dialog for each open unlocked database + for (int i = 0, c = count(); i < c; ++i) { + auto* dbWidget = databaseWidgetFromIndex(i); + if (dbWidget && dbWidget->isLocked()) { + m_databaseOpenDialog->addDatabaseTab(dbWidget); + } + } + // default to the current tab + m_databaseOpenDialog->setActiveDatabaseTab(currentDatabaseWidget()); + displayUnlockDialog(); +} + +/** + * Display the unlock dialog after it's been initialized. + * This is an internal method, it should only be called by unlockDatabaseInDialog or unlockAnyDatabaseInDialog. + */ +void DatabaseTabWidget::displayUnlockDialog() +{ #ifdef Q_OS_MACOS + auto intent = m_databaseOpenDialog->intent(); if (intent == DatabaseOpenDialog::Intent::AutoType || intent == DatabaseOpenDialog::Intent::Browser) { macUtils()->raiseOwnWindow(); Tools::wait(200); @@ -753,7 +785,7 @@ void DatabaseTabWidget::performGlobalAutoType() if (config()->get(Config::Security_RelockAutoType).toBool()) { m_dbWidgetPendingLock = currentDatabaseWidget(); } - unlockDatabaseInDialog(currentDatabaseWidget(), DatabaseOpenDialog::Intent::AutoType); + unlockAnyDatabaseInDialog(DatabaseOpenDialog::Intent::AutoType); } } @@ -761,6 +793,6 @@ void DatabaseTabWidget::performBrowserUnlock() { auto dbWidget = currentDatabaseWidget(); if (dbWidget && dbWidget->isLocked()) { - unlockDatabaseInDialog(dbWidget, DatabaseOpenDialog::Intent::Browser); + unlockAnyDatabaseInDialog(DatabaseOpenDialog::Intent::Browser); } } diff --git a/src/gui/DatabaseTabWidget.h b/src/gui/DatabaseTabWidget.h index e59681ea73..ad2347923e 100644 --- a/src/gui/DatabaseTabWidget.h +++ b/src/gui/DatabaseTabWidget.h @@ -76,6 +76,7 @@ public slots: void closeDatabaseFromSender(); void unlockDatabaseInDialog(DatabaseWidget* dbWidget, DatabaseOpenDialog::Intent intent); void unlockDatabaseInDialog(DatabaseWidget* dbWidget, DatabaseOpenDialog::Intent intent, const QString& filePath); + void unlockAnyDatabaseInDialog(DatabaseOpenDialog::Intent intent); void relockPendingDatabase(); void showDatabaseSecurity(); @@ -105,6 +106,7 @@ private slots: QSharedPointer execNewDatabaseWizard(); void updateLastDatabases(const QString& filename); bool warnOnExport(); + void displayUnlockDialog(); QPointer m_dbWidgetStateSync; QPointer m_dbWidgetPendingLock;