From ed0e085377440a68cef63ade6dc2afd322c22df9 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 10 Aug 2026 20:43:05 +0200 Subject: feat(view): follow a background sync without a keystroke The thread list now updates itself when a sync finishes, whether it is empty or populated. New threads appear where the sort puts them, threads that stopped matching leave, and threads whose state changed repaint. Refreshing used to mean re-running the query, which cleared the model, the selection, the message pane and the undo stack, so 0.8.0 declined to do it on a cron timer and asked the user to press Enter instead. The result was a list that quietly disagreed with the database: mail indexed by cron never appeared, and an Unread view read to the end sat empty in front of it. ThreadListModel::reconcile() diffs a result against the current rows by thread id instead, so a surviving thread keeps its row, its persistent index and its loaded replies. Order comes from the result and is never imposed here, which is what makes the sort dropdown authoritative. The undo constraint this was sized around did not exist: no undo entry was ever keyed on a row. ThreadTagCommand stores thread ids and MessageTagCommand stores message ids, and applyTagChange() looks its target up by id, so an entry already survived its rows leaving the view. A thread read out of the current view now leaves the list, which is correct and would otherwise strand the reader, so MessageView grows a notice saying the open thread no longer matches, with a button that re-queries it. Recovery lists the whole conversation, expands it, and restores the message that was on screen rather than reopening at the first one. Ten defects were found building this, nine of them by hand testing: - SyncMonitor::start() polls synchronously, so an idle lock file emits stateChanged(Idle) from inside buildUi() and the first handler to touch a widget segfaults before the window exists. - QTreeView sets a current index when it takes focus with none set, and current drives loading, so new mail opened itself and was marked read without the user having looked at it. Selection is now required. - The notice outlived what it described, both when the pane was blanked and when another message replaced it. - Retiring the "Background sync completed" message left the bar claiming a sync was still running: silent means saying nothing new, not leaving a stale claim on screen. - A thread root sets both the thread id and the message id, so treating the message id as the message-row case discarded it for the commonest way to open a thread. - A freshly queried root does not know its own first message until the tree loads, so recovery selected nothing and left the pane blank. - A user query mid-recovery had its result hijacked by the pending selection. - MessageView emitted the recovery signal with its own members, so a direct connection handed MainWindow references that runCurrentQuery() then cleared by blanking the pane. The ids went empty mid-slot and no recovery ever ran. Every test passed against this, because reaching a slot through invokeMethod copies its arguments. A Qt signal argument is a reference until something copies it. Emitting a member to a slot that can re-enter the emitter is a use-after-write, and it presents as a wrong value rather than as a crash. Co-Authored-By: Claude Opus 5 --- src/messageview.cpp | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) (limited to 'src/messageview.cpp') diff --git a/src/messageview.cpp b/src/messageview.cpp index b804d37..b6c3fa3 100644 --- a/src/messageview.cpp +++ b/src/messageview.cpp @@ -188,6 +188,47 @@ MessageView::MessageView(QWidget *parent) blockedRow->addWidget(m_loadRemoteButton); blockedRow->addStretch(); + // The stale-thread notice, deliberately the same shape as the row above: + // a sentence and a button, above the message, leaving it readable. The + // user asked for this rather than for a dialog, and a dialog would be + // wrong anyway, since nothing here needs an answer before the message can + // be read. + m_staleBar = new QWidget(this); + m_staleBar->setObjectName(QStringLiteral("staleThreadBar")); + m_staleLabel = new QLabel( + tr("This thread no longer matches the current query."), m_staleBar); + m_staleButton = new QPushButton(tr("Show it anyway"), m_staleBar); + m_staleButton->setObjectName(QStringLiteral("staleThreadButton")); + connect(m_staleButton, &QPushButton::clicked, this, [this] { + if (m_staleThreadId.isEmpty()) + return; + + // COPIES, not the members themselves, and this is load-bearing rather + // than tidy. A direct connection passes these by reference all the way + // into MainWindow::recoverStaleThread(), which calls runCurrentQuery(), + // which blanks the pane, which calls setStaleThread() and assigns to + // the very members those references name. The ids then read as empty + // for the rest of the slot, so the recovery target was stored as an + // empty string and nothing was ever recovered: the thread came back + // collapsed with a blank pane, which is exactly the reported symptom. + // + // Invisible to a test that reaches the slot through invokeMethod, + // because that copies the arguments; it needs the real signal. + const QString threadId = m_staleThreadId; + const QString messageId = m_staleMessageId; + + // The message on screen goes with the request. Recovering the thread + // alone would reopen it at its first message, and the user was reading + // message four of eight. + emit staleThreadRecoveryRequested(threadId, messageId); + }); + auto *staleRow = new QHBoxLayout(m_staleBar); + staleRow->setContentsMargins(0, 0, 0, 0); + staleRow->addWidget(m_staleLabel); + staleRow->addWidget(m_staleButton); + staleRow->addStretch(); + m_staleBar->hide(); + m_attachmentBar = new QWidget(this); m_attachmentBar->setObjectName(QStringLiteral("attachmentBar")); new QHBoxLayout(m_attachmentBar); @@ -200,6 +241,7 @@ MessageView::MessageView(QWidget *parent) auto *layout = new QVBoxLayout(this); layout->addLayout(headerRow); layout->addLayout(blockedRow); + layout->addWidget(m_staleBar); layout->addWidget(m_view, 1); layout->addWidget(m_attachmentBar); layout->addWidget(m_tagStrip); @@ -279,6 +321,12 @@ void MessageView::clear() m_blockedLabel->hide(); m_loadRemoteButton->hide(); + // The stale notice describes the message that WAS rendered, so it goes with + // it, for the same reason as the blocked-content bar above. Left behind, it + // sits over a blank pane naming a thread that is no longer shown, and its + // button offers to recover a thread the user has navigated away from. + setStaleThread(QString(), QString()); + // clear() does not go through render(), so the bar has to be emptied // here or the previous thread's attachments stay offered. rebuildAttachmentBar(); @@ -654,6 +702,14 @@ void MessageView::saveAttachment(const Attachment &attachment) emit statusMessage(tr("Saved %1").arg(written)); } +void MessageView::setStaleThread(const QString &threadId, + const QString &messageId) +{ + m_staleThreadId = threadId; + m_staleMessageId = messageId; + m_staleBar->setVisible(!threadId.isEmpty()); +} + void MessageView::toggleHtml() { const bool anyHtml = std::any_of( -- cgit v1.2.3