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.
keepassxreboot · Jan 13, 2020 · f8c1d4b · f8c1d4b
1 parent 4607320
commit f8c1d4b
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 4 deletions.
diff --git a/src/gui/DatabaseWidget.cpp b/src/gui/DatabaseWidget.cpp
@@ -154,6 +154,7 @@ DatabaseWidget::DatabaseWidget(QSharedPointer<Database> 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<Entry*> selectedEntries)
     }
 
     refreshSearch();
+
+    m_entryView->setFirstEntryActive();
+    auto* currentEntry = currentSelectedEntry();
+    if (currentEntry) {
+        m_previewView->setEntry(currentEntry);
+    } else {
+        m_previewView->setGroup(groupView()->currentGroup());
+    }
 }
 
 bool DatabaseWidget::confirmDeleteEntries(QList<Entry*> entries, bool permanent)

diff --git 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
@@ -77,8 +77,8 @@ private slots:
 
     const QScopedPointer<Ui::EntryPreviewWidget> m_ui;
     bool m_locked;
-    Entry* m_currentEntry;
-    Group* m_currentGroup;
+    QPointer<Entry> m_currentEntry;
+    QPointer<Group> m_currentGroup;
     QTimer m_totpTimer;
     quint8 m_selectedTabEntry;
     quint8 m_selectedTabGroup;

diff --git 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,13 +996,15 @@ 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),
                groupView,
                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,13 +1016,24 @@ 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);
     QTest::mouseClick(entryDeleteWidget, Qt::LeftButton);
     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<EntryPreviewWidget*>("previewWidget");
+    QVERIFY(previewWidget);
+    auto* groupTitleLabel = previewWidget->findChild<QLabel*>("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());
 }