From c3dd54e390d236b7d68bae456ef39c5fa7fb49d5 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Sat, 15 Aug 2026 15:44:38 +0200 Subject: fix(ui): load a thread that was already displayed when the query ran Running a query blanks the message pane but left m_currentThreadId, m_currentMessageId and m_currentMessageThreadId naming the thread that had been showing. Both selection handlers compare a newly selected row against those to decide whether it is already on display, so a result containing that same thread was recognised as "already showing" and onThreadSelected() was never called. The card painted as selected, the status bar reported one thread, and the pane stayed on the placeholder. This is why it looked like an `id:` query defect. The id is copied out of the details dialog of the message being read, so that thread is current at the moment the query replaces the view. Any query returning a different thread hides the fault entirely. Filed as the unverified half of item 66 and assumed to be the same empty-MessageIdRole failure. It is not: 66's fix was correct and this reproduced against it, so it is recorded as item 96. Four hypotheses were eliminated by measurement first: the row does carry the message id, the account-scoped query does return it, MimeParser parses the reported message (ok, 40701 bytes of HTML), and both real ids resolve bare and quoted. The regression test's first query must open the SAME thread the second one returns; with two different threads it passes against the defect, which is how the first version of it was green. Reverting the fix fails it. Co-Authored-By: Claude Opus 5 --- tests/test_mainwindow.cpp | 105 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 104 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index c569f05..b96b158 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -82,7 +82,11 @@ class WorkerBackedWindow public: /// Builds the database, writes the config and loads it. Check isValid() /// and error() before constructing the window. - bool build() + /// `accountKey` and `accountMaildir` add one [account.] section, which + /// is what makes runQuery() scope the bar's text with scopedQuery(). A test + /// that never selects an account can leave them empty. + bool build(const QString &accountKey = QString(), + const QString &accountMaildir = QString()) { if (!m_fixture.isValid()) { m_error = QStringLiteral("fixture directory invalid"); @@ -113,6 +117,12 @@ public: // went unnoticed as broken once already. out << "[general]\n" << "notmuch_config=" << m_fixture.configPath() << "\n"; + if (!accountKey.isEmpty()) { + // QSettings reads `/` in a section name as a group separator, + // so the section is [account.key], never [account/key]. + out << "\n[account." << accountKey << "]\n" + << "maildir=" << accountMaildir << "\n"; + } } file.close(); @@ -188,6 +198,7 @@ private slots: void aRefreshsBatchesLeaveTheStatusBarAlone(); void selectingAThreadRootShowsItInTheMessagePane(); void anUnexpandedRootRendersOneMessageNotTheConversation(); + void aSingleMessageIdQuerysCardOpensInTheMessagePane(); void autoSyncIsNotArmedWhenDisabledOrWithNothingPending(); void autoSyncSkipsWhileABackgroundSyncIsRunning(); void aSuccessfulSyncRefreshesRatherThanRerunningTheQuery(); @@ -7184,4 +7195,96 @@ void TestMainWindow::anUnexpandedRootRendersOneMessageNotTheConversation() "the pane rendered a conversation, not a single message"); } +void TestMainWindow::aSingleMessageIdQuerysCardOpensInTheMessagePane() +{ + // Item 66's unverified half, reported again against 0.23.0 with a + // screenshot: an `id:` query in the bar produces exactly one card, the + // status bar reports "1 thread selected (1 message)", and the pane stays + // on the placeholder. + // + // The id is shaped like the real one that fails: dots, digits and an @. + // + // An ACCOUNT is configured and selected, because that is the state the + // report was made from and it is the only thing that changes the query: + // runQuery() wraps the bar's text in path:"/**" and (...). + WorkerBackedWindow backed; + const QString wanted = + QStringLiteral("1786718040388.1f6b48f1-64d1@mail.example.org"); + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/Inbox"), wanted, + QStringLiteral("A single message"), + QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("The only message."))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + + QLineEdit *queryEdit = + window.findChild(QStringLiteral("queryEdit")); + QVERIFY2(queryEdit, "no query bar: the window was never built"); + auto *view = window.findChild(); + QVERIFY2(view, "no thread list view"); + auto *model = window.findChild(); + QVERIFY2(model, "no thread list model"); + auto *pane = window.findChild(); + QVERIFY2(pane, "no message view"); + + auto *accountBox = + window.findChild(QStringLiteral("accountBox")); + QVERIFY2(accountBox, "no account dropdown"); + const int account = accountBox->findData(QStringLiteral("acct")); + QVERIFY2(account >= 0, "the configured account is not in the dropdown"); + accountBox->setCurrentIndex(account); + + // THE SAME MESSAGE IS READ FIRST, and that is the whole defect. The id is + // copied out of the details dialog of a message being read, so the `id:` + // query is always typed while that very thread is the current one. + // runQuery() blanks the pane but leaves m_currentThreadId naming it, so + // when the row comes back onSelectionChanged() compares the two, finds them + // equal, and never calls onThreadSelected: nothing is ever loaded. + // + // A first query returning a DIFFERENT thread passes against the bug, which + // is why the earlier version of this test was green. + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + QTRY_VERIFY_WITH_TIMEOUT(!pane->showingPlaceholder(), 15000); + const QString firstThreadId = window.currentThreadId(); + QVERIFY2(!firstThreadId.isEmpty(), "the first view never opened a thread"); + + // Exactly what the user types, `id:` and the bare id, unquoted. + queryEdit->setText(QStringLiteral("id:%1").arg(wanted)); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + + const QModelIndex root = model->index(0, 0, QModelIndex()); + QVERIFY(root.isValid()); + + // The row must name the message before the click can be blamed for + // anything: an empty MessageIdRole is a different defect and would make + // the assertion below true for the wrong reason. + QCOMPARE(model->data(root, ThreadListModel::MessageIdRole).toString(), + wanted); + + QVERIFY2(pane->showingPlaceholder(), + "the pane was not blank to begin with"); + // The row IS the thread that was showing when the query ran. Asserted so a + // fixture change that made them different threads could not quietly turn + // this back into the passing test it was before the cause was found. + QCOMPARE(model->data(root, ThreadListModel::ThreadIdRole).toString(), + firstThreadId); + + // The pane was blanked, so nothing is on display and the window must not + // still claim otherwise. This is the fix's own contract: leave it set and + // the selection below is read as "already showing" and never loads. + QVERIFY2(window.currentThreadId().isEmpty(), + "runQuery blanked the pane but still names a current thread"); + + view->setCurrentIndex(root); + QTRY_VERIFY_WITH_TIMEOUT(!pane->showingPlaceholder(), 15000); +} + #include "test_mainwindow.moc" -- cgit v1.2.3