From 0d3eb047c7e1e42482b1d37e23e4701b66bd1138 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Fri, 10 Jan 2020 22:28:31 -0500 Subject: [PATCH] Prevent crash when all entries are deleted from a group * Fix #4093 - The first entry in the list is selected after deleting an entry * Prevents crashes due to dangling pointers held by the Entry Preview Widget when entries were deleted. * Improve GUI tests to ensure this new behavior occurs. --- src/gui/DatabaseWidget.cpp | 9 +++++++++ src/gui/EntryPreviewWidget.cpp | 6 ++++-- src/gui/EntryPreviewWidget.h | 4 ++-- tests/gui/TestGui.cpp | 16 ++++++++++++++++ 4 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/gui/DatabaseWidget.cpp b/src/gui/DatabaseWidget.cpp index e7b6179fb..eb33c09c0 100644 --- a/src/gui/DatabaseWidget.cpp +++ b/src/gui/DatabaseWidget.cpp @@ -154,6 +154,7 @@ DatabaseWidget::DatabaseWidget(QSharedPointer db, QWidget* parent) m_shareLabel->setVisible(false); #endif + m_previewView->setObjectName("previewWidget"); m_previewView->hide(); m_previewSplitter->addWidget(m_entryView); m_previewSplitter->addWidget(m_previewView); @@ -552,6 +553,14 @@ void DatabaseWidget::deleteEntries(QList selectedEntries) } refreshSearch(); + + m_entryView->setFirstEntryActive(); + auto* currentEntry = currentSelectedEntry(); + if (currentEntry) { + m_previewView->setEntry(currentEntry); + } else { + m_previewView->setGroup(groupView()->currentGroup()); + } } bool DatabaseWidget::confirmDeleteEntries(QList entries, bool permanent) diff --git a/src/gui/EntryPreviewWidget.cpp b/src/gui/EntryPreviewWidget.cpp index e46e3a663..2e2e37dbc 100644 --- a/src/gui/EntryPreviewWidget.cpp +++ b/src/gui/EntryPreviewWidget.cpp @@ -145,10 +145,12 @@ void EntryPreviewWidget::setDatabaseMode(DatabaseWidget::Mode mode) } if (mode == DatabaseWidget::Mode::ViewMode) { - if (m_ui->stackedWidget->currentWidget() == m_ui->pageGroup) { + if (m_currentGroup && m_ui->stackedWidget->currentWidget() == m_ui->pageGroup) { setGroup(m_currentGroup); - } else { + } else if (m_currentEntry) { setEntry(m_currentEntry); + } else { + hide(); } } } diff --git a/src/gui/EntryPreviewWidget.h b/src/gui/EntryPreviewWidget.h index e8e7d2172..e1a7aff38 100644 --- a/src/gui/EntryPreviewWidget.h +++ b/src/gui/EntryPreviewWidget.h @@ -77,8 +77,8 @@ private: const QScopedPointer m_ui; bool m_locked; - Entry* m_currentEntry; - Group* m_currentGroup; + QPointer m_currentEntry; + QPointer m_currentGroup; QTimer m_totpTimer; quint8 m_selectedTabEntry; quint8 m_selectedTabGroup; diff --git a/tests/gui/TestGui.cpp b/tests/gui/TestGui.cpp index 48b5c7351..9118d3e21 100644 --- a/tests/gui/TestGui.cpp +++ b/tests/gui/TestGui.cpp @@ -54,6 +54,7 @@ #include "gui/CloneDialog.h" #include "gui/DatabaseTabWidget.h" #include "gui/DatabaseWidget.h" +#include "gui/EntryPreviewWidget.h" #include "gui/FileDialog.h" #include "gui/MessageBox.h" #include "gui/PasswordEdit.h" @@ -967,6 +968,7 @@ void TestGui::testDeleteEntry() QWidget* entryDeleteWidget = toolBar->widgetForAction(entryDeleteAction); entryView->setFocus(); + // Move one entry to the recycling bin QCOMPARE(m_dbWidget->currentMode(), DatabaseWidget::Mode::ViewMode); clickIndex(entryView->model()->index(1, 1), entryView, Qt::LeftButton); QVERIFY(entryDeleteWidget->isVisible()); @@ -979,6 +981,7 @@ void TestGui::testDeleteEntry() QCOMPARE(entryView->model()->rowCount(), 3); QCOMPARE(m_db->metadata()->recycleBin()->entries().size(), 1); + // Select multiple entries and move them to the recycling bin clickIndex(entryView->model()->index(1, 1), entryView, Qt::LeftButton); clickIndex(entryView->model()->index(2, 1), entryView, Qt::LeftButton, Qt::ControlModifier); QCOMPARE(entryView->selectionModel()->selectedRows().size(), 2); @@ -993,6 +996,7 @@ void TestGui::testDeleteEntry() QCOMPARE(entryView->model()->rowCount(), 1); QCOMPARE(m_db->metadata()->recycleBin()->entries().size(), 3); + // Go to the recycling bin QCOMPARE(groupView->currentGroup(), m_db->rootGroup()); QModelIndex rootGroupIndex = groupView->model()->index(0, 0); clickIndex(groupView->model()->index(groupView->model()->rowCount(rootGroupIndex) - 1, 0, rootGroupIndex), @@ -1000,6 +1004,7 @@ void TestGui::testDeleteEntry() Qt::LeftButton); QCOMPARE(groupView->currentGroup()->name(), m_db->metadata()->recycleBin()->name()); + // Delete one entry from the bin clickIndex(entryView->model()->index(0, 1), entryView, Qt::LeftButton); MessageBox::setNextAnswer(MessageBox::Cancel); QTest::mouseClick(entryDeleteWidget, Qt::LeftButton); @@ -1011,6 +1016,7 @@ void TestGui::testDeleteEntry() QCOMPARE(entryView->model()->rowCount(), 2); QCOMPARE(m_db->metadata()->recycleBin()->entries().size(), 2); + // Select the remaining entries and delete them clickIndex(entryView->model()->index(0, 1), entryView, Qt::LeftButton); clickIndex(entryView->model()->index(1, 1), entryView, Qt::LeftButton, Qt::ControlModifier); MessageBox::setNextAnswer(MessageBox::Delete); @@ -1018,6 +1024,16 @@ void TestGui::testDeleteEntry() QCOMPARE(entryView->model()->rowCount(), 0); QCOMPARE(m_db->metadata()->recycleBin()->entries().size(), 0); + // Ensure the entry preview widget shows the recycling group since all entries are deleted + auto* previewWidget = m_dbWidget->findChild("previewWidget"); + QVERIFY(previewWidget); + auto* groupTitleLabel = previewWidget->findChild("groupTitleLabel"); + QVERIFY(groupTitleLabel); + + QTRY_VERIFY(groupTitleLabel->isVisible()); + QVERIFY(groupTitleLabel->text().contains(m_db->metadata()->recycleBin()->name())); + + // Go back to the root group clickIndex(groupView->model()->index(0, 0), groupView, Qt::LeftButton); QCOMPARE(groupView->currentGroup(), m_db->rootGroup()); }