diff options
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 84 | ||||
| -rw-r--r-- | src/htmlbuilder.cpp | 255 | ||||
| -rw-r--r-- | src/htmlbuilder.h | 52 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 101 | ||||
| -rw-r--r-- | src/mainwindow.h | 37 | ||||
| -rw-r--r-- | src/messageview.cpp | 75 | ||||
| -rw-r--r-- | src/messageview.h | 30 | ||||
| -rw-r--r-- | src/notmuchworker.cpp | 30 | ||||
| -rw-r--r-- | src/notmuchworker.h | 14 | ||||
| -rw-r--r-- | src/resources.qrc | 5 | ||||
| -rw-r--r-- | tests/CMakeLists.txt | 8 | ||||
| -rw-r--r-- | tests/test_htmlbuilder.cpp | 171 | ||||
| -rw-r--r-- | tests/test_messageview.cpp | 74 | ||||
| -rw-r--r-- | tests/test_notmuchworker.cpp | 51 |
14 files changed, 982 insertions, 5 deletions
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md index cba4e81..e82c8fc 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md @@ -78,7 +78,7 @@ taking that too literally. | 27 | The UI cannot see a sync it did not start | feedback | S | **done** | | 28 | Re-adding `unread` counts 2 unsynced changes, not 0 | correctness | S | **done** | | 29 | Sync button stays enabled during a background sync | feedback | XS | **done** | -| 30 | The blank right pane is wasted space | presentation | M | open | +| 30 | The blank right pane is wasted space | presentation | M | **done** | | 31 | The quit prompt has no highlighted default button | discoverability | XS | **done** | | 32 | Esc does not blank the right pane | workflow | XS | **done** | | 33 | Status bar messages never expire | feedback | S | **done** | @@ -1712,6 +1712,88 @@ pane. The **helper counts are the largest piece**: three `notmuch count` calls crossing to the worker, plus a refresh policy, since a count goes stale the moment a tag is edited. +### Outcome (done 2026-08-07) + +Built as specified, and confirmed against the running application in both +themes. Decisions taken while building, and three defects worth recording. + +**The counts refresh when the pane is about to show**, chosen by the user over +refreshing on the tag-list triggers or only after a sync. `showPlaceholderPane()` +is the single route to a blank pane, so the numbers are fetched exactly when +they are about to be read and never in the background. A generation counter +discards a superseded reply, and `onCountsReady` repaints only while the +placeholder is still displayed, so a late answer cannot replace an opened +thread. Counts are of THREADS, matching what a click on the line produces. + +**The helper lines are real links**, since JavaScript is off in this profile and +a count cannot be clickable any other way. They carry a `qtmaildir-query:` URL +caught in `acceptNavigationRequest`, and **the handler is gated on the +placeholder actually being displayed**. A message body is attacker-controlled +HTML and can carry the same URL; without the gate a link in a stranger's mail +could drive the thread list. The consequence would be mild (a query runs, +nothing mutates or is sent) but it is a boundary worth keeping shut. +`showThread`, `clear` and `showError` all close the gate, and a test asserts it. + +**Sizes are clamped, not fixed and not fluid**, per the user's "a mix of both". +The pane is a splitter panel whose width runs from a couple of hundred pixels to +most of a screen. Measured: the wordmark renders 26px in a 300px pane and 57.6px +in a 900px one, with `bodyScrollW == clientW` at every width tested. + +#### Three defects, all invisible + +1. **Every CSS percentage was invalid, and the pane still looked plausible.** + The stylesheet was built with `QString::arg` and `%%` for each percentage, + but **`arg()` does not collapse `%%` into `%`**: the document reached the + browser carrying `50%%`, and every declaration containing one was dropped. + That silently disabled the grid mask, the glow and both radial gradients. + The pane rendered, and the flat result read as "close but not like the + mockup" rather than as a fault. Fixed by substituting **named tokens** + (`@ACCENT@`, `@GRID@`) with `replace()`, which cannot collide with a percent + sign. `placeholderStyleHasNoUnsubstitutedTokens` pins it. + +2. **A geometry probe confirmed the layout while that bug was live.** It + measured `.title`, `.content` and `.icon-tile`, none of which carry a + percentage, so it reported everything correct. This is the failure mode + `CLAUDE.md` already warns about, in a new costume: the probe found what it + looked for and was trusted to report what it never checked. **A probe over a + stylesheet must assert the properties that the suspected fault would break**, + not the ones that happen to be convenient to measure. + +3. **The font test passed against a broken build.** Pointing one `@font-face` + at a nonexistent resource survived it: the other face alone satisfied both + the `data:font/woff2` check and the document-size check. Rewritten to require + both faces with a payload each, and verified by mutation. + +#### The mockup's light values do not survive a real pane + +Only the dark set ported cleanly. Rendered side by side, three light values +failed, all of them contrast rather than hue, because the mockup is a full-bleed +1920x1080 render and this is a pane against white: + +- The grid at `#d9dfe8` on white is roughly a 2% luminance step and vanished + outright, where `#182840` on `#060b10` reads clearly at the same opacity. +- The glow **subtracts** light on a light background instead of adding it, so + 14% washed most of the pane purple. +- The tile at `#f0f3f7` inside a `#d9dfe8` border did not separate from the + background, leaving the icon floating. + +`glowAlpha` and `gridOpacity` are therefore per-set rather than shared, and +`tileBorder` is its own colour rather than reusing `grid`. Landed at +`#b9c4d4` @ 45% for the grid, 6% for the glow, `#c7d0dd` for the tile border. +The dark set is unchanged. + +**The mask and the glow are sized relative to the pane**, the one deliberate +departure from the mockup's numbers. Its `circle` (farthest-corner) mask +completes its fade inside a 1920x1080 frame; the same ratio in a ~990x650 pane +puts the fade past the corners, so the grid ran uniform to the edges. +`closest-side` pins it to the nearer edge, and the glow is `min(95%, 900px)` +rather than a flat 900px that was taller than a short pane. + +**`resources.qrc` is now compiled into every test binary.** Library code reads +`:/fonts/` and a qrc inside the static library registers from a global +initialiser the linker drops, so without this a test would exercise only the +missing-resource fallback and pass against a broken build. + ## 31. The quit prompt has no highlighted default button **Observed (user, 2026-08-04):** "quit popup has no Predefined answer (there's diff --git a/src/htmlbuilder.cpp b/src/htmlbuilder.cpp index c8f039f..30fab9f 100644 --- a/src/htmlbuilder.cpp +++ b/src/htmlbuilder.cpp @@ -19,8 +19,10 @@ #include "htmlbuilder.h" #include <QCoreApplication> +#include <QFile> #include <QGuiApplication> #include <QRegularExpression> +#include <QUrl> namespace { @@ -84,6 +86,259 @@ HtmlBuilder::Palette HtmlBuilder::paletteFrom(const QPalette &palette) return result; } +HtmlBuilder::BrandPalette HtmlBuilder::brandPaletteFrom(const QPalette &palette) +{ + // Base, the same surface paletteFrom() reads, so the placeholder and a + // rendered message never disagree about which way round the theme is. + const bool dark = palette.color(QPalette::Base).lightnessF() < 0.5; + + BrandPalette brand; + if (dark) { + brand.background = QColor(0x06, 0x0b, 0x10); + brand.backgroundIn = QColor(0x0c, 0x15, 0x20); + brand.grid = QColor(0x18, 0x28, 0x40); + brand.tile = QColor(0x10, 0x1e, 0x2d); + brand.tileBorder = QColor(0x18, 0x28, 0x40); + brand.accent = QColor(0xa8, 0x55, 0xf7); + brand.accentEdge = QColor(0x7c, 0x3a, 0xed); + brand.title = QColor(0xc4, 0xd6, 0xe8); + brand.subtitle = QColor(0x7a, 0x9b, 0xb8); + brand.glowAlpha = 16; + } else { + // **Not the mockup's light values as written.** Rendered side by side + // with the dark set, three of them did not survive contact with a real + // pane, because the same nominal contrast behaves differently against + // white than against near-black: + // + // - The grid at #d9dfe8 on white is about a 2% luminance step and + // vanished entirely, where #182840 on #060b10 reads clearly. Darkened + // here, and the opacity is raised for this set alone below. + // - The glow SUBTRACTS light on white instead of adding it, so 14% + // washed most of the pane purple rather than hinting at a glow. The + // dark set keeps 16 for the opposite reason. + // - The tile at #f0f3f7 inside a #d9dfe8 border did not separate from + // the background, leaving the icon floating. + // + // The hues are the mockup's throughout; only their strength changed. + brand.background = QColor(0xff, 0xff, 0xff); + brand.backgroundIn = QColor(0xf4, 0xf6, 0xfa); + brand.grid = QColor(0xb9, 0xc4, 0xd4); + brand.tile = QColor(0xf0, 0xf3, 0xf7); + brand.tileBorder = QColor(0xc7, 0xd0, 0xdd); + brand.accent = QColor(0x93, 0x33, 0xea); + brand.accentEdge = QColor(0x7c, 0x3a, 0xed); + brand.title = QColor(0x1f, 0x29, 0x37); + brand.subtitle = QColor(0x37, 0x41, 0x51); + brand.glowAlpha = 6; + brand.gridOpacity = 45; + } + return brand; +} + +namespace { + +/// A bundled font as a data: URI. +/// +/// The mockup reaches Google Fonts with an @import, which the interceptor +/// blocks by design and which would be a network request from a mail client +/// besides. The faces ship in the binary and are inlined here, so the document +/// fetches nothing at all. +QString fontDataUri(const char *resource) +{ + QFile file(QString::fromLatin1(resource)); + if (!file.open(QIODevice::ReadOnly)) { + // Falls back to the generic family in the font stack rather than + // failing to render. A missing resource is a build fault, not + // something the user can act on mid-session. + return QString(); + } + return QStringLiteral("data:font/woff2;base64,") + + QString::fromLatin1(file.readAll().toBase64()); +} + +/// rgba() from a colour and a percentage, for the translucent washes. +QString rgba(const QColor &c, int percent) +{ + return QStringLiteral("rgba(%1,%2,%3,%4)") + .arg(c.red()).arg(c.green()).arg(c.blue()) + .arg(percent / 100.0); +} + +/// The placeholder's stylesheet. +/// +/// Sizes are clamped rather than fixed or purely fluid: this is a splitter +/// panel whose width varies from a couple of hundred pixels to most of a +/// screen. A fixed wordmark is lost in a wide pane and overflows a narrow one; +/// a purely fluid one is unreadable at one end and absurd at the other. The +/// vw term scales it, the clamp bounds stop it going either way. +/// +/// Substituted by NAME, not by QString::arg positions. The stylesheet is full +/// of CSS percentages, and **arg() does NOT collapse "%%" into "%"**: every +/// percentage written that way stayed literally "50%%", which is invalid, so +/// the browser dropped each declaration containing one. That silently killed +/// the mask, the glow and both radial gradients while the pane still rendered +/// and still looked plausible, and a probe measuring only the properties +/// without percentages reported everything correct. Named tokens cannot +/// collide with a percent sign at all. +const char *kPlaceholderStyleTemplate = R"CSS( +@font-face { font-family: 'Oxanium qtmaildir'; font-weight: 800; + font-style: normal; font-display: block; + src: url('@FONT_OXANIUM@') format('woff2'); } +@font-face { font-family: 'Plex qtmaildir'; font-weight: 400; + font-style: normal; font-display: block; + src: url('@FONT_PLEX@') format('woff2'); } +* { margin: 0; padding: 0; box-sizing: border-box; } +html, body { width: 100%; height: 100%; overflow: hidden; + background: @BG@; } +.bg { position: absolute; inset: 0; + background: radial-gradient(circle at 30% 20%, @BG_IN@ 0%, @BG@ 55%, @BG@ 100%); + display: flex; align-items: center; justify-content: center; } +/* The grid and the glow are sized RELATIVE TO THE PANE, which is the one place + this departs from the mockup's numbers rather than porting them. + + The mockup draws into a fixed 1920x1080 box and scales the whole box to fit. + Its mask is `circle at 50% 45%` fading out by 70%, which in a box that shape + means the fade completes well inside the frame and the grid dissolves into + darkness around the lockup. Taking those same values into a box the size of + this pane keeps the RATIO but loses the effect: `circle` with no explicit + extent resolves to farthest-corner, so in a pane roughly 990x650 the fade + only completes past the corners and the grid reads as uniform to the edges, + which is what it looked like. The same applies to the 900px glow, which is + larger than a short pane is tall, so its falloff never appears. + + `closest-side` pins the fade to the nearer edge instead, so the vignette + completes inside the pane at any splitter position, and the glow is a + percentage of the pane rather than a pixel count. */ +.grid { position: absolute; inset: 0; + background-image: linear-gradient(@GRID@ 1px, transparent 1px), + linear-gradient(90deg, @GRID@ 1px, transparent 1px); + background-size: 64px 64px; opacity: @GRID_OPACITY@; + mask-image: radial-gradient(closest-side circle at 50% 45%, + rgba(0,0,0,0.9) 0%, transparent 75%); + -webkit-mask-image: radial-gradient(closest-side circle at 50% 45%, + rgba(0,0,0,0.9) 0%, transparent 75%); } +.glow { position: absolute; + width: min(95%, 900px); aspect-ratio: 1; + background: radial-gradient(circle, @GLOW@ 0%, transparent 70%); + top: 50%; left: 50%; transform: translate(-50%, -55%); } +/* Natural height, centred. The user rejected a fixed vertical split: this + pane's height varies enormously and a ratio breaks at one extreme. */ +.content { position: relative; z-index: 2; width: 100%; + padding: 0 clamp(12px, 4vw, 48px); + display: flex; flex-direction: column; align-items: center; + gap: clamp(10px, 2.2vw, 22px); } +/* Header ROW, per the decision of 2026-08-07: icon left, wordmark right, + with the glow and grid still centred behind. */ +.lockup { display: flex; align-items: center; + gap: clamp(10px, 2.4vw, 26px); } +.icon-tile { flex: none; + width: clamp(48px, 11vw, 104px); height: clamp(48px, 11vw, 104px); + border-radius: clamp(12px, 2.6vw, 26px); + background: @TILE@; border: 1px solid @TILE_BORDER@; + display: flex; align-items: center; justify-content: center; + box-shadow: 0 0 60px @TILE_SHADOW@; } +.icon-tile svg { width: 68%; height: 68%; } +.wordmark { display: flex; flex-direction: column; gap: 0.25em; } +.title { font-family: 'Oxanium qtmaildir', sans-serif; font-weight: 800; + font-size: clamp(26px, 6.4vw, 60px); letter-spacing: 0.01em; + color: @TITLE@; line-height: 1; white-space: nowrap; } +.title .accent { color: @ACCENT@; } +.subtitle { font-family: 'Plex qtmaildir', sans-serif; font-weight: 400; + font-size: clamp(9px, 1.7vw, 15px); letter-spacing: 0.04em; + color: @SUBTITLE@; text-transform: uppercase; } +.helpers { display: flex; flex-wrap: wrap; justify-content: center; + gap: clamp(8px, 1.8vw, 18px); + font-family: 'Plex qtmaildir', sans-serif; + font-size: clamp(10px, 1.8vw, 15px); } +.helpers a, .helpers span { color: @SUBTITLE@; text-decoration: none; + border-bottom: 1px solid transparent; + padding-bottom: 1px; } +.helpers a:hover { color: @ACCENT@; border-bottom-color: @ACCENT@; } +.footer { font-family: 'Plex qtmaildir', sans-serif; + font-size: clamp(8px, 1.4vw, 12px); color: @SUBTITLE@; opacity: 0.75; + text-align: center; line-height: 1.6; } +.footer a { color: @SUBTITLE@; text-decoration: none; + border-bottom: 1px solid @GRID@; } +.footer a:hover { color: @ACCENT@; } +)CSS"; + +} // namespace + +QString HtmlBuilder::buildPlaceholder(const QList<PlaceholderHelper> &helpers, + const QString &version, + const BrandPalette &brand) +{ + QString style = QString::fromUtf8(kPlaceholderStyleTemplate); + const QList<QPair<QString, QString>> tokens = { + { QStringLiteral("@BG@"), brand.background.name() }, + { QStringLiteral("@BG_IN@"), brand.backgroundIn.name() }, + { QStringLiteral("@GRID@"), brand.grid.name() }, + { QStringLiteral("@TILE_BORDER@"), brand.tileBorder.name() }, + { QStringLiteral("@GRID_OPACITY@"), + QString::number(brand.gridOpacity / 100.0) }, + { QStringLiteral("@TILE@"), brand.tile.name() }, + { QStringLiteral("@ACCENT@"), brand.accent.name() }, + { QStringLiteral("@TITLE@"), brand.title.name() }, + { QStringLiteral("@SUBTITLE@"), brand.subtitle.name() }, + { QStringLiteral("@GLOW@"), rgba(brand.accent, brand.glowAlpha) }, + { QStringLiteral("@TILE_SHADOW@"), rgba(brand.accent, brand.glowAlpha - 4) }, + { QStringLiteral("@FONT_OXANIUM@"), fontDataUri(":/fonts/Oxanium-ExtraBold.woff2") }, + { QStringLiteral("@FONT_PLEX@"), fontDataUri(":/fonts/IBMPlexSans-Regular.woff2") }, + }; + for (const auto &token : tokens) + style.replace(token.first, token.second); + + QString helperHtml; + for (const PlaceholderHelper &helper : helpers) { + const QString label = helper.label.toHtmlEscaped(); + if (helper.query.isEmpty()) { + // A state, not a search. Rendering it as a link would invite a + // click that runs an empty query and empties the thread list. + helperHtml += QStringLiteral("<span>%1</span>").arg(label); + continue; + } + + // Percent-encoded into the URL, then escaped into the attribute. A + // saved query is user-written and reaches here verbatim, so both + // layers are needed: one keeps it a single URL, the other keeps it + // inside the attribute. + const QString href = QString::fromLatin1( + QUrl::toPercentEncoding(helper.query)).toHtmlEscaped(); + helperHtml += QStringLiteral( + "<a href=\"qtmaildir-query:%1\">%2</a>").arg(href, label); + } + + return QStringLiteral( + "<!DOCTYPE html><html><head><meta charset=\"utf-8\">" + "<style>%1</style></head><body><div class=\"bg\">" + "<div class=\"grid\"></div><div class=\"glow\"></div>" + "<div class=\"content\">" + "<div class=\"lockup\">" + "<div class=\"icon-tile\">" + "<svg viewBox=\"0 0 256 256\" xmlns=\"http://www.w3.org/2000/svg\">" + "<path d=\"M100,70 L176,70 A14,14 0 0 1 190,84 L190,172 " + "A14,14 0 0 1 176,186 L100,186 L40,128 Z\" fill=\"%2\" stroke=\"%3\" " + "stroke-width=\"6\" stroke-linejoin=\"round\"/>" + "<circle cx=\"100\" cy=\"128\" r=\"15\" fill=\"%4\"/>" + "</svg></div>" + "<div class=\"wordmark\">" + "<div class=\"title\">qt<span class=\"accent\">Mail</span>Dir</div>" + "<div class=\"subtitle\">%5</div>" + "</div></div>" + "<div class=\"helpers\">%6</div>" + "<div class=\"footer\">%7<br>" + "<a href=\"https://danix.xyz/qtmaildir\">danix.xyz/qtmaildir</a>" + "</div></div></div></body></html>") + .arg(style, brand.accent.name(), brand.accentEdge.name(), + brand.tile.name(), + QCoreApplication::translate( + "HtmlBuilder", "local mail, tagged and searched"), + helperHtml, + QCoreApplication::translate( + "HtmlBuilder", "Copyright © 2026 Danilo M. · " + "version %1").arg(version.toHtmlEscaped())); +} + HtmlBuilder::Palette HtmlBuilder::defaultPalette() { if (const QGuiApplication *app = diff --git a/src/htmlbuilder.h b/src/htmlbuilder.h index 75fd1f8..3ade530 100644 --- a/src/htmlbuilder.h +++ b/src/htmlbuilder.h @@ -81,6 +81,58 @@ public: QColor quote; ///< Quoted lines in plain text. }; + /// The brand colours of the placeholder pane. + /// + /// **A deliberate exception to the Palette above**, which derives from the + /// desktop theme. A logo is brand rather than chrome, so these are the + /// values from the user's mockup and are not blended toward anything. The + /// desktop theme still decides WHICH set is used, so the pane never renders + /// a light lockup on a dark desktop. + struct BrandPalette { + QColor background; ///< The pane, behind the radial wash. + QColor backgroundIn; ///< The lighter centre of that wash. + QColor grid; ///< Grid rules, and the icon tile's border. + QColor tile; ///< The icon tile's fill. + QColor tileBorder; ///< The icon tile's edge. Separate from `grid`, + ///< which needs a different strength on light. + QColor accent; ///< "Mail" in the wordmark, and the envelope. + QColor accentEdge; ///< The envelope's stroke. + QColor title; ///< The wordmark, apart from the accent span. + QColor subtitle; ///< The tagline, the helpers and the footer. + + /// Percent alpha of the accent glow. Deliberately different between the + /// two sets: on dark the glow adds light and can be generous, on light + /// it subtracts and the same value washes the whole pane. + int glowAlpha = 0; + + /// Percent opacity of the grid, for the same reason. + int gridOpacity = 35; + }; + + /// Picks the dark or the light brand set from the desktop palette. + /// + /// Decided on the window's Base lightness, the same surface the document + /// Palette reads, so the two agree about which way round the theme is. + static BrandPalette brandPaletteFrom(const QPalette &palette); + + /// One helper line under the wordmark: a count, and the query it runs. + /// + /// An empty query renders as text rather than as a link, which is what the + /// sync line uses: it reports a state rather than naming a search. + struct PlaceholderHelper { + QString label; ///< Already-translated, e.g. "12 unread". + QString query; ///< notmuch query, or empty for a non-link line. + }; + + /// The pane shown when no thread is displayed. + /// + /// Rendered into the same web view as a message rather than into a second + /// widget stacked behind it, so there is one document path and one set of + /// security rules. + static QString buildPlaceholder(const QList<PlaceholderHelper> &helpers, + const QString &version, + const BrandPalette &brand); + /// Derives the document palette from a widget palette. /// /// The dim and border colours are blends rather than fixed greys, which is diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 77322bc..5af6624 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -577,6 +577,8 @@ void MainWindow::buildUi() m_messageView->setTagColors(&m_tagColors); connect(m_messageView, &MessageView::statusMessage, this, [this](const QString &text) { m_statusLabel->setText(text); }); + connect(m_messageView, &MessageView::queryRequested, + this, &MainWindow::onPlaceholderQueryRequested); m_splitter = new QSplitter(Qt::Horizontal, central); m_splitter->addWidget(m_threadView); @@ -781,6 +783,7 @@ void MainWindow::registerActions() // CLAUDE.md. m_currentThreadId.clear(); m_messageView->clear(); + showPlaceholderPane(); m_markReadTimer->stop(); m_markReadThreadId.clear(); }); @@ -1060,6 +1063,8 @@ void MainWindow::wireWorker() this, &MainWindow::onWorkerError); connect(m_worker, &NotmuchWorker::allTagsReady, this, &MainWindow::onAllTagsReady); + connect(m_worker, &NotmuchWorker::countsReady, + this, &MainWindow::onCountsReady); // A confirmed write clears the pending revert: without this, a later // unrelated error would roll back a change that actually succeeded. @@ -1092,6 +1097,92 @@ void MainWindow::onAllTagsReady(const QStringList &tags) m_queryCompleter->setTags(tags); } +namespace { + +/// The queries behind the placeholder's helper lines, in render order. +/// +/// Wire format, deliberately untranslated: `tag:` is notmuch syntax, not user +/// -facing prose. Only the labels beside them are translated. +const std::array<const char *, 3> kPlaceholderQueries = { + "tag:unread", + "tag:flagged", + "tag:inbox", +}; + +} // namespace + +QList<HtmlBuilder::PlaceholderHelper> MainWindow::placeholderHelpers() const +{ + QList<HtmlBuilder::PlaceholderHelper> helpers; + + // Empty until the first reply lands. Rendering three zeroes meanwhile + // would be worse than rendering nothing: a zero is a claim. + if (m_placeholderCounts.size() == int(kPlaceholderQueries.size())) { + const QStringList labels = { + tr("%n unread", "", m_placeholderCounts.at(0)), + tr("%n flagged", "", m_placeholderCounts.at(1)), + tr("%n in inbox", "", m_placeholderCounts.at(2)), + }; + + for (int i = 0; i < labels.size(); ++i) { + // A query notmuch could not count yields -1; skip that line rather + // than print a negative number at the user. + if (m_placeholderCounts.at(i) < 0) + continue; + helpers.append({ labels.at(i), + QString::fromLatin1(kPlaceholderQueries[i]) }); + } + } + + // The sync line, and only when something needs attention: a line that is + // always there becomes wallpaper and stops being read. + if (m_lastSyncFailed) { + helpers.append({ tr("last sync failed"), QString() }); + } else if (const int pending = pendingEditCount(); pending > 0) { + helpers.append({ tr("%n change(s) waiting to sync", "", pending), + QString() }); + } + + return helpers; +} + +void MainWindow::showPlaceholderPane() +{ + m_messageView->showPlaceholder(placeholderHelpers()); + + QStringList queries; + for (const char *query : kPlaceholderQueries) + queries.append(QString::fromLatin1(query)); + + QMetaObject::invokeMethod(m_worker, "requestCounts", Qt::QueuedConnection, + Q_ARG(QStringList, queries), + Q_ARG(quint64, ++m_countsGeneration)); +} + +void MainWindow::onCountsReady(const QVector<int> &counts, quint64 generation) +{ + // A reply for a superseded request carries counts taken before whatever + // prompted the newer one, so accepting it would repaint the pane with + // older numbers than it already has. + if (generation != m_countsGeneration) + return; + + m_placeholderCounts = counts; + + // Only repaint what is actually on screen. Without this, a reply arriving + // after the user opened a thread would replace the message with the logo. + if (m_messageView->showingPlaceholder()) + m_messageView->showPlaceholder(placeholderHelpers()); +} + +void MainWindow::onPlaceholderQueryRequested(const QString &query) +{ + // Through the query bar rather than straight to the worker, so the bar + // shows what is being displayed and the user can edit it from there. + m_queryEdit->setText(query); + runCurrentQuery(); +} + void MainWindow::showWarnings() { const QStringList warnings = m_config.warnings() + m_keyMap.warnings(); @@ -1132,6 +1223,7 @@ void MainWindow::runCurrentQuery() ++m_generation; m_model->clear(); m_messageView->clear(); + showPlaceholderPane(); // Undo entries refer to rows that are about to be discarded. The model // update they invert would be a no-op against the new result set, leaving @@ -1287,6 +1379,7 @@ void MainWindow::onSelectionChanged() m_markReadThreadId.clear(); m_currentThreadId.clear(); m_messageView->clear(); + showPlaceholderPane(); } void MainWindow::onThreadSelected(const QModelIndex ¤t, @@ -1317,6 +1410,7 @@ void MainWindow::onThreadSelected(const QModelIndex ¤t, m_markReadThreadId.clear(); m_currentThreadId.clear(); m_messageView->clear(); + showPlaceholderPane(); return; } @@ -1488,6 +1582,7 @@ void MainWindow::onSyncFinished(bool success, int exitCode) // what failed to put them there. m_pendingTagEdits.clear(); m_unnettablePendingEdits = 0; + m_lastSyncFailed = false; updatePendingIndicator(); showTransientStatus(tr("Sync complete")); @@ -1544,6 +1639,12 @@ void MainWindow::onSyncFinished(bool success, int exitCode) "has been left open.")); } } else { + // Latched until a sync succeeds, so the placeholder's sync line still + // says so on the next blank pane rather than only in a status message + // the user may not have been looking at. A skipped run does not set + // this: it is a branch of its own above, and a skip means another + // process is doing the work rather than that the work failed. + m_lastSyncFailed = true; m_statusLabel->setText(tr("Sync failed (exit %1)").arg(exitCode)); m_syncLogPane->show(); diff --git a/src/mainwindow.h b/src/mainwindow.h index adb973c..8043727 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -27,6 +27,7 @@ #include <functional> #include "config.h" +#include "htmlbuilder.h" #include "keymap.h" // Included rather than forward-declared: SyncPhaseTracker is held by value, so // its size must be known here. MailSync itself stays a forward declaration. @@ -184,6 +185,13 @@ private slots: void onTagsApplied(const TagChange &change); void onAllTagsReady(const QStringList &tags); + /// Thread counts for the placeholder's helper lines, in the order + /// requestPlaceholderCounts() asked for them. + void onCountsReady(const QVector<int> &counts, quint64 generation); + + /// Runs a query the user clicked on the placeholder pane. + void onPlaceholderQueryRequested(const QString &query); + private: void buildUi(); @@ -199,6 +207,20 @@ private: /// Asks the worker to re-enumerate the database tags for the completer. void requestAllTags(); + /// Shows the placeholder pane and asks the worker to refresh its counts. + /// + /// **The single route to a blank pane.** Every site that used to call + /// MessageView::clear() goes through here, so the pane is never left empty + /// by accident and the counts are refreshed exactly when they are about to + /// be looked at. A count goes stale the moment a tag is edited, and one + /// nobody is looking at is not worth keeping fresh. + void showPlaceholderPane(); + + /// The helper lines, built from the last counts received. Rendered with + /// whatever the previous answer was until the new one lands, so the pane + /// never flashes empty while the worker replies. + QList<HtmlBuilder::PlaceholderHelper> placeholderHelpers() const; + void showWarnings(); void showShortcutReference(); void showAbout(); @@ -385,6 +407,21 @@ private: /// Indeterminate, shown only while a sync runs. See setSyncBusy(). QProgressBar *m_syncProgress = nullptr; + /// The last counts the worker answered, one per kPlaceholderQueries entry. + /// Empty until the first reply, which renders the pane without its helper + /// lines rather than with three zeroes that would be a lie. + QVector<int> m_placeholderCounts; + + /// Discriminates a counts reply from a superseded request, the same way the + /// query generation does. A reply for an older request is dropped rather + /// than repainting the pane with counts taken before the last edit. + quint64 m_countsGeneration = 0; + + /// Set when a sync ends in failure, cleared when one succeeds. Drives the + /// placeholder's sync line, which appears only when something needs + /// attention, so it must survive until the next successful run. + bool m_lastSyncFailed = false; + /// Holds the sync log and its close button, so the pane can be dismissed. QWidget *m_syncLogPane = nullptr; QPlainTextEdit *m_syncLog = nullptr; diff --git a/src/messageview.cpp b/src/messageview.cpp index 10b21a9..b804d37 100644 --- a/src/messageview.cpp +++ b/src/messageview.cpp @@ -43,12 +43,15 @@ #include <QWebEngineView> #include <algorithm> +#include <functional> +#include <utility> #include "cidschemehandler.h" #include "htmlbuilder.h" #include "requestinterceptor.h" #include "tagstrip.h" #include "threadcidmap.h" +#include "version.h" namespace { @@ -56,8 +59,11 @@ namespace { class MessagePage : public QWebEnginePage { public: - MessagePage(QWebEngineProfile *profile, QObject *parent) - : QWebEnginePage(profile, parent) {} + using QueryHandler = std::function<bool(const QString &)>; + + MessagePage(QWebEngineProfile *profile, QObject *parent, + QueryHandler onQuery) + : QWebEnginePage(profile, parent), m_onQuery(std::move(onQuery)) {} protected: bool acceptNavigationRequest(const QUrl &url, NavigationType type, @@ -78,6 +84,24 @@ protected: return true; if (type == NavigationTypeLinkClicked) { + // The placeholder's helper lines. JavaScript is off in this + // profile, so a clickable count can only be a real link, and this + // is where it is turned back into an action. + // + // The handler decides whether to accept it, not this function: the + // view refuses unless the placeholder is what is actually + // displayed, so a qtmaildir-query: link inside a message body is + // dropped rather than handed a query to run. + if (url.scheme() == QLatin1String("qtmaildir-query")) { + // path() already percent-decodes; verified against Qt 6.11, + // which returns tag:unread for qtmaildir-query:tag%3Aunread. + // Decoding it a second time would corrupt a query carrying a + // literal '%', which notmuch accepts in a quoted term. + if (m_onQuery) + m_onQuery(url.path()); + return false; + } + QDesktopServices::openUrl(url); return false; } @@ -86,6 +110,9 @@ protected: // navigation would replace the pane, which no message may do. return !isMainFrame; } + +private: + QueryHandler m_onQuery; }; } // namespace @@ -105,7 +132,16 @@ MessageView::MessageView(QWidget *parent) m_profile->installUrlSchemeHandler(QByteArrayLiteral("cid"), m_cidHandler); m_view = new QWebEngineView(this); - m_view->setPage(new MessagePage(m_profile, m_view)); + // The gate the queryRequested() documentation describes: a helper link is + // only honoured while the placeholder is what is on screen, so the same + // URL inside a message body reaches here and is dropped. + m_view->setPage(new MessagePage(m_profile, m_view, + [this](const QString &query) { + if (!m_showingPlaceholder) + return false; + emit queryRequested(query); + return true; + })); QWebEngineSettings *settings = m_view->settings(); settings->setAttribute(QWebEngineSettings::JavascriptEnabled, false); @@ -196,9 +232,40 @@ void MessageView::setDocument(const QString &html) m_view->setHtml(html, documentUrl()); } +void MessageView::showPlaceholder( + const QList<HtmlBuilder::PlaceholderHelper> &helpers) +{ + // Everything clear() drops, dropped again: this is reachable directly and + // must not leave a previous thread's parts serveable behind the logo. + m_items.clear(); + m_tagStrip->setTags({}); + m_cidHandler->setParts({}); + m_interceptor->setAllowedCids({}); + m_interceptor->resetForNewMessage(); + + m_headerLabel->clear(); + m_detailsButton->hide(); + m_blockedLabel->hide(); + m_loadRemoteButton->hide(); + rebuildAttachmentBar(); + + // Set before the document loads, not after: acceptNavigationRequest reads + // it, and a click cannot arrive before setDocument() returns, but ordering + // it this way makes that independent of how the load is scheduled. + m_showingPlaceholder = true; + + // The widget's own palette, not qApp's, for the reason the render path + // uses it: a style sheet or a themed parent can give this pane different + // colours from the application. + setDocument(HtmlBuilder::buildPlaceholder( + helpers, QStringLiteral(QTMAILDIR_VERSION), + HtmlBuilder::brandPaletteFrom(palette()))); +} + void MessageView::clear() { m_items.clear(); + m_showingPlaceholder = false; m_tagStrip->setTags({}); // No thread is displayed, so nothing may be served or allowed. Without @@ -221,6 +288,7 @@ void MessageView::showThread(const QList<ThreadRenderItem> &items) { m_items = items; m_preferHtml = true; + m_showingPlaceholder = false; // Every thread starts from a clean policy: no remote grant carries over. m_interceptor->resetForNewMessage(); @@ -258,6 +326,7 @@ void MessageView::showThread(const QList<ThreadRenderItem> &items) void MessageView::showError(const QString &text, const QString &filePath) { m_items.clear(); + m_showingPlaceholder = false; // An error card references nothing, so the policy is emptied rather than // left holding the previous thread's parts. diff --git a/src/messageview.h b/src/messageview.h index c55f5c5..135c85d 100644 --- a/src/messageview.h +++ b/src/messageview.h @@ -59,6 +59,22 @@ public: void showError(const QString &text, const QString &filePath); void clear(); + /// Shows the branded pane used when no thread is displayed. + /// + /// Separate from clear(), which still exists and still blanks: clear() + /// drops the previous thread's state, and a caller that wants the + /// placeholder asks for it afterwards. Keeping them apart is what stops + /// the pane flashing a logo between selecting a thread and rendering it, + /// which the item lists as a constraint. + /// + /// helpers are already-translated lines; an empty query makes one plain + /// text rather than a link. + void showPlaceholder(const QList<HtmlBuilder::PlaceholderHelper> &helpers); + + /// True while the placeholder is what the view is showing. Lets the window + /// re-render it with fresh counts without guessing what is on screen. + bool showingPlaceholder() const { return m_showingPlaceholder; } + /// Supplies the tag strip's colours. Not owned; must outlive the view. void setTagColors(const TagColors *colours); @@ -98,6 +114,17 @@ public slots: signals: void statusMessage(const QString &text); + /// A helper line on the placeholder was clicked. The window runs the query; + /// the view has no business driving the query bar itself. + /// + /// **Gated on the placeholder being what is displayed.** A message body is + /// attacker-controlled HTML and can carry a qtmaildir-query: link as easily + /// as any other; without the gate, clicking one would let a stranger's mail + /// drive the thread list. The consequence is mild (a query runs, nothing is + /// mutated or sent), but "a link in a message does something inside the + /// app" is a boundary worth keeping shut rather than arguing about. + void queryRequested(const QString &query); + protected: /// Turns Ctrl+wheel over the body into zoom, and Ctrl+middle-click into a /// reset. Both events are delivered to the web view's internal QQuickWidget @@ -147,6 +174,9 @@ private: QList<ThreadRenderItem> m_items; bool m_preferHtml = true; + /// Gates queryRequested(), so a link in a message body cannot run a query. + bool m_showingPlaceholder = false; + QWebEngineProfile *m_profile = nullptr; QWebEngineView *m_view = nullptr; RequestInterceptor *m_interceptor = nullptr; diff --git a/src/notmuchworker.cpp b/src/notmuchworker.cpp index 3525a2b..973d110 100644 --- a/src/notmuchworker.cpp +++ b/src/notmuchworker.cpp @@ -362,3 +362,33 @@ void NotmuchWorker::requestAllTags(quint64 generation) result.sort(); emit allTagsReady(result, generation); } + +void NotmuchWorker::requestCounts(const QStringList &queries, quint64 generation) +{ + if (!openReadOnly()) + return; + + QVector<int> counts; + counts.reserve(queries.size()); + + for (const QString &query : queries) { + NmQuery nmQuery(notmuch_query_create(m_db, query.toUtf8().constData())); + + unsigned int count = 0; + // -1 rather than a skipped entry: the caller pairs these with its own + // labels positionally, so a dropped answer would put a real number + // against the wrong name, which is worse than showing none. + if (!nmQuery || + notmuch_query_count_threads(nmQuery.get(), &count) + != NOTMUCH_STATUS_SUCCESS) { + counts.append(-1); + continue; + } + + // Threads, matching what the thread list shows. A message count would + // disagree with the number of rows a click on this line produces. + counts.append(static_cast<int>(count)); + } + + emit countsReady(counts, generation); +} diff --git a/src/notmuchworker.h b/src/notmuchworker.h index 96e75f8..05c1d3a 100644 --- a/src/notmuchworker.h +++ b/src/notmuchworker.h @@ -74,6 +74,15 @@ public slots: /// after a sync, and after a tag mutation introduces an unknown tag. void requestAllTags(quint64 generation); + /// Thread counts for the placeholder pane's helper lines, one per query, + /// answered in the order asked. Counts rather than results: the pane says + /// how much there is, and clicking a line runs the query properly. + /// + /// Requested when the pane is about to go blank rather than kept fresh in + /// the background. A count goes stale the moment a tag is edited, and + /// refreshing one nobody is looking at is work for nothing. + void requestCounts(const QStringList &queries, quint64 generation); + signals: void threadsReady(const QVector<ThreadSummary> &threads, quint64 generation); void queryFinished(int totalThreads, quint64 generation); @@ -81,6 +90,11 @@ signals: void tagsApplied(const TagChange &change); void allTagsReady(const QStringList &tags, quint64 generation); + /// One entry per requested query, in the order they were asked for. A query + /// notmuch rejects yields -1 rather than dropping the entry, so the + /// positional correspondence the caller relies on always holds. + void countsReady(const QVector<int> &counts, quint64 generation); + void errorOccurred(const QString &message); private: diff --git a/src/resources.qrc b/src/resources.qrc index 7bdb592..f6bb6a4 100644 --- a/src/resources.qrc +++ b/src/resources.qrc @@ -2,5 +2,10 @@ <RCC version="1.0"> <qresource prefix="/"> <file alias="icons/qtmaildir.svg">../assets/icons/qtmaildir.svg</file> + <!-- Subset, and embedded as data: URIs by HtmlBuilder::buildPlaceholder. + The web view's interceptor blocks every remote request, so the + mockup's Google Fonts @import cannot be ported. --> + <file alias="fonts/Oxanium-ExtraBold.woff2">../assets/fonts/Oxanium-ExtraBold-subset.woff2</file> + <file alias="fonts/IBMPlexSans-Regular.woff2">../assets/fonts/IBMPlexSans-Regular-subset.woff2</file> </qresource> </RCC> diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 0f6ec5a..c09ef78 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1,6 +1,12 @@ # add_qtmaildir_test(<name>) builds tests/test_<name>.cpp and registers it. +# +# resources.qrc is compiled into each test binary for the same reason the app +# compiles it rather than the static library: a qrc inside a .a registers from +# a global initialiser the linker then drops. Library code reads :/fonts/ when +# it builds the placeholder pane, so a test that never links the qrc would +# exercise only the missing-resource fallback and pass against a broken build. function(add_qtmaildir_test name) - add_executable(test_${name} test_${name}.cpp) + add_executable(test_${name} test_${name}.cpp ${CMAKE_SOURCE_DIR}/src/resources.qrc) target_link_libraries(test_${name} PRIVATE qtmaildir_lib Qt6::Test) add_test(NAME ${name} COMMAND test_${name}) endfunction() diff --git a/tests/test_htmlbuilder.cpp b/tests/test_htmlbuilder.cpp index 7fa45a2..fdb57c5 100644 --- a/tests/test_htmlbuilder.cpp +++ b/tests/test_htmlbuilder.cpp @@ -48,6 +48,15 @@ private slots: // Theming. void aDarkPaletteProducesADarkDocument(); void everyColourComesFromThePalette(); + + // The placeholder pane (item 30). + void placeholderPicksTheBrandSetFromTheDesktopTheme(); + void placeholderHelperBecomesALinkOnItsQuery(); + void placeholderHelperWithoutAQueryIsNotALink(); + void placeholderEscapesHelperText(); + void placeholderEmbedsItsFontsRatherThanFetchingThem(); + void placeholderReferencesNoRemoteResource(); + void placeholderStyleHasNoUnsubstitutedTokens(); void theBodyAlwaysGetsABackground(); void aSendersOwnHtmlIsNotRecoloured(); }; @@ -357,6 +366,168 @@ void TestHtmlBuilder::everyColourComesFromThePalette() } } +void TestHtmlBuilder::placeholderPicksTheBrandSetFromTheDesktopTheme() +{ + // The brand colours are fixed, deliberately: this is the one place where + // the desktop palette does NOT supply the values. What the desktop decides + // is which of the two sets is used, and getting that backwards is the + // failure the item warns about, a light lockup on a dark desktop. + const HtmlBuilder::BrandPalette dark = HtmlBuilder::brandPaletteFrom( + makeTestPalette(QColor(0x1a, 0x1a, 0x1a), QColor(0xee, 0xee, 0xee))); + const HtmlBuilder::BrandPalette light = HtmlBuilder::brandPaletteFrom( + makeTestPalette(QColor(0xff, 0xff, 0xff), QColor(0x11, 0x11, 0x11))); + + QCOMPARE(dark.background.name(), QStringLiteral("#060b10")); + QCOMPARE(light.background.name(), QStringLiteral("#ffffff")); + + // Not merely different: the right way round. A set whose background is + // darker than its text is the dark set, whichever values it holds. + QVERIFY(dark.background.lightnessF() < dark.title.lightnessF()); + QVERIFY(light.background.lightnessF() > light.title.lightnessF()); +} + +void TestHtmlBuilder::placeholderHelperBecomesALinkOnItsQuery() +{ + // JavaScript is off in this profile, so a helper can only be actionable by + // being a real link that the page's navigation handler intercepts. + const QString html = HtmlBuilder::buildPlaceholder( + { { QStringLiteral("12 unread"), QStringLiteral("tag:unread") } }, + QStringLiteral("0.10.0"), + HtmlBuilder::brandPaletteFrom(QPalette())); + + QVERIFY(html.contains(QStringLiteral("href=\"qtmaildir-query:tag%3Aunread\""))); + QVERIFY(html.contains(QStringLiteral("12 unread"))); +} + +void TestHtmlBuilder::placeholderHelperWithoutAQueryIsNotALink() +{ + // The sync line reports a state rather than naming a search, so clicking it + // must not run an empty query and wipe the thread list. + const QString html = HtmlBuilder::buildPlaceholder( + { { QStringLiteral("3 edits waiting to sync"), QString() } }, + QStringLiteral("0.10.0"), + HtmlBuilder::brandPaletteFrom(QPalette())); + + QVERIFY(html.contains(QStringLiteral("3 edits waiting to sync"))); + QVERIFY(!html.contains(QStringLiteral("qtmaildir-query:"))); +} + +void TestHtmlBuilder::placeholderEscapesHelperText() +{ + // A helper label carries a count this code produced, but the query half is + // built from configuration and a saved query is user-written. Neither may + // reach the document unescaped, and the query is doubly encoded: percent + // for the URL, then HTML for the attribute. + const QString html = HtmlBuilder::buildPlaceholder( + { { QStringLiteral("<script>alert(1)</script>"), + QStringLiteral("tag:\"a\"><script>") } }, + QStringLiteral("0.10.0"), + HtmlBuilder::brandPaletteFrom(QPalette())); + + QVERIFY(!html.contains(QStringLiteral("<script>"))); + QVERIFY(html.contains(QStringLiteral("<script>"))); +} + +void TestHtmlBuilder::placeholderEmbedsItsFontsRatherThanFetchingThem() +{ + // The mockup @imports Google Fonts, which the interceptor blocks by design. + // The fonts ship in the binary and must arrive as data: URIs, or the pane + // silently falls back to a system font and stops looking like the brand. + const QString html = HtmlBuilder::buildPlaceholder( + {}, QStringLiteral("0.10.0"), + HtmlBuilder::brandPaletteFrom(QPalette())); + + QVERIFY(!html.contains(QStringLiteral("fonts.googleapis.com"))); + + // BOTH faces, each with a real payload. Counting @font-face rules or + // checking the document's total size passes with one face missing, since + // the other is large enough on its own to carry either check: a mutation + // pointing one src at a nonexistent resource survived exactly that test. + // A missing resource yields an empty src, so the length is what catches it. + static const QRegularExpression src( + QStringLiteral("src: url\\('data:font/woff2;base64,([^']*)'\\)")); + auto it = src.globalMatch(html); + int faces = 0; + while (it.hasNext()) { + ++faces; + QVERIFY(it.next().captured(1).size() > 1000); + } + QCOMPARE(faces, 2); +} + +void TestHtmlBuilder::placeholderReferencesNoRemoteResource() +{ + // The load-bearing security check, asserted as a negative for the same + // reason everyColourComesFromThePalette is: one leftover reference is the + // entire defect, and it would be invisible because the interceptor blocks + // it and the pane just renders slightly wrong. + const QString html = HtmlBuilder::buildPlaceholder( + { { QStringLiteral("12 unread"), QStringLiteral("tag:unread") } }, + QStringLiteral("0.10.0"), + HtmlBuilder::brandPaletteFrom(QPalette())); + + QVERIFY(!html.contains(QStringLiteral("//fonts"))); + QVERIFY(!html.contains(QStringLiteral("@import"))); + + // The one http: URL is the SVG namespace, which is an identifier and never + // fetched. Asserting its exact value rather than excluding the scheme + // wholesale: a second http: URL appearing later would be a real resource. + static const QRegularExpression http(QStringLiteral("http://[^\"' ]*")); + auto plain = http.globalMatch(html); + while (plain.hasNext()) { + QCOMPARE(plain.next().captured(0), + QStringLiteral("http://www.w3.org/2000/svg")); + } + + // Every https: URL must be the footer's website link, which is a link the + // user clicks and not a resource the document fetches. + static const QRegularExpression https(QStringLiteral("https://[^\"' ]*")); + auto it = https.globalMatch(html); + while (it.hasNext()) { + QCOMPARE(it.next().captured(0), + QStringLiteral("https://danix.xyz/qtmaildir")); + } +} + +void TestHtmlBuilder::placeholderStyleHasNoUnsubstitutedTokens() +{ + // The defect this exists for shipped once and was invisible. The template + // used QString::arg with "%%" for every CSS percentage, and arg() does NOT + // collapse "%%" into "%", so the stylesheet reached the browser carrying + // "50%%". Each declaration holding one was dropped as invalid, which + // silently disabled the grid mask, the glow and both radial gradients. The + // pane still rendered, still looked plausible, and a geometry probe that + // happened to measure only percentage-free properties reported it correct. + const QString html = HtmlBuilder::buildPlaceholder( + { { QStringLiteral("12 unread"), QStringLiteral("tag:unread") } }, + QStringLiteral("0.10.0"), + HtmlBuilder::brandPaletteFrom(QPalette())); + + const qsizetype start = html.indexOf(QStringLiteral("<style>")); + const qsizetype end = html.indexOf(QStringLiteral("</style>")); + QVERIFY(start >= 0 && end > start); + const QString style = html.mid(start, end - start); + + // No doubled percent survives into the document. + QVERIFY2(!style.contains(QStringLiteral("%%")), + "the stylesheet carries '%%', which CSS rejects: every rule " + "containing one is silently dropped"); + + // No token went unreplaced. A renamed colour would otherwise leave + // '@ACCENT@' sitting in the CSS as a dropped declaration. + static const QRegularExpression token(QStringLiteral("@[A-Z_]+@")); + const QRegularExpressionMatch leftover = token.match(style); + QVERIFY2(!leftover.hasMatch(), + qPrintable(QStringLiteral("unsubstituted token '%1' in the " + "stylesheet").arg(leftover.captured(0)))); + + // The three effects the bug disabled, each asserted by name so that + // deleting one is a test failure rather than a silent visual regression. + QVERIFY(style.contains(QStringLiteral("mask-image"))); + QVERIFY(style.contains(QStringLiteral("radial-gradient"))); + QVERIFY(style.contains(QStringLiteral("aspect-ratio"))); +} + void TestHtmlBuilder::theBodyAlwaysGetsABackground() { // The original CSS set no background at all, which is why the pane was diff --git a/tests/test_messageview.cpp b/tests/test_messageview.cpp index 57d7b42..e94fb86 100644 --- a/tests/test_messageview.cpp +++ b/tests/test_messageview.cpp @@ -47,6 +47,8 @@ private slots: void headerEscapesUntrustedValues(); void headerOmitsAnAbsentCc(); void detailsDialogIsOfferedForEveryThread(); + void placeholderRendersAndReportsItself(); + void aMessageBodyCannotRunAQuery(); private: QWebEngineView *webViewOf(MessageView *view) const @@ -492,5 +494,77 @@ void TestMessageView::detailsDialogIsOfferedForEveryThread() QVERIFY(!button || !button->isVisible()); } +void TestMessageView::placeholderRendersAndReportsItself() +{ + MessageView view; + QWebEngineView *web = webViewOf(&view); + QVERIFY(web); + + QSignalSpy loaded(web, &QWebEngineView::loadFinished); + view.showPlaceholder({ { QStringLiteral("7 unread"), + QStringLiteral("tag:unread") } }); + + QVERIFY2(loaded.wait(15000), "the placeholder document never loaded"); + QVERIFY2(loaded.last().at(0).toBool(), + "loadFinished reported failure: the navigation was rejected, " + "which is what a base-URL mismatch looks like"); + + // Rendered, not merely loaded. The wordmark is split across elements by + // the accent span, so the helper line is what proves the content arrived. + QString text; + bool done = false; + web->page()->toPlainText([&](const QString &result) { + text = result; + done = true; + }); + QTRY_VERIFY_WITH_TIMEOUT(done, 15000); + QVERIFY2(text.contains(QStringLiteral("7 unread")), qPrintable(text)); + + QVERIFY(view.showingPlaceholder()); +} + +void TestMessageView::aMessageBodyCannotRunAQuery() +{ + // The gate behind queryRequested(). A message body is attacker-controlled + // HTML and can carry a qtmaildir-query: link; the view only honours one + // while the placeholder is what is displayed, so this asserts the state + // that decides it rather than synthesising a click, which would need the + // page's protected navigation handler. + MessageView view; + QSignalSpy queries(&view, &MessageView::queryRequested); + + view.showPlaceholder({ { QStringLiteral("7 unread"), + QStringLiteral("tag:unread") } }); + QVERIFY(view.showingPlaceholder()); + + ParsedMessage message; + message.ok = true; + message.from = QStringLiteral("Mallory <mallory@example.org>"); + message.subject = QStringLiteral("Click me"); + message.htmlBody = QStringLiteral( + "<a href=\"qtmaildir-query:tag%3Adeleted\">a link</a>"); + + ThreadRenderItem item; + item.message = message; + item.cidPrefix = QStringLiteral("m0"); + item.expanded = true; + + view.showThread({ item }); + + // Showing any message closes the gate, so a link in that message's own + // body has nothing to reach. + QVERIFY2(!view.showingPlaceholder(), + "the gate stayed open while a message was displayed: a link in a " + "message body could run a query"); + + view.clear(); + QVERIFY(!view.showingPlaceholder()); + + view.showError(QStringLiteral("broken"), QStringLiteral("/tmp/x")); + QVERIFY(!view.showingPlaceholder()); + + QVERIFY(queries.isEmpty()); +} + QTEST_MAIN(TestMessageView) #include "test_messageview.moc" diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp index 488b630..be62ad3 100644 --- a/tests/test_notmuchworker.cpp +++ b/tests/test_notmuchworker.cpp @@ -58,6 +58,9 @@ private slots: void requestAllTagsReturnsSortedTags(); void requestAllTagsOnUnreadableConfigEmitsError(); + void requestCountsAnswersOneCountPerQuery(); + void requestCountsKeepsPositionOnAnInvalidQuery(); + private: /// Tags of one message, read back through a fresh worker query. QStringList tagsOf(const QString &messageId); @@ -472,5 +475,53 @@ void TestNotmuchWorker::requestAllTagsOnUnreadableConfigEmitsError() QVERIFY(ready.isEmpty()); } +void TestNotmuchWorker::requestCountsAnswersOneCountPerQuery() +{ + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy spy(&worker, &NotmuchWorker::countsReady); + + worker.requestCounts({ QStringLiteral("tag:unread"), + QStringLiteral("tag:inbox"), + QStringLiteral("tag:flagged") }, 9); + + QCOMPARE(spy.count(), 1); + QCOMPARE(spy.at(0).at(1).value<quint64>(), quint64(9)); + + // Threads, not messages: thread A holds two messages and must count once, + // which is the number the pane's "N in inbox" line claims to be showing. + const QVector<int> counts = spy.at(0).at(0).value<QVector<int>>(); + QCOMPARE(counts, QVector<int>({ 1, 3, 0 })); +} + +void TestNotmuchWorker::requestCountsKeepsPositionOnAnInvalidQuery() +{ + // The caller pairs answers with its own labels by index, so every query + // must produce exactly one entry at its own position. Dropping one would + // shift every later count onto the wrong label, and the pane would show a + // real number against the wrong name rather than visibly breaking. + // + // **notmuch's query parser rejects almost nothing.** malformedQuery... + // above records the same finding: an unbalanced quote parses and matches + // nothing. `((((` behaves the same way and counts 0 rather than failing, + // which is why this asserts the positional contract rather than a -1 that + // no query string can actually provoke. The -1 branch remains for a + // notmuch_query_create allocation failure, which a test cannot reach. + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy spy(&worker, &NotmuchWorker::countsReady); + + worker.requestCounts({ QStringLiteral("tag:unread"), + QStringLiteral("(((("), + QStringLiteral("tag:inbox") }, 1); + + QCOMPARE(spy.count(), 1); + const QVector<int> counts = spy.at(0).at(0).value<QVector<int>>(); + QCOMPARE(counts.size(), 3); + + // The queries either side keep their own answers, which is the property + // the pane depends on. + QCOMPARE(counts.at(0), 1); + QCOMPARE(counts.at(2), 3); +} + QTEST_MAIN(TestNotmuchWorker) #include "test_notmuchworker.moc" |
