diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-14 20:06:08 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-14 20:06:08 +0200 |
| commit | 4a4849421ddac044212cd0e17f9aee8ff2606292 (patch) | |
| tree | bedfb35f370b637eedcb3cd4d24be8ac75b257b5 /src/mainwindow.cpp | |
| parent | bde7409ef817089298718376e46a57b2d303cf02 (diff) | |
| download | qtmaildir-4a4849421ddac044212cd0e17f9aee8ff2606292.tar.gz qtmaildir-4a4849421ddac044212cd0e17f9aee8ff2606292.zip | |
Revert the message-scoped auto mark-read
Reverts bde7409 and 66f1159. The user hit the worst possible symptom:
clicking one message marked a DIFFERENT, unrelated message read.
The cause is in markCurrentThreadRead, which reads
m_model->threadAt(current.row()). CLAUDE.md records this exact trap: a
tree numbers rows PER PARENT, so a reply's row() indexes its siblings
and threadAt() on it answers about an unrelated thread near the top of
the list. The guards then compared the right ids against the wrong
thread and let a write through for whatever message the timer's state
named.
That fault predates these commits, but they made it reachable and
harmful: while the write was thread-scoped the mismatch was mostly
masked, and scoping it to a single message turned it into "a random
message is now read".
Reverting rather than fixing forward. Marking the wrong mail read syncs
out to the server and cannot be undone from here, so the safe state is
the previous behaviour, which is too broad but predictable. The item 66
work in 4a4f82f stands: a thread root still renders one message.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'src/mainwindow.cpp')
| -rw-r--r-- | src/mainwindow.cpp | 47 |
1 files changed, 10 insertions, 37 deletions
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index ae5e8fc..fb34fe2 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -3355,36 +3355,16 @@ void MainWindow::markCurrentThreadRead() } const ThreadSummary thread = m_model->threadAt(current.row()); - if (thread.threadId != m_markReadThreadId) { + if (thread.threadId != m_markReadThreadId + || !thread.tags.contains(QStringLiteral("unread"))) { m_markReadThreadId.clear(); return; } - // No thread-level `unread` check here any more. It would ask the wrong - // question now that one message is marked rather than the thread: a thread - // carries `unread` while ANY message in it is unread, so a read root under - // unread replies would pass this and a write would be sent for a message - // that is already read. The scheduling side still checks it, which stops a - // fully-read thread from arming a timer at all; what survives to here is - // decided per message below. - + const QStringList threadIds = { m_markReadThreadId }; m_markReadThreadId.clear(); - // The MESSAGE on screen, not the thread it belongs to. - // - // This marked the whole thread until item 66, and that was coherent while - // a root click rendered the whole conversation: everything marked read had - // been displayed. Once a root began rendering a single message, the same - // code cleared `unread` from replies the user had never seen. Not a - // cosmetic slip: maildir.synchronize_flags is on, so removing `unread` - // rewrites Maildir filenames and the next sync carries it to the server. - // - // m_currentMessageId is what the pane actually rendered, set beside the - // loadMessage that produced it. - if (m_currentMessageId.isEmpty()) - return; - - // sendMessageTagChange, NOT tagSelected: this deliberately does not go on + // sendThreadTagChange, NOT tagSelected: this deliberately does not go on // the undo stack. The user never took this action, so hijacking Ctrl+Z to // reverse it would undo something they did not do, and toggle_unread // already gives them a direct way to put it back. Decided 2026-08-03. @@ -3392,8 +3372,8 @@ void MainWindow::markCurrentThreadRead() // It still funnels through the one applyTags path, per CLAUDE.md; what // differs is only whether the inverse is pushed, which is a window-level // decision above the worker. - sendMessageTagChange({ m_currentMessageId }, {}, - { QStringLiteral("unread") }, tr("Mark read")); + sendThreadTagChange(threadIds, {}, { QStringLiteral("unread") }, + tr("Mark read")); } void MainWindow::editTagsOnSelection() @@ -3482,17 +3462,10 @@ void MainWindow::sendMessageTagChange(const QStringList &messageIds, if (messageIds.isEmpty()) return; - // Optimistic, but scoped to the message. applyTagChange() is keyed by - // THREAD and would repaint the whole row as though every message in it had - // changed, which for a one-message edit is a lie; applyMessageTagChange() - // updates that message and lets the thread's own tags follow only when the - // answer is unambiguous. - // - // Not optional for auto mark-read: without it the write goes out, the - // status bar counts an unsynced edit, and the card stays bold with - // `unread` on it until the next query. The user reported exactly that. - for (const QString &messageId : messageIds) - m_model->applyMessageTagChange(messageId, add, remove); + // No optimistic model update. applyTagChange is keyed by THREAD and would + // repaint the whole row as though every message in it had changed, which + // for a one-message edit is a lie the user would see and then watch + // silently correct itself on the next query. // The accounts this touches, resolved through the containing threads: the // account is a property of the thread, and the sync needs the channel |
