diff options
Diffstat (limited to 'tests')
| -rw-r--r-- | tests/test_mainwindow.cpp | 464 | ||||
| -rw-r--r-- | tests/test_threadlistmodel.cpp | 224 |
2 files changed, 243 insertions, 445 deletions
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 740e7fa..d77cee3 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -46,6 +46,12 @@ #include "mainwindow.h" #include "messageview.h" #include "notmuchworker.h" +#include "carddelegate.h" +#include "cardlayout.h" + +#include <QImage> +#include <QPainter> +#include <QScrollBar> #include "tagchip.h" #include "threadlistmodel.h" #include "threadlistview.h" @@ -95,10 +101,9 @@ private slots: void anUnobservableLockTableLeavesTheSyncButtonUsable(); void theStatusBarFollowsTheSyncPhase(); void aSelectedReadThreadIsNotDimmedIntoTheHighlight(); - void thePillRowSpansTheWholeWidthNotOneColumn(); void childRowsAreIndentedUnderTheirThread(); void aThreadWithRepliesDrawsAVisibleExpander(); - void noTagStripIsPaintedUnderAMessageRow(); + void cardsNeverScrollSideways(); void replyRowsKeepTheirTextUnderTheThreadLine(); void clickingTheExpanderTogglesTheThread(); void selectingAMessageRowTargetsThatMessageNotItsThread(); @@ -451,11 +456,12 @@ void TestMainWindow::headerStateFromADifferentColumnLayoutIsDiscarded() window.close(); } - // Forge a state file from an older layout: same blob, wrong column count. + // Forge a state file from the five-column layout. Nothing reads these keys + // any more, and that is exactly what must be verified: a blob saved by an + // older version has to be ignored rather than applied to a one-column view. { QSettings state(MainWindow::uiStatePath(), QSettings::IniFormat); - state.setValue(QStringLiteral("threadlist/columns"), - int(ThreadListModel::ColumnCount) - 1); + state.setValue(QStringLiteral("threadlist/columns"), 5); state.setValue(QStringLiteral("threadlist/header"), QByteArray("not a header this model could have saved")); } @@ -466,9 +472,7 @@ void TestMainWindow::headerStateFromADifferentColumnLayoutIsDiscarded() auto *view = reopened.findChild<QTreeView *>(); QVERIFY(view); - QCOMPARE(view->columnWidth(ThreadListModel::AttachmentColumn), 28); - QCOMPARE(view->columnWidth(ThreadListModel::DateColumn), 130); - QCOMPARE(view->columnWidth(ThreadListModel::SubjectColumn), 520); + QCOMPARE(view->model()->columnCount(), 1); QFile::remove(MainWindow::uiStatePath()); QStandardPaths::setTestModeEnabled(false); @@ -548,83 +552,6 @@ static ThreadSummary makeThread(const QString &id, const QStringList &tags) return thread; } -void TestMainWindow::thePillRowSpansTheWholeWidthNotOneColumn() -{ - // The pills are a row-wide strip under the cells, not content of the - // subject cell. Drawn from the subject column's delegate they stop at that - // column's edge, so a thread with several tags loses the last of them; and - // they inherit the column's left edge, which puts them under the subject - // rather than under the row. - // - // The property: pills appear to the LEFT of where the subject column - // starts, which no per-cell delegate on that column could produce. - const Config config; - MainWindow window(config); - - auto *model = window.findChild<ThreadListModel *>(); - QVERIFY(model); - auto *view = window.findChild<QTreeView *>(); - QVERIFY(view); - - ThreadSummary thread = makeThread(QStringLiteral("t1"), {}); - thread.tags = QStringList{ QStringLiteral("mailing-list/SBo"), - QStringLiteral("signed") }; - model->appendBatch({ thread }); - - window.resize(1400, 300); - window.show(); - QVERIFY(QTest::qWaitForWindowExposed(&window)); - QApplication::processEvents(); - - const int subjectLeft = - view->columnViewportPosition(ThreadListModel::SubjectColumn); - QVERIFY2(subjectLeft > 40, - qPrintable(QStringLiteral("the subject column starts at x=%1, too " - "close to the left edge to tell a " - "row-wide strip from a subject-cell one") - .arg(subjectLeft))); - // The strip must have somewhere to paint that the subject cell does not - // reach, or this test cannot fail. - QVERIFY2(subjectLeft < view->viewport()->width(), - qPrintable(QStringLiteral("the subject column is off-screen " - "(x=%1, viewport %2), so nothing it " - "draws is measurable") - .arg(subjectLeft) - .arg(view->viewport()->width()))); - - QImage shot(view->viewport()->size(), QImage::Format_ARGB32); - shot.fill(Qt::transparent); - view->viewport()->render(&shot); - - // Count pixels matching the tag colours EXACTLY, not "saturated" pixels. - // A looser test counts the antialiased edge of the selection highlight - // blending into the background, which is several hundred distinct - // near-background colours and passes whatever the strip does. Both earlier - // versions of this test did precisely that. - QSet<QRgb> pillColours; - const QVariantList colours = - model->index(0, ThreadListModel::SubjectColumn) - .data(ThreadListModel::PillColoursRole).toList(); - QVERIFY2(!colours.isEmpty(), "the model supplied no pill colours"); - for (const QVariant &colour : colours) - pillColours.insert(colour.value<QColor>().rgb()); - - const int rowHeight = threadRowHeight(view, 0); - QVERIFY(rowHeight > 0); - - int chipPixels = 0; - for (int y = 0; y < qMin(rowHeight, shot.height()); ++y) { - for (int x = 0; x < qMin(subjectLeft, shot.width()); ++x) { - if (pillColours.contains(shot.pixel(x, y) | 0xff000000)) - ++chipPixels; - } - } - - QVERIFY2(chipPixels > 0, - "no pill-coloured pixels left of the subject column: the strip is " - "still confined to that cell rather than spanning the row"); -} - void TestMainWindow::childRowsAreIndentedUnderTheirThread() { const Config config; @@ -661,14 +588,14 @@ void TestMainWindow::childRowsAreIndentedUnderTheirThread() view->expand(root); QApplication::processEvents(); - // Measured on the TREE POSITION column, not on column 0. A QTreeView - // indents only the column carrying the expander, verified against Qt 6.11: - // with setTreePosition(4), column 0 reports the same left edge for a thread - // and its reply (0 and 0) while column 4 reports 420 and 440. Asserting on - // column 0 therefore fails against a perfectly indented tree. - const int treeColumn = ThreadListModel::SubjectColumn; - const QModelIndex rootCell = model->index(0, treeColumn, QModelIndex()); - const QModelIndex child = model->index(0, treeColumn, root); + // One column, and setIndentation(0): Qt indents nothing, CardLayout draws + // the indent itself. So visualRect reports the SAME rect for a thread and + // its reply, and the indent has to be read off the layout rather than off + // the geometry. That is the trap CLAUDE.md records in reverse: there, + // visualRect reported an indent the text did not have; here it reports + // none while the text is indented. + const QModelIndex rootCell = model->index(0, 0, QModelIndex()); + const QModelIndex child = model->index(0, 0, root); QVERIFY(child.isValid()); // Guards before the claim: a probe that cannot see both rows can report @@ -679,55 +606,84 @@ void TestMainWindow::childRowsAreIndentedUnderTheirThread() "the reply row has no height: it is collapsed or off-screen, and " "an indent test against it would pass without drawing anything"); - QVERIFY2(view->visualRect(child).left() > view->visualRect(rootCell).left(), - "the reply is not indented relative to its thread"); - - // The geometry being indented is NOT the same as the reply LOOKING - // indented, and asserting only the former shipped a build with no visible - // nesting at all. A thread row draws an account chip before its subject and - // a reply row does not, so the reply's text starts about a chip's width to - // the left of the thread's; at Qt's default 20px indent that difference - // swallows the shift entirely. + // The indent is NOT in the geometry. setIndentation(0) means visualRect + // reports the same left edge for both rows, deliberately: CardLayout draws + // the indent inside the card's own rect. Asserting on visualRect here + // would fail against a perfectly indented list, which is the mirror of the + // trap CLAUDE.md records for item 20, where visualRect reported an indent + // the text did not have. // - // So the real property: where the TEXT lands. The reply's subject must - // begin to the right of the thread's, which is what the eye reads as - // nesting. - const int chipWidth = - TagChip::sizeFor(QFontMetrics(view->font()), - model->data(rootCell, ThreadListModel::AccountLabelRole) - .toString()).width(); - QVERIFY2(chipWidth > 0, - "the thread row has no account chip, so this test cannot measure " - "the offset it is meant to compensate for"); - - const int threadTextLeft = view->visualRect(rootCell).left() + chipWidth; - QVERIFY2(view->visualRect(child).left() > threadTextLeft, + // So the real property, as before: where the TEXT lands. It is read off + // the layout, which is what the delegate paints from. + CardLayout::Input threadIn; + threadIn.isMessage = false; + threadIn.depth = 0; + CardLayout::Input replyIn; + replyIn.isMessage = true; + replyIn.depth = + model->data(child, ThreadListModel::MessageDepthRole).toInt(); + QVERIFY2(replyIn.depth > 0, + "the reply reports depth 0, so there is no nesting to measure"); + + const QRect rect = view->visualRect(rootCell); + const CardLayout threadCard = + CardLayout::compute(threadIn, rect, view->font()); + const CardLayout replyCard = + CardLayout::compute(replyIn, rect, view->font()); + + QVERIFY2(replyCard.contentLeft > threadCard.contentLeft, qPrintable(QStringLiteral("the reply's text starts at x=%1, not " - "right of the thread's text at x=%2: the " - "indent does not beat the account chip " - "and the nesting is invisible") - .arg(view->visualRect(child).left()) - .arg(threadTextLeft))); + "right of the thread's at x=%2: the " + "nesting is invisible") + .arg(replyCard.contentLeft) + .arg(threadCard.contentLeft))); + + // And the spine that makes the nesting read as one block rather than as an + // arbitrary offset. + QCOMPARE(replyCard.spines.size(), replyIn.depth); +} + +void TestMainWindow::cardsNeverScrollSideways() +{ + const Config config; + MainWindow window(config); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + auto *view = window.findChild<ThreadListView *>(); + QVERIFY(view); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + // A long subject, so the guard below is not vacuous: this is exactly the + // content that used to make the subject column wider than the viewport. + model->appendBatch({ makeThread( + QStringLiteral("t1"), + QStringList{ QStringLiteral("inbox") }) }); + QApplication::processEvents(); + + // Item 51: clicking a row used to scroll the list sideways, because the + // subject column was wider than the viewport and auto-scroll brought the + // clicked index fully into view. A card is exactly viewport width, so + // there is nowhere to scroll to. + QVERIFY2(view->visualRect(model->index(0, 0)).height() > 0, + "no card is drawn, so there is no layout to assert about"); + QCOMPARE(view->horizontalScrollBar()->minimum(), + view->horizontalScrollBar()->maximum()); } void TestMainWindow::aThreadWithRepliesDrawsAVisibleExpander() { // The expander is the ONLY thing saying a thread can be opened, and it took - // four wrong attempts to get on screen, each of which looked correct in - // code: - // - // - QTreeView::drawBranches, the documented hook, runs BEFORE the row's - // cells, so with the expander on a content column the delegate's own - // background paints over it. A 60-pixel triangle survived as 8. - // - Sizing it from the row rather than the branch rect put most of it - // outside that rect. - // - Moving it into the delegate but calling it from only one of the two - // branches left every real row without one, since every real row has an - // account chip and takes the other branch. + // four wrong attempts to get on screen before item 53, each of which looked + // correct in code and none of which a geometry or role assertion could see. + // So this counts painted pixels. // - // None of those is visible to a test that asserts on geometry or on model - // roles, so this one counts painted pixels of the palette colour the glyph - // is drawn in. + // Painted through the DELEGATE rather than through viewport()->render(). + // The viewport render returns a blank image here: CLAUDE.md records that it + // does so in several ordinary situations, and this test proved it again, + // reporting zero ink over a card the delegate demonstrably paints 2183 + // pixels into. A probe that sees nothing cannot report on anything. const Config config; MainWindow window(config); @@ -737,7 +693,7 @@ void TestMainWindow::aThreadWithRepliesDrawsAVisibleExpander() QVERIFY(view); // Two threads: one with replies, one without. The second is the control, - // and without it a test that counts text pixels would pass on any row. + // and without it a test that counts ink would pass on any card. ThreadSummary withReplies = makeThread( QStringLiteral("t1"), QStringList{ TagColors::tagForAccountKey(QStringLiteral("work")) }); @@ -748,61 +704,66 @@ void TestMainWindow::aThreadWithRepliesDrawsAVisibleExpander() lone.totalCount = 1; model->appendBatch({ withReplies, lone }); - window.resize(1400, 300); - window.show(); - QVERIFY(QTest::qWaitForWindowExposed(&window)); - QApplication::processEvents(); - - const QModelIndex first = - model->index(0, ThreadListModel::SubjectColumn, QModelIndex()); - const QModelIndex second = - model->index(1, ThreadListModel::SubjectColumn, QModelIndex()); - - // Guards: both rows on screen, and the model agreeing about which has - // replies. Without these a zero count could mean anything. - QVERIFY2(view->visualRect(first).height() > 0, "the first row is not drawn"); - QVERIFY2(view->visualRect(second).height() > 0, - "the control row is not drawn"); - QVERIFY(model->data(first, ThreadListModel::HasRepliesRole).toBool()); - QVERIFY(!model->data(second, ThreadListModel::HasRepliesRole).toBool()); - - QImage shot(view->viewport()->size(), QImage::Format_ARGB32); - shot.fill(Qt::transparent); - view->viewport()->render(&shot); - - // The exact colour the glyph is filled with, matched exactly rather than by - // a brightness threshold, which would count antialiased subject text. - const QRgb glyph = view->palette().color(QPalette::Text).rgb(); - - // Only the strip in front of the subject text, so the subject's own glyphs - // cannot be counted. kExpanderWidth is the room the delegate reserves. - const auto countGlyphPixels = [&](const QModelIndex &index) { - const QRect rect = view->visualRect(index); + const QModelIndex first = model->index(0, 0, QModelIndex()); + const QModelIndex second = model->index(1, 0, QModelIndex()); + + // Guards: the model agrees about which thread has replies, and only that + // one is offered an expander at all. + QCOMPARE(model->data(first, ThreadListModel::ReplyCountRole).toInt(), 2); + QCOMPARE(model->data(second, ThreadListModel::ReplyCountRole).toInt(), 0); + + const QFont font = view->font(); + const int height = CardLayout::heightFor(font); + + const auto inkInExpander = [&](const QModelIndex &index) { + QImage shot(400, height, QImage::Format_ARGB32); + shot.fill(Qt::white); + QPainter painter(&shot); + QStyleOptionViewItem option; + option.rect = QRect(0, 0, 400, height); + option.font = font; + option.palette = QApplication::palette(); + option.state = QStyle::State_Enabled; + CardDelegate delegate; + delegate.paint(&painter, option, index); + painter.end(); + + const QRect rect = CardDelegate::expanderRectFor(option, index); int found = 0; - for (int y = rect.top(); y < qMin(rect.bottom(), shot.height()); ++y) { - for (int x = rect.left(); - x < qMin(rect.left() + SubjectDelegate::kExpanderWidth, - shot.width()); + for (int y = rect.top(); y <= rect.bottom() && y < shot.height(); ++y) { + for (int x = rect.left(); x <= rect.right() && x < shot.width(); ++x) { - if ((shot.pixel(x, y) | 0xff000000) == (glyph | 0xff000000)) + if ((shot.pixel(x, y) | 0xff000000) != 0xffffffffu) ++found; } } - return found; + + // Guard on the probe itself: prove it can see the card's own text + // before trusting it about the expander. A probe that finds no ink + // anywhere reports "nothing was drawn" whatever the delegate did. + int anyInk = 0; + for (int y = 0; y < shot.height(); ++y) + for (int x = 0; x < shot.width(); ++x) + if ((shot.pixel(x, y) | 0xff000000) != 0xffffffffu) + ++anyInk; + return std::pair<int, int>(found, anyInk); }; - const int drawn = countGlyphPixels(first); - const int control = countGlyphPixels(second); + const auto [drawn, drawnAnywhere] = inkInExpander(first); + const auto [control, controlAnywhere] = inkInExpander(second); - QVERIFY2(drawn > 12, - qPrintable(QStringLiteral("only %1 expander pixels: the glyph is " - "clipped or painted over, which is how " - "it shipped as an invisible dot") - .arg(drawn))); + QVERIFY2(drawnAnywhere > 0 && controlAnywhere > 0, + "the probe finds no ink on either card, so it cannot report on " + "the expander either"); - // The control must have none, or the count above is measuring something - // every row draws. - QCOMPARE(control, 0); + QVERIFY2(drawn > 12, + qPrintable(QStringLiteral("only %1 pixels in the expander's rect: " + "the reply count is clipped or painted " + "over").arg(drawn))); + QVERIFY2(control == 0, + qPrintable(QStringLiteral("a thread with no replies drew %1 " + "pixels where an expander would go") + .arg(control))); } void TestMainWindow::selectingAThreadRowNamesHowManyMessagesItStandsFor() @@ -1110,7 +1071,7 @@ void TestMainWindow::clickingTheExpanderTogglesTheThread() const QModelIndex root = model->index(0, 0, QModelIndex()); const QModelIndex subject = - model->index(0, ThreadListModel::SubjectColumn, QModelIndex()); + model->index(0, 0, QModelIndex()); const QRect rect = view->visualRect(subject); // Guards: the row is drawn, it claims to have replies, and it starts @@ -1119,11 +1080,15 @@ void TestMainWindow::clickingTheExpanderTogglesTheThread() QVERIFY(model->data(subject, ThreadListModel::HasRepliesRole).toBool()); QVERIFY(!view->isExpanded(root)); - // Aimed at the glyph itself: the delegate reserves kExpanderWidth at the - // left of the subject cell and centres the triangle in it. - const QPoint hit(rect.left() + SubjectDelegate::kExpanderWidth / 2, - rect.top() + SubjectDelegate::kRowPadding - + QFontMetrics(view->font()).height() / 2); + // Aimed at the rect the delegate reports, not at one reconstructed here: + // the drawn target and the clickable one cannot drift if both come from + // the same call. + QStyleOptionViewItem option; + option.rect = rect; + option.font = view->font(); + const QRect expander = CardDelegate::expanderRectFor(option, subject); + QVERIFY2(!expander.isEmpty(), "the card offers no expander to click"); + const QPoint hit = expander.center(); QTest::mouseClick(view->viewport(), Qt::LeftButton, Qt::NoModifier, hit); QApplication::processEvents(); @@ -1177,7 +1142,7 @@ void TestMainWindow::replyRowsKeepTheirTextUnderTheThreadLine() QApplication::processEvents(); const QModelIndex child = - model->index(0, ThreadListModel::AuthorsColumn, root); + model->index(0, 0, root); const QRect rect = view->visualRect(child); QVERIFY2(rect.height() > 0, "the reply row is not on screen"); @@ -1203,121 +1168,6 @@ void TestMainWindow::replyRowsKeepTheirTextUnderTheThreadLine() .arg(textPixels))); } -void TestMainWindow::noTagStripIsPaintedUnderAMessageRow() -{ - // The strip is a row-wide band of the THREAD's tags. Painted under every - // reply as well it would stripe the list and repeat identical tags down the - // whole expansion. - // - // TWO independent guards stop that, and this test is aimed at the SECOND: - // the model returns no pills for a child row, and the view skips child rows - // in its walk. Asserting against the real model tests only the first, and - // the view's guard can be deleted without the test noticing: verified by - // mutation, which passed with the skip removed. So the model is replaced - // here by one that hands out pills for EVERY row, thread and reply alike, - // leaving the view's own skip as the only thing that can keep the reply - // rows clean. - /// Hands out the same pills for a message row as for a thread row, which - /// the real model never does. Without this the view's skip is unobservable. - class PillsEverywhereModel : public ThreadListModel - { - public: - QVariant data(const QModelIndex &index, int role) const override - { - if (role == PillTagsRole) { - return QStringList{ QStringLiteral("mailing-list/SBo"), - QStringLiteral("signed") }; - } - if (role == PillColoursRole) { - return QVariantList{ QVariant::fromValue(QColor(Qt::magenta)), - QVariant::fromValue(QColor(Qt::cyan)) }; - } - return ThreadListModel::data(index, role); - } - }; - - PillsEverywhereModel model; - ThreadListView view; - view.setModel(&model); - view.setTreePosition(ThreadListModel::SubjectColumn); - view.setUniformRowHeights(true); - - // The delegates MainWindow installs, and not optional here. The strip's - // band is measured against SubjectDelegate::rowHeightFor; without the - // delegate the rows take the default height, the band overflows into the - // row below, and the thread's own strip paints across the reply. That - // reads exactly like a missing skip in the walk and is not one. - view.setItemDelegate(new RowStyleDelegate(&view)); - view.setItemDelegateForColumn(ThreadListModel::SubjectColumn, - new SubjectDelegate(&view)); - view.setColumnWidth(ThreadListModel::AttachmentColumn, 28); - view.setColumnWidth(ThreadListModel::FlagColumn, 28); - view.setColumnWidth(ThreadListModel::DateColumn, 130); - view.setColumnWidth(ThreadListModel::AuthorsColumn, 180); - view.setColumnWidth(ThreadListModel::SubjectColumn, 520); - - ThreadSummary thread = makeThread(QStringLiteral("t1"), {}); - thread.tags = QStringList{ QStringLiteral("mailing-list/SBo"), - QStringLiteral("signed") }; - model.appendBatch({ thread }); - - MessageNode first; - first.messageId = QStringLiteral("m0@example.org"); - first.threadId = QStringLiteral("t1"); - first.depth = 0; - MessageNode reply; - reply.messageId = QStringLiteral("m1@example.org"); - reply.threadId = QStringLiteral("t1"); - reply.depth = 1; - model.setThreadMessages(QStringLiteral("t1"), { first, reply }); - - view.resize(1400, 300); - view.show(); - QVERIFY(QTest::qWaitForWindowExposed(&view)); - - const QModelIndex root = model.index(0, 0, QModelIndex()); - view.expand(root); - QApplication::processEvents(); - - const QModelIndex child = model.index(0, 0, root); - const QRect childRect = view.visualRect(child); - QVERIFY2(childRect.height() > 0, "the reply row is not on screen"); - - // The exact colours the stub supplies, so an antialiased edge of anything - // else cannot be counted as a pill. - QSet<QRgb> pillColours; - pillColours.insert(QColor(Qt::magenta).rgb()); - pillColours.insert(QColor(Qt::cyan).rgb()); - - QImage shot(view.viewport()->size(), QImage::Format_ARGB32); - shot.fill(Qt::transparent); - view.viewport()->render(&shot); - - // Guard proving the probe can see pills at all: the THREAD row must have - // them, or a zero count under the reply proves nothing about the reply. - const QRect rootRect = view.visualRect(root); - int threadPills = 0; - for (int y = rootRect.top(); y < qMin(rootRect.bottom(), shot.height()); ++y) { - for (int x = 0; x < shot.width(); ++x) { - if (pillColours.contains(shot.pixel(x, y) | 0xff000000)) - ++threadPills; - } - } - QVERIFY2(threadPills > 0, - "no pill pixels under the THREAD row either, so this probe cannot " - "tell a missing strip from a broken render"); - - int replyPills = 0; - for (int y = childRect.top(); y < qMin(childRect.bottom(), shot.height()); ++y) { - for (int x = 0; x < shot.width(); ++x) { - if (pillColours.contains(shot.pixel(x, y) | 0xff000000)) - ++replyPills; - } - } - - QCOMPARE(replyPills, 0); -} - void TestMainWindow::aSelectedReadThreadIsNotDimmedIntoTheHighlight() { // Read threads carry a dimmed Qt::ForegroundRole, blended against the @@ -1383,15 +1233,15 @@ void TestMainWindow::aSelectedReadThreadIsNotDimmedIntoTheHighlight() QVERIFY2(delegate, "the thread view has no styled delegate"); const QModelIndex index = - model->index(0, ThreadListModel::SubjectColumn); + model->index(0, 0); // initStyleOption is protected, so the resolved palette is reached the way // the painter does: through a subclass that exposes it. - struct Probe : SubjectDelegate { - using SubjectDelegate::initStyleOption; + struct Probe : CardDelegate { + using CardDelegate::initStyleOption; }; const auto *probe = static_cast<const Probe *>( - static_cast<const SubjectDelegate *>(delegate)); + static_cast<const CardDelegate *>(delegate)); probe->initStyleOption(&selected, index); probe->initStyleOption(&unselected, index); diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp index 2e3eede..49b8894 100644 --- a/tests/test_threadlistmodel.cpp +++ b/tests/test_threadlistmodel.cpp @@ -47,22 +47,20 @@ private slots: void appendingEmptyBatchIsNoOp(); void clearResetsModel(); void reportsSubjectAndAuthors(); - void subjectShowsMessageCountOnlyForRealThreads(); + void theReplyCountExcludesTheRootMessage(); void unreadThreadsRenderBold(); void readThreadsAreDimmedAndUnreadAreNot(); void flaggedThreadsShowAStar(); void pillTagsExcludeWhatTheRowAlreadyShows(); - void theStarColumnIsNarrowAndCarriesNoText(); void theUnreadCueDoesNotDependOnFontWeight(); void aDoomedThreadKeepsItsContrastEvenWhenRead(); - void tagsAreTheFirstColumnAndSubjectTheLast(); void accountTagBecomesAChipLabel(); void unreadStylingSurvivesAnAccountChip(); void accountChipUsesTheConfiguredColour(); void deletedThreadsAreRedAndStruckThrough(); - void attachmentColumnIsFirstAndMarksOnlyTaggedThreads(); + void attachmentIsMarkedOnlyOnTaggedThreads(); void spamThreadsAreOrangeAndStruckThrough(); - void doomedStylingCoversEveryColumn(); + void doomedStylingCoversTheWholeCard(); void ordinaryThreadsCarryNoRowColour(); void threadIdIsReachableFromAnIndex(); void invalidIndexesReturnNothing(); @@ -120,7 +118,7 @@ void TestThreadListModel::repliesBecomeChildRowsUnderTheirThread() QCOMPARE(model.rowCount(root), 2); const QModelIndex child = - model.index(0, ThreadListModel::SubjectColumn, root); + model.index(0, 0, root); QVERIFY(child.isValid()); QCOMPARE(model.parent(child), model.index(0, 0, QModelIndex())); @@ -159,20 +157,17 @@ void TestThreadListModel::messageRowsShowTheirOwnSenderAndSubject() QStringLiteral("Re: A subject")) }); const QModelIndex root = model.index(0, 0, QModelIndex()); - const QModelIndex authors = - model.index(0, ThreadListModel::AuthorsColumn, root); - const QModelIndex subject = - model.index(0, ThreadListModel::SubjectColumn, root); + const QModelIndex reply = model.index(0, 0, root); - QCOMPARE(model.data(authors, Qt::DisplayRole).toString(), + QCOMPARE(model.data(reply, ThreadListModel::SendersRole).toString(), QStringLiteral("Bob <bob@example.org>")); - QCOMPARE(model.data(subject, Qt::DisplayRole).toString(), + QCOMPARE(model.data(reply, ThreadListModel::SubjectRole).toString(), QStringLiteral("Re: A subject")); // No tag strip under a child row. The strip is a row-wide band carrying the // THREAD's tags; one under every reply would stripe the list and repeat the // same tags down the whole expansion. - QVERIFY(model.data(subject, ThreadListModel::PillTagsRole) + QVERIFY(model.data(reply, ThreadListModel::PillTagsRole) .toStringList().isEmpty()); } @@ -397,10 +392,10 @@ void TestThreadListModel::rootRowsSurviveTheTreeConversion() // A tree model reports its roots under an INVALID parent. QCOMPARE(model.rowCount(QModelIndex()), 1); - QCOMPARE(model.columnCount(QModelIndex()), ThreadListModel::ColumnCount); + QCOMPARE(model.columnCount(QModelIndex()), 1); const QModelIndex root = - model.index(0, ThreadListModel::SubjectColumn, QModelIndex()); + model.index(0, 0, QModelIndex()); QVERIFY(root.isValid()); QVERIFY(!model.parent(root).isValid()); QCOMPARE(model.data(root, ThreadListModel::ThreadIdRole).toString(), @@ -422,7 +417,7 @@ void TestThreadListModel::startsEmpty() { ThreadListModel model; QCOMPARE(model.rowCount(), 0); - QCOMPARE(model.columnCount(), ThreadListModel::ColumnCount); + QCOMPARE(model.columnCount(), 1); } void TestThreadListModel::appendsBatches() @@ -467,21 +462,21 @@ void TestThreadListModel::reportsSubjectAndAuthors() ThreadListModel model; model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("hello")) }); - const QModelIndex authors = model.index(0, ThreadListModel::AuthorsColumn); - QCOMPARE(model.data(authors, Qt::DisplayRole).toString(), + // One index, every field, by role. The card draws them all at once, so + // reading them through Qt::DisplayRole as five columns did is no longer + // possible: DisplayRole answers the subject alone. + const QModelIndex card = model.index(0, 0); + QCOMPARE(model.data(card, ThreadListModel::SendersRole).toString(), QStringLiteral("Alice")); + QVERIFY(model.data(card, ThreadListModel::DateRole).toDateTime().isValid()); + QCOMPARE(model.data(card, ThreadListModel::SubjectRole).toString(), + QStringLiteral("hello")); - const QModelIndex date = model.index(0, ThreadListModel::DateColumn); - QVERIFY(!model.data(date, Qt::DisplayRole).toString().isEmpty()); - - // Tags are no longer a column; they reach the strip under the message - // pane through a role instead. - const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); - QCOMPARE(model.data(subject, ThreadListModel::TagsRole).toStringList(), + QCOMPARE(model.data(card, ThreadListModel::TagsRole).toStringList(), QStringList({ QStringLiteral("inbox"), QStringLiteral("unread") })); } -void TestThreadListModel::subjectShowsMessageCountOnlyForRealThreads() +void TestThreadListModel::theReplyCountExcludesTheRootMessage() { ThreadListModel model; @@ -491,12 +486,18 @@ void TestThreadListModel::subjectShowsMessageCountOnlyForRealThreads() multi.totalCount = 4; model.appendBatch({ single, multi }); - QCOMPARE(model.data(model.index(0, ThreadListModel::SubjectColumn), - Qt::DisplayRole).toString(), - QStringLiteral("alone")); - QCOMPARE(model.data(model.index(1, ThreadListModel::SubjectColumn), - Qt::DisplayRole).toString(), - QStringLiteral("group (4)")); + // The count used to be a "(4)" suffix on the subject. It is the expander + // on the card's second line now, and it counts REPLIES: totalCount + // includes the root message, which is the card itself. + QCOMPARE(model.data(model.index(0, 0), + ThreadListModel::ReplyCountRole).toInt(), 0); + QCOMPARE(model.data(model.index(1, 0), + ThreadListModel::ReplyCountRole).toInt(), 3); + + // And the subject is bare, with no count spliced into it. + QCOMPARE(model.data(model.index(1, 0), + ThreadListModel::SubjectRole).toString(), + QStringLiteral("group")); } void TestThreadListModel::unreadThreadsRenderBold() @@ -507,11 +508,11 @@ void TestThreadListModel::unreadThreadsRenderBold() model.appendBatch({ read, makeThread(QStringLiteral("t2"), QStringLiteral("unread")) }); const QVariant readFont = - model.data(model.index(0, ThreadListModel::SubjectColumn), Qt::FontRole); + model.data(model.index(0, 0), Qt::FontRole); QVERIFY(!readFont.isValid()); const QVariant unreadFont = - model.data(model.index(1, ThreadListModel::SubjectColumn), Qt::FontRole); + model.data(model.index(1, 0), Qt::FontRole); QVERIFY(unreadFont.isValid()); QVERIFY(unreadFont.value<QFont>().bold()); } @@ -534,10 +535,10 @@ void TestThreadListModel::readThreadsAreDimmedAndUnreadAreNot() { read, makeThread(QStringLiteral("t2"), QStringLiteral("unread")) }); const QVariant readFg = - model.data(model.index(0, ThreadListModel::SubjectColumn), + model.data(model.index(0, 0), Qt::ForegroundRole); const QVariant unreadFg = - model.data(model.index(1, ThreadListModel::SubjectColumn), + model.data(model.index(1, 0), Qt::ForegroundRole); QVERIFY2(readFg.isValid(), "a read thread carries no dimming"); @@ -559,16 +560,17 @@ void TestThreadListModel::flaggedThreadsShowAStar() QStringLiteral("flagged") }; model.appendBatch({ plain, starred }); - const QString none = - model.data(model.index(0, ThreadListModel::FlagColumn), - Qt::DisplayRole).toString(); - const QString star = - model.data(model.index(1, ThreadListModel::FlagColumn), - Qt::DisplayRole).toString(); - - QVERIFY2(none.isEmpty(), "an unflagged thread shows something in the column"); - QVERIFY2(!star.isEmpty(), "a flagged thread shows nothing"); - QCOMPARE(star, ThreadListModel::flagGlyph()); + QVERIFY2(!model.data(model.index(0, 0), + ThreadListModel::IsFlaggedRole).toBool(), + "an unflagged thread reports itself flagged"); + QVERIFY2(model.data(model.index(1, 0), + ThreadListModel::IsFlaggedRole).toBool(), + "a flagged thread does not report itself flagged"); + + // The glyph the delegate draws from that flag must be something a font can + // render: an unrenderable codepoint shows as tofu, which reads as + // breakage rather than as a mark. + QVERIFY(!ThreadListModel::flagGlyph().isEmpty()); } void TestThreadListModel::pillTagsExcludeWhatTheRowAlreadyShows() @@ -592,7 +594,7 @@ void TestThreadListModel::pillTagsExcludeWhatTheRowAlreadyShows() model.appendBatch({ thread }); const QStringList pills = - model.data(model.index(0, ThreadListModel::SubjectColumn), + model.data(model.index(0, 0), ThreadListModel::PillTagsRole).toStringList(); QVERIFY2(pills.contains(QStringLiteral("SBo")), qPrintable(pills.join(','))); @@ -622,29 +624,6 @@ void TestThreadListModel::pillTagsExcludeWhatTheRowAlreadyShows() QCOMPARE(pills, sorted); } -void TestThreadListModel::theStarColumnIsNarrowAndCarriesNoText() -{ - // A marker column, like the paperclip beside it: centred, and never - // carrying the subject or anything else that would want width. - ThreadListModel model; - ThreadSummary starred = makeThread(QStringLiteral("t1"), - QStringLiteral("starred")); - starred.tags = QStringList{ QStringLiteral("flagged") }; - model.appendBatch({ starred }); - - const QModelIndex index = model.index(0, ThreadListModel::FlagColumn); - QCOMPARE(model.data(index, Qt::TextAlignmentRole).toInt(), - int(Qt::AlignCenter)); - - // The glyph is one character, whether it is the star or its fallback: a - // column sized for a marker cannot hold a word. - QCOMPARE(ThreadListModel::flagGlyph().size(), 1); - - // And it says what it means, for anyone who cannot tell the glyph apart - // from the paperclip beside it. - QVERIFY(!model.data(index, Qt::ToolTipRole).toString().isEmpty()); -} - void TestThreadListModel::theUnreadCueDoesNotDependOnFontWeight() { // The property that matters, stated directly: strip every font from the @@ -657,17 +636,14 @@ void TestThreadListModel::theUnreadCueDoesNotDependOnFontWeight() model.appendBatch( { read, makeThread(QStringLiteral("t2"), QStringLiteral("unread")) }); - for (int column = 0; column < ThreadListModel::ColumnCount; ++column) { - const QVariant readFg = - model.data(model.index(0, column), Qt::ForegroundRole); - const QVariant unreadFg = - model.data(model.index(1, column), Qt::ForegroundRole); + const QVariant readFg = + model.data(model.index(0, 0), Qt::ForegroundRole); + const QVariant unreadFg = + model.data(model.index(1, 0), Qt::ForegroundRole); - QVERIFY2(readFg != unreadFg, - qPrintable(QStringLiteral("column %1 renders read and unread " - "identically once the font is " - "ignored").arg(column))); - } + QVERIFY2(readFg != unreadFg, + "read and unread cards render identically once the font is " + "ignored"); } void TestThreadListModel::aDoomedThreadKeepsItsContrastEvenWhenRead() @@ -684,32 +660,11 @@ void TestThreadListModel::aDoomedThreadKeepsItsContrastEvenWhenRead() model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {}); - const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + const QModelIndex subject = model.index(0, 0); QCOMPARE(model.data(subject, Qt::ForegroundRole).value<QBrush>().color(), QColor(Qt::white)); } -void TestThreadListModel::tagsAreTheFirstColumnAndSubjectTheLast() -{ - // Subject stretches to fill the view, so whatever sits after it is pushed - // off-screen. Tags used to be there, which is why acting on a thread - // looked like it did nothing: the only column that changed was invisible. - QCOMPARE(ThreadListModel::SubjectColumn, ThreadListModel::ColumnCount - 1); - - ThreadListModel model; - model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("hello")) }); - QCOMPARE(model.headerData(ThreadListModel::SubjectColumn, Qt::Horizontal, - Qt::DisplayRole).toString(), - QStringLiteral("Subject")); - - // No tags column at all: spelling out a dozen tags per row consumed most - // of the list's width and was unreadable. - for (int column = 0; column < ThreadListModel::ColumnCount; ++column) { - QVERIFY(model.headerData(column, Qt::Horizontal, Qt::DisplayRole) - .toString() != QStringLiteral("Tags")); - } -} - void TestThreadListModel::accountTagBecomesAChipLabel() { // The account tag is a different taxonomy from a functional one: which @@ -721,7 +676,7 @@ void TestThreadListModel::accountTagBecomesAChipLabel() QStringLiteral("account-webmail-personal") }; model.appendBatch({ thread }); - const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + const QModelIndex subject = model.index(0, 0); QCOMPARE(model.data(subject, ThreadListModel::AccountLabelRole).toString(), QStringLiteral("webmail-personal")); QVERIFY(model.data(subject, ThreadListModel::AccountColourRole) @@ -732,7 +687,7 @@ void TestThreadListModel::accountTagBecomesAChipLabel() ThreadSummary untagged = makeThread(QStringLiteral("t2"), QStringLiteral("hi")); untagged.tags = QStringList{ QStringLiteral("inbox") }; plain.appendBatch({ untagged }); - QVERIFY(plain.data(plain.index(0, ThreadListModel::SubjectColumn), + QVERIFY(plain.data(plain.index(0, 0), ThreadListModel::AccountLabelRole).toString().isEmpty()); } @@ -748,7 +703,7 @@ void TestThreadListModel::unreadStylingSurvivesAnAccountChip() QStringLiteral("account-webmail-personal") }; model.appendBatch({ thread }); - const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + const QModelIndex subject = model.index(0, 0); QVERIFY(!model.data(subject, ThreadListModel::AccountLabelRole) .toString().isEmpty()); @@ -771,7 +726,7 @@ void TestThreadListModel::accountChipUsesTheConfiguredColour() thread.tags = QStringList{ QStringLiteral("account-webmail-personal") }; model.appendBatch({ thread }); - QCOMPARE(model.data(model.index(0, ThreadListModel::SubjectColumn), + QCOMPARE(model.data(model.index(0, 0), ThreadListModel::AccountColourRole).value<QColor>(), QColor(QStringLiteral("#cc0000"))); } @@ -783,7 +738,7 @@ void TestThreadListModel::deletedThreadsAreRedAndStruckThrough() thread.tags = QStringList{ QStringLiteral("inbox") }; model.appendBatch({ thread }); - const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + const QModelIndex subject = model.index(0, 0); QVERIFY(!model.data(subject, Qt::BackgroundRole).isValid()); model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {}); @@ -808,7 +763,7 @@ void TestThreadListModel::spamThreadsAreOrangeAndStruckThrough() model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("spam") }, {}); - const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + const QModelIndex subject = model.index(0, 0); QCOMPARE(model.data(subject, Qt::BackgroundRole).value<QBrush>().color(), ThreadListModel::spamColour()); QVERIFY(model.data(subject, Qt::FontRole).value<QFont>().strikeOut()); @@ -817,10 +772,12 @@ void TestThreadListModel::spamThreadsAreOrangeAndStruckThrough() QVERIFY(ThreadListModel::spamColour() != ThreadListModel::deletedColour()); } -void TestThreadListModel::doomedStylingCoversEveryColumn() +void TestThreadListModel::doomedStylingCoversTheWholeCard() { - // A cue on one column would vanish the moment that column scrolled out of - // view, which is the bug this whole change exists to fix. + // The cue is on the card itself. It used to be asserted per column, + // because a cue on one column vanished the moment that column scrolled out + // of view; one column cannot scroll away, but the roles still have to be + // answered or a deleted card looks untouched. ThreadListModel model; ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("doomed")); thread.tags = QStringList{ QStringLiteral("inbox") }; @@ -828,13 +785,11 @@ void TestThreadListModel::doomedStylingCoversEveryColumn() model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {}); - for (int column = 0; column < ThreadListModel::ColumnCount; ++column) { - const QModelIndex index = model.index(0, column); - QVERIFY2(model.data(index, Qt::BackgroundRole).isValid(), - qPrintable(QStringLiteral("column %1 has no background").arg(column))); - QVERIFY2(model.data(index, Qt::FontRole).value<QFont>().strikeOut(), - qPrintable(QStringLiteral("column %1 is not struck through").arg(column))); - } + const QModelIndex index = model.index(0, 0); + QVERIFY2(model.data(index, Qt::BackgroundRole).isValid(), + "a deleted card has no background"); + QVERIFY2(model.data(index, Qt::FontRole).value<QFont>().strikeOut(), + "a deleted card is not struck through"); } void TestThreadListModel::ordinaryThreadsCarryNoRowColour() @@ -848,7 +803,7 @@ void TestThreadListModel::ordinaryThreadsCarryNoRowColour() model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {}); model.applyTagChange(QStringLiteral("t1"), {}, { QStringLiteral("deleted") }); - const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + const QModelIndex subject = model.index(0, 0); QVERIFY(!model.data(subject, Qt::BackgroundRole).isValid()); const QVariant font = model.data(subject, Qt::FontRole); QVERIFY(!font.isValid() || !font.value<QFont>().strikeOut()); @@ -871,7 +826,7 @@ void TestThreadListModel::threadIdIsReachableFromAnIndex() model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("one")), makeThread(QStringLiteral("t2"), QStringLiteral("two")) }); - const QModelIndex index = model.index(1, ThreadListModel::SubjectColumn); + const QModelIndex index = model.index(1, 0); QCOMPARE(model.data(index, ThreadListModel::ThreadIdRole).toString(), QStringLiteral("t2")); } @@ -889,7 +844,7 @@ void TestThreadListModel::invalidIndexesReturnNothing() // the reset in clear() before data() ever sees it. data() still checks its // own bounds, but that guard is unreachable defence, not something these // assertions can falsify. - QVERIFY(!model.index(0, ThreadListModel::ColumnCount).isValid()); + QVERIFY(!model.index(0, 1).isValid()); QVERIFY(!model.index(5, 0).isValid()); QVERIFY(!model.index(-1, 0).isValid()); @@ -953,9 +908,9 @@ void TestThreadListModel::tagChangeSignalsExactlyTheChangedRow() const QModelIndex bottomRight = changed.first().at(1).value<QModelIndex>(); QCOMPARE(topLeft.row(), 1); QCOMPARE(bottomRight.row(), 1); + // One column, so the range is a single index: the card repaints whole. QCOMPARE(topLeft.column(), 0); - // The whole row repaints: unread state changes the font of every column. - QCOMPARE(bottomRight.column(), ThreadListModel::ColumnCount - 1); + QCOMPARE(bottomRight.column(), 0); } void TestThreadListModel::tagChangeForUnknownThreadIsIgnored() @@ -1006,12 +961,11 @@ void TestThreadListModel::modelPassesQtTester() model.clear(); } -void TestThreadListModel::attachmentColumnIsFirstAndMarksOnlyTaggedThreads() +void TestThreadListModel::attachmentIsMarkedOnlyOnTaggedThreads() { - // Leftmost, and narrow: the point is to see an attachment without opening - // the thread, which only works if the column is never scrolled away. - QCOMPARE(ThreadListModel::AttachmentColumn, 0); - + // The mark is drawn on the card's second line by CardDelegate. What the + // model owes it is the flag and the glyph, which is what this asserts: + // the column that used to carry it is gone. ThreadSummary plain = makeThread(QStringLiteral("t1"), QStringLiteral("no attachment")); ThreadSummary withFile = makeThread(QStringLiteral("t2"), @@ -1024,13 +978,12 @@ void TestThreadListModel::attachmentColumnIsFirstAndMarksOnlyTaggedThreads() model.appendBatch({ plain, withFile }); const QModelIndex plainCell = - model.index(0, ThreadListModel::AttachmentColumn); + model.index(0, 0); const QModelIndex fileCell = - model.index(1, ThreadListModel::AttachmentColumn); + model.index(1, 0); - QVERIFY(model.data(plainCell, Qt::DisplayRole).toString().isEmpty()); - QCOMPARE(model.data(fileCell, Qt::DisplayRole).toString(), - ThreadListModel::attachmentGlyph()); + QVERIFY(!model.data(plainCell, ThreadListModel::HasAttachmentRole).toBool()); + QVERIFY(model.data(fileCell, ThreadListModel::HasAttachmentRole).toBool()); // The glyph must be something a font can draw. An unrenderable codepoint // shows as a tofu box, which reads as breakage rather than as a marker. @@ -1040,11 +993,6 @@ void TestThreadListModel::attachmentColumnIsFirstAndMarksOnlyTaggedThreads() // have an attachment on hover. QVERIFY(model.data(plainCell, Qt::ToolTipRole).toString().isEmpty()); QVERIFY(!model.data(fileCell, Qt::ToolTipRole).toString().isEmpty()); - - // The header carries no text: a label would set a minimum width far wider - // than the icon and defeat the narrow column. - QVERIFY(model.headerData(ThreadListModel::AttachmentColumn, Qt::Horizontal, - Qt::DisplayRole).toString().isEmpty()); } void TestThreadListModel::modelHasOneColumn() |
