Skip to content

Commit

Permalink
Fix minor UX issues with DB Open Widget
Browse files Browse the repository at this point in the history 
* Only clear password field when switching tabs or minimizing. This prevents the setting "Remember Key Files and Hardware Keys" from being useless with multiple databases.
* Convert key file field to Line Edit, simplifies usage. Fix clear field button as well.
* Removed need for clearForms to check if the database is being opened (was a solution to tab switching while unlocking, no longer a problem).
  • Loading branch information
@droidmonkey
droidmonkey committed Jun 6, 2020
1 parent 6c91254 commit 1ad0184
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 68 deletions.
78 changes: 36 additions & 42 deletions src/gui/DatabaseOpenWidget.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@
#include <QFont>
#include <QSharedPointer>

namespace
{
constexpr int clearFormsDelay = 30000;
}

DatabaseOpenWidget::DatabaseOpenWidget(QWidget* parent)
: DialogyWidget(parent)
, m_ui(new Ui::DatabaseOpenWidget())
Expand All @@ -47,14 +52,20 @@ DatabaseOpenWidget::DatabaseOpenWidget(QWidget* parent)

m_ui->messageWidget->setHidden(true);

m_hideTimer.setInterval(clearFormsDelay);
m_hideTimer.setSingleShot(true);
connect(&m_hideTimer, &QTimer::timeout, this, [this] {
// Reset the password field after being hidden for a set time
m_ui->editPassword->setText("");
m_ui->editPassword->setShowPassword(false);
});

QFont font;
font.setPointSize(font.pointSize() + 4);
font.setBold(true);
m_ui->labelHeadline->setFont(font);
m_ui->labelHeadline->setText(tr("Unlock KeePassXC Database"));

m_ui->comboKeyFile->lineEdit()->addAction(m_ui->keyFileClearIcon, QLineEdit::TrailingPosition);

connect(m_ui->buttonBrowseFile, SIGNAL(clicked()), SLOT(browseKeyFile()));

connect(m_ui->buttonBox, SIGNAL(accepted()), SLOT(openDatabase()));
Expand All @@ -65,11 +76,11 @@ DatabaseOpenWidget::DatabaseOpenWidget(QWidget* parent)
m_ui->keyFileLabelHelp->setIcon(resources()->icon("system-help").pixmap(QSize(12, 12)));
connect(m_ui->keyFileLabelHelp, SIGNAL(clicked(bool)), SLOT(openKeyFileHelp()));

connect(m_ui->comboKeyFile->lineEdit(), SIGNAL(textChanged(QString)), SLOT(handleKeyFileComboEdited()));
connect(m_ui->comboKeyFile, SIGNAL(currentIndexChanged(int)), SLOT(handleKeyFileComboChanged()));
connect(m_ui->keyFileLineEdit, SIGNAL(textChanged(QString)), SLOT(keyFileTextChanged()));
m_ui->keyFileLineEdit->addAction(m_ui->keyFileClearIcon, QLineEdit::TrailingPosition);
m_ui->keyFileClearIcon->setIcon(resources()->icon("edit-clear-locationbar-rtl"));
m_ui->keyFileClearIcon->setVisible(false);
connect(m_ui->keyFileClearIcon, SIGNAL(triggered(bool)), SLOT(clearKeyFileEdit()));
connect(m_ui->keyFileClearIcon, SIGNAL(triggered(bool)), SLOT(clearKeyFileText()));

#ifdef WITH_XC_YUBIKEY
m_ui->hardwareKeyProgress->setVisible(false);
Expand Down Expand Up @@ -114,33 +125,32 @@ void DatabaseOpenWidget::showEvent(QShowEvent* event)
{
DialogyWidget::showEvent(event);
m_ui->editPassword->setFocus();
m_hideTimer.stop();
}

void DatabaseOpenWidget::hideEvent(QHideEvent* event)
{
DialogyWidget::hideEvent(event);

// Clear the forms if we are minimized
// Schedule form clearing if we are hidden
if (!isVisible()) {
clearForms();
m_hideTimer.start();
}
}

void DatabaseOpenWidget::load(const QString& filename)
{
clearForms();

m_filename = filename;
m_ui->fileNameLabel->setRawText(m_filename);

m_ui->comboKeyFile->addItem(tr("Select key file..."), -1);
m_ui->comboKeyFile->setCurrentIndex(0);
m_ui->keyFileClearIcon->setVisible(false);
m_keyFileComboEdited = false;

if (config()->get(Config::RememberLastKeyFiles).toBool()) {
auto lastKeyFiles = config()->get(Config::LastKeyFiles).toHash();
if (lastKeyFiles.contains(m_filename)) {
m_ui->comboKeyFile->addItem(lastKeyFiles[m_filename].toString());
m_ui->comboKeyFile->setCurrentIndex(1);
m_ui->keyFileLineEdit->setText(lastKeyFiles[m_filename].toString());
}
}

Expand All @@ -160,14 +170,12 @@ void DatabaseOpenWidget::load(const QString& filename)

void DatabaseOpenWidget::clearForms()
{
if (!m_isOpeningDatabase) {
m_ui->editPassword->setText("");
m_ui->editPassword->setShowPassword(false);
m_ui->comboKeyFile->clear();
m_ui->comboKeyFile->setEditText("");
m_ui->checkTouchID->setChecked(false);
m_db.reset();
}
m_ui->editPassword->setText("");
m_ui->editPassword->setShowPassword(false);
m_ui->keyFileLineEdit->clear();
m_ui->checkTouchID->setChecked(false);
m_ui->challengeResponseCombo->clear();
m_db.reset();
}

QSharedPointer<Database> DatabaseOpenWidget::database()
Expand All @@ -183,8 +191,7 @@ QString DatabaseOpenWidget::filename()
void DatabaseOpenWidget::enterKey(const QString& pw, const QString& keyFile)
{
m_ui->editPassword->setText(pw);
m_ui->comboKeyFile->setCurrentIndex(-1);
m_ui->comboKeyFile->setEditText(keyFile);
m_ui->keyFileLineEdit->setText(keyFile);
openDatabase();
}

Expand All @@ -200,7 +207,6 @@ void DatabaseOpenWidget::openDatabase()
m_ui->editPassword->setShowPassword(false);
QCoreApplication::processEvents();

m_isOpeningDatabase = true;
m_db.reset(new Database());
QString error;

Expand Down Expand Up @@ -230,10 +236,8 @@ void DatabaseOpenWidget::openDatabase()
config()->set(Config::UseTouchID, useTouchID);
#endif
emit dialogFinished(true);
m_isOpeningDatabase = false;
clearForms();
} else {
m_isOpeningDatabase = false;
if (m_ui->editPassword->text().isEmpty() && !m_retryUnlockWithEmptyPassword) {
QScopedPointer<QMessageBox> msgBox(new QMessageBox(this));
msgBox->setIcon(QMessageBox::Critical);
Expand Down Expand Up @@ -298,8 +302,8 @@ QSharedPointer<CompositeKey> DatabaseOpenWidget::databaseKey()
lastKeyFiles.remove(m_filename);

auto key = QSharedPointer<FileKey>::create();
QString keyFilename = m_ui->comboKeyFile->currentText();
if (!m_ui->comboKeyFile->currentText().isEmpty() && m_keyFileComboEdited) {
QString keyFilename = m_ui->keyFileLineEdit->text();
if (!keyFilename.isEmpty()) {
QString errorMsg;
if (!key->load(keyFilename, &errorMsg)) {
m_ui->messageWidget->showMessage(tr("Failed to open key file: %1").arg(errorMsg), MessageWidget::Error);
Expand Down Expand Up @@ -375,28 +379,18 @@ void DatabaseOpenWidget::browseKeyFile()
}

if (!filename.isEmpty()) {
m_ui->comboKeyFile->setCurrentIndex(-1);
m_ui->comboKeyFile->setEditText(filename);
m_ui->keyFileLineEdit->setText(filename);
}
}

void DatabaseOpenWidget::clearKeyFileEdit()
{
m_ui->comboKeyFile->setCurrentIndex(0);
// make sure that handler is called even if 0 was the current index already
handleKeyFileComboChanged();
}

void DatabaseOpenWidget::handleKeyFileComboEdited()
void DatabaseOpenWidget::clearKeyFileText()
{
m_keyFileComboEdited = true;
m_ui->keyFileClearIcon->setVisible(true);
m_ui->keyFileLineEdit->clear();
}

void DatabaseOpenWidget::handleKeyFileComboChanged()
void DatabaseOpenWidget::keyFileTextChanged()
{
m_keyFileComboEdited = m_ui->comboKeyFile->currentIndex() != 0;
m_ui->keyFileClearIcon->setVisible(m_keyFileComboEdited);
m_ui->keyFileClearIcon->setVisible(!m_ui->keyFileLineEdit->text().isEmpty());
}

void DatabaseOpenWidget::pollHardwareKey()
Expand Down
21 changes: 10 additions & 11 deletions src/gui/DatabaseOpenWidget.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#define KEEPASSX_DATABASEOPENWIDGET_H

#include <QScopedPointer>
#include <QTimer>

#include "gui/DialogyWidget.h"
#include "keys/CompositeKey.h"
Expand Down Expand Up @@ -53,30 +54,28 @@ class DatabaseOpenWidget : public DialogyWidget
void hideEvent(QHideEvent* event) override;
QSharedPointer<CompositeKey> databaseKey();

const QScopedPointer<Ui::DatabaseOpenWidget> m_ui;
QSharedPointer<Database> m_db;
QString m_filename;
bool m_retryUnlockWithEmptyPassword = false;

protected slots:
virtual void openDatabase();
void reject();

private slots:
void browseKeyFile();
void clearKeyFileEdit();
void handleKeyFileComboEdited();
void handleKeyFileComboChanged();
void clearKeyFileText();
void keyFileTextChanged();
void pollHardwareKey();
void hardwareKeyResponse(bool found);
void openHardwareKeyHelp();
void openKeyFileHelp();

protected:
const QScopedPointer<Ui::DatabaseOpenWidget> m_ui;
QSharedPointer<Database> m_db;
QString m_filename;
bool m_retryUnlockWithEmptyPassword = false;

private:
bool m_pollingHardwareKey = false;
bool m_keyFileComboEdited = false;
bool m_isOpeningDatabase = false;
QTimer m_hideTimer;

Q_DISABLE_COPY(DatabaseOpenWidget)
};

Expand Down
14 changes: 4 additions & 10 deletions src/gui/DatabaseOpenWidget.ui
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@
<string>Key File:</string>
</property>
<property name="buddy">
<cstring>comboKeyFile</cstring>
<cstring>keyFileLineEdit</cstring>
</property>
</widget>
</item>
Expand Down Expand Up @@ -406,21 +406,15 @@
<number>0</number>
</property>
<item row="0" column="1">
<widget class="QComboBox" name="comboKeyFile">
<property name="enabled">
<bool>true</bool>
</property>
<widget class="QLineEdit" name="keyFileLineEdit">
<property name="sizePolicy">
<sizepolicy hsizetype="Expanding" vsizetype="Fixed">
<horstretch>0</horstretch>
<verstretch>0</verstretch>
</sizepolicy>
</property>
<property name="accessibleName">
<string>Key file selection</string>
</property>
<property name="editable">
<bool>true</bool>
<string>Key file to unlock the database</string>
</property>
</widget>
</item>
Expand Down Expand Up @@ -610,7 +604,7 @@
</customwidgets>
<tabstops>
<tabstop>editPassword</tabstop>
<tabstop>comboKeyFile</tabstop>
<tabstop>keyFileLineEdit</tabstop>
<tabstop>buttonBrowseFile</tabstop>
<tabstop>challengeResponseCombo</tabstop>
<tabstop>buttonRedetectYubikey</tabstop>
Expand Down
6 changes: 1 addition & 5 deletions src/gui/KeePass1OpenWidget.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,12 @@ void KeePass1OpenWidget::openDatabase()
KeePass1Reader reader;

QString password;
QString keyFileName;
QString keyFileName = m_ui->keyFileLineEdit->text();

if (!m_ui->editPassword->text().isEmpty() || m_retryUnlockWithEmptyPassword) {
password = m_ui->editPassword->text();
}

if (!m_ui->comboKeyFile->currentText().isEmpty() && m_ui->comboKeyFile->currentData() != -1) {
keyFileName = m_ui->comboKeyFile->currentText();
}

QFile file(m_filename);
if (!file.open(QIODevice::ReadOnly)) {
m_ui->messageWidget->showMessage(tr("Unable to open the database.").append("\n").append(file.errorString()),
Expand Down

0 comments on commit 1ad0184

Please sign in to comment.