From 93e6a533f4b0cc1a75000b2f8c77181dfc56e199 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 10 Aug 2026 09:23:50 +0200 Subject: fix(ui): expand flat threads, reach the first message, localise the date Four faults from the first hand test, two of them behavioural. An expander that opened onto nothing. setThreadMessages kept only nodes with depth > 0, and notmuch_thread_get_toplevel_messages returns every message at depth 0 when a thread carries no usable In-Reply-To, so a flat thread contributed no children while its card still advertised the count. Measured in the user's database: of 396 inbox threads three are flat, one of them nine messages long, and every two-message thread of that kind was affected, which is exactly why the fault looked like "the expander only works with more than one reply". The rule is now position, not depth: every message except the first, which is the root card itself. That is also the correct rule rather than a workaround, since the row under the root is the second message however notmuch chose to nest it. The thread's first message was unreachable. Selecting a root card loaded the whole thread, so the pane showed every message with only the last expanded, and no row in the list offered the first one: the reply rows are messages two onward. The root card now renders its own message, which is what the card already claims to be. It keeps its thread id, unlike the message-row path, so mark-read and the tag-change repaint still work; that is asserted, because clearing it is the obvious way to write this and silently disables both. Before the replies are loaded the model has no first message to name and the whole thread stays the honest answer. Dates ignored the locale. One hardcoded "yyyy-MM-dd hh:mm" produced a US-looking format on an Italian desktop; QLocale::system() now formats it, and the width reserved for the date comes from the same function so a longer locale cannot clip. The expander was a bare number on the card's own background. It is a pill now, carrying "3 replies" (and "1 reply", singular), sized from the label actually drawn and measured in both glyph states so it does not resize under the pointer on click. Its fill is blended from Text toward Base rather than taken from QPalette::Button, which is #2b2b2b against a Base of #2b2b2b on the user's theme: byte identical, so the pill was invisible. A theme may make any two roles equal; a blend is defined against the surface it sits on and cannot collide with it. Checked by rendering both a dark and a light palette and looking. --- tests/test_mainwindow.cpp | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) (limited to 'tests/test_mainwindow.cpp') diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index cdb08ca..922705c 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -105,6 +105,7 @@ private slots: void childRowsAreIndentedUnderTheirThread(); void aThreadWithRepliesDrawsAVisibleExpander(); void cardsNeverScrollSideways(); + void selectingARootCardKeepsItsThreadForMarkRead(); void nextThreadLeavesTheLastReply(); void altDownSkipsReplies(); void bothThreadStepBindingsReachTheAction(); @@ -667,8 +668,12 @@ NavFixture buildNavFixture(MainWindow &window) f.view = window.findChild(); f.model = window.findChild(); + // unread, so a selection arms the mark-read timer: scheduleMarkRead() + // returns early for a thread that is already read, and a fixture without + // it would make a mark-read assertion pass for the wrong reason. ThreadSummary first = makeThread(QStringLiteral("T1"), - QStringList{ QStringLiteral("inbox") }); + QStringList{ QStringLiteral("inbox"), + QStringLiteral("unread") }); first.totalCount = 2; ThreadSummary second = makeThread(QStringLiteral("T2"), QStringList{ QStringLiteral("inbox") }); @@ -693,6 +698,38 @@ NavFixture buildNavFixture(MainWindow &window) } // namespace +void TestMainWindow::selectingARootCardKeepsItsThreadForMarkRead() +{ + // A root card is BOTH a message and a thread: it renders the thread's + // first message, and it is still the thread that gets marked read and + // repainted on a tag change. The message-row path deliberately clears the + // current thread id; doing that here too would silently disable mark-read + // and the tag-change repaint for every thread root in the list. + const Config config; + MainWindow window(config); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + const NavFixture f = buildNavFixture(window); + f.view->setCurrentIndex(f.root); + QApplication::processEvents(); + + // Guard: the fixture loads replies, so the root knows its own message and + // the branch under test is the one that runs. + QVERIFY2(!f.model->data(f.root, ThreadListModel::MessageIdRole) + .toString().isEmpty(), + "the root card does not know its first message, so this exercises " + "the fallback rather than the path it is written for"); + + // A mark-read timer armed for the thread is what proves the thread id + // survived: scheduleMarkRead() is only reached on the thread-row path. + auto *timer = window.findChild(QStringLiteral("markReadTimer")); + QVERIFY(timer); + QVERIFY2(timer->isActive(), + "no mark-read timer for a selected root card: its thread id was " + "cleared along with the switch to rendering one message"); +} + void TestMainWindow::nextThreadLeavesTheLastReply() { const Config config; -- cgit v1.2.3