From 441b93a75b9941d395edfe4b65ed0af2da9e0021 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 14 Aug 2026 12:42:38 +0200 Subject: feat(search): build notmuch terms for the right-click actions One place for the query grammar behind every search surface, with no widget involved so it is tested without a painter or a web engine. extend() parenthesises both sides. The query bar may hold a hand-written disjunction, and 'a or b AND c' binds as 'a or (b AND c)', which widens a search meant to narrow it and reports nothing. --- tests/CMakeLists.txt | 1 + tests/test_searchterm.cpp | 146 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+) create mode 100644 tests/test_searchterm.cpp (limited to 'tests') diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e5142b6..5b0b85b 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -54,3 +54,4 @@ add_qtmaildir_test(querycompleter) add_qtmaildir_test(tagdialog) add_qtmaildir_test(tagrules) add_qtmaildir_test(rulequery) +add_qtmaildir_test(searchterm) diff --git a/tests/test_searchterm.cpp b/tests/test_searchterm.cpp new file mode 100644 index 0000000..e0bdb60 --- /dev/null +++ b/tests/test_searchterm.cpp @@ -0,0 +1,146 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include + +#include "searchterm.h" + +/// The query grammar of the right-click search actions. +/// +/// Asserted on the CONSTRUCTED STRING throughout, never on a query notmuch +/// refuses: notmuch's parser rejects almost nothing, so `from:((((` parses +/// cleanly and matches zero. A test expecting a failure would pass against +/// correct code and against broken code alike. +class TestSearchTerm : public QObject +{ + Q_OBJECT +private slots: + void quotesAPlainValue(); + void escapesEmbeddedQuotes(); + void collapsesWhitespaceAndNewlines(); + void rejectsEmptyAndWhitespaceOnly(); + void capsAnOverlongSelection(); + void buildsAFieldTerm(); + void omitsAFieldWithNoValue(); + void buildsADateRangeForOneDay(); + void tagIsNotQuoted(); + void extendParenthesisesBothSides(); + void extendOntoAnEmptyQueryIsAReplace(); +}; + +void TestSearchTerm::quotesAPlainValue() +{ + QCOMPARE(SearchTerm::quote(QStringLiteral("Quarterly report")), + QStringLiteral("\"Quarterly report\"")); +} + +void TestSearchTerm::escapesEmbeddedQuotes() +{ + // A selection is arbitrary prose and can carry a quote. Unescaped, it ends + // the quoted string early and the rest becomes stray query syntax, which + // notmuch accepts and matches nothing on. + QCOMPARE(SearchTerm::quote(QStringLiteral("say \"hello\" now")), + QStringLiteral("\"say \\\"hello\\\" now\"")); +} + +void TestSearchTerm::collapsesWhitespaceAndNewlines() +{ + QCOMPARE(SearchTerm::quote(QStringLiteral(" two\n\nlines\there ")), + QStringLiteral("\"two lines here\"")); +} + +void TestSearchTerm::rejectsEmptyAndWhitespaceOnly() +{ + // An empty term must yield an empty string, which is what every caller + // tests to decide whether to offer a menu entry at all. + QVERIFY(SearchTerm::quote(QString()).isEmpty()); + QVERIFY(SearchTerm::quote(QStringLiteral(" \n\t ")).isEmpty()); +} + +void TestSearchTerm::capsAnOverlongSelection() +{ + // A multi-kilobyte selection is a mis-drag, not a query. + const QString huge(5000, QLatin1Char('x')); + const QString term = SearchTerm::quote(huge); + QVERIFY2(term.size() < 300, + qPrintable(QStringLiteral("term was %1 chars").arg(term.size()))); + QVERIFY(term.startsWith(QStringLiteral("\"xxx"))); + QVERIFY(term.endsWith(QLatin1Char('"'))); +} + +void TestSearchTerm::buildsAFieldTerm() +{ + QCOMPARE(SearchTerm::field(QStringLiteral("from"), + QStringLiteral("Foo ")), + QStringLiteral("from:\"Foo \"")); +} + +void TestSearchTerm::omitsAFieldWithNoValue() +{ + // A message with no Cc must not offer cc:"" , which parses cleanly and + // matches nothing, so the entry would look enabled and do nothing. + QVERIFY(SearchTerm::field(QStringLiteral("cc"), QString()).isEmpty()); +} + +void TestSearchTerm::buildsADateRangeForOneDay() +{ + // notmuch's date: range is inclusive at both ends, so one day is the day + // named twice rather than the day and its successor. + const QDate day(2026, 8, 14); + QCOMPARE(SearchTerm::onDate(day), + QStringLiteral("date:2026-08-14..2026-08-14")); + QVERIFY(SearchTerm::onDate(QDate()).isEmpty()); +} + +void TestSearchTerm::tagIsNotQuoted() +{ + // A tag name is a token from a known vocabulary, not prose. Quoting one + // is not wrong but reads badly in the bar, and the user edits that text. + QCOMPARE(SearchTerm::tag(QStringLiteral("inbox")), + QStringLiteral("tag:inbox")); + // A tag containing a space is the exception and does need quoting. + QCOMPARE(SearchTerm::tag(QStringLiteral("to do")), + QStringLiteral("tag:\"to do\"")); + QVERIFY(SearchTerm::tag(QString()).isEmpty()); +} + +void TestSearchTerm::extendParenthesisesBothSides() +{ + // THE case this exists for. The bar may hold a hand-written disjunction, + // and `a or b AND c` binds as `a or (b AND c)`: the result WIDENS a search + // the user asked to narrow. Both sides are wrapped so neither can rebind. + QCOMPARE(SearchTerm::extend(QStringLiteral("tag:inbox or tag:flagged"), + QStringLiteral("from:foo@example.org")), + QStringLiteral("(tag:inbox or tag:flagged) AND (from:foo@example.org)")); +} + +void TestSearchTerm::extendOntoAnEmptyQueryIsAReplace() +{ + // Rather than "() AND (x)", which matches nothing. + QCOMPARE(SearchTerm::extend(QString(), QStringLiteral("tag:inbox")), + QStringLiteral("tag:inbox")); + QCOMPARE(SearchTerm::extend(QStringLiteral(" "), + QStringLiteral("tag:inbox")), + QStringLiteral("tag:inbox")); + // And an empty new term leaves the existing query alone. + QCOMPARE(SearchTerm::extend(QStringLiteral("tag:inbox"), QString()), + QStringLiteral("tag:inbox")); +} + +QTEST_MAIN(TestSearchTerm) +#include "test_searchterm.moc" -- cgit v1.2.3 From 8cf71bb617c34f52357ce0597c7d07601337828b Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 14 Aug 2026 12:45:58 +0200 Subject: refactor(mime): expose the Date: header parse as MimeParser::parseDate The date search needs it and the logic already existed inside a file-local function, including the fix for Qt::RFC2822Date rejecting a string that carries a trailing timezone comment. Extracted rather than rewritten, so the second caller cannot end up without that fix. --- src/mimeparser.cpp | 15 ++++++++++----- src/mimeparser.h | 11 +++++++++++ tests/test_mimeparser.cpp | 23 +++++++++++++++++++++++ 3 files changed, 44 insertions(+), 5 deletions(-) (limited to 'tests') diff --git a/src/mimeparser.cpp b/src/mimeparser.cpp index ecf56f1..2782a4a 100644 --- a/src/mimeparser.cpp +++ b/src/mimeparser.cpp @@ -272,20 +272,25 @@ QString recipientSummary(const QString &rawTo, int maxNames) return summary; } -QString attachmentFolderName(const QString &rfc822Date, const QString &subject) +QDateTime MimeParser::parseDate(const QString &rfc822Date) { - // The date prefix sorts chronologically in a file manager. A Date: header - // that does not parse is simply dropped rather than guessed at. // A trailing timezone comment, "... +0200 (CEST)", is legal per RFC 5322 // and common in the wild, but Qt::RFC2822Date rejects the whole string // when one is present (verified on Qt 6.11). Strip comments before - // parsing, or every such message silently loses its date prefix. + // parsing, or every such message silently loses its date. QString cleaned = rfc822Date; cleaned.remove(QRegularExpression(QStringLiteral("\\s*\\([^)]*\\)"))); cleaned = cleaned.trimmed(); + return QDateTime::fromString(cleaned, Qt::RFC2822Date); +} + +QString attachmentFolderName(const QString &rfc822Date, const QString &subject) +{ + // The date prefix sorts chronologically in a file manager. A Date: header + // that does not parse is simply dropped rather than guessed at. QString prefix; - const QDateTime parsed = QDateTime::fromString(cleaned, Qt::RFC2822Date); + const QDateTime parsed = MimeParser::parseDate(rfc822Date); if (parsed.isValid()) prefix = parsed.toString(QStringLiteral("yyyy-MM-dd")); diff --git a/src/mimeparser.h b/src/mimeparser.h index 9fceb2f..da54434 100644 --- a/src/mimeparser.h +++ b/src/mimeparser.h @@ -19,6 +19,7 @@ #pragma once #include +#include #include #include #include @@ -143,4 +144,14 @@ public: MimeParser(); ParsedMessage parse(const QString &filePath) const; + + /// Parses an RFC 2822 `Date:` header, returning an invalid QDateTime when + /// nothing usable is there. + /// + /// **Strips comments before parsing**, because `Qt::RFC2822Date` rejects + /// the entire string when a trailing timezone comment is present, and + /// `... +0200 (CEST)` is legal per RFC 5322 and common in the wild + /// (verified on Qt 6.11). A parser without this silently loses the date on + /// a large share of real mail. + static QDateTime parseDate(const QString &rfc822Date); }; diff --git a/tests/test_mimeparser.cpp b/tests/test_mimeparser.cpp index bac71f6..af6307e 100644 --- a/tests/test_mimeparser.cpp +++ b/tests/test_mimeparser.cpp @@ -47,6 +47,7 @@ private slots: void recipientSummarySurvivesUnusableInput(); void folderNameSurvivesATimezoneComment(); void savingABatchNeverOverwrites(); + void parsesADateWithATimezoneComment(); private: QString fixture(const QString &name) const @@ -485,5 +486,27 @@ void TestMimeParser::recipientSummarySurvivesUnusableInput() recipientSummary(QStringLiteral("\"unterminated ")); } +void TestMimeParser::parsesADateWithATimezoneComment() +{ + // Qt::RFC2822Date rejects the WHOLE string when a trailing comment is + // present (verified on Qt 6.11), and "+0200 (CEST)" is both legal and + // common. Without the comment stripped, every such message loses its date + // silently: the attachment folder loses its prefix, and a date search + // offers nothing with no indication why. + const QDateTime withComment = MimeParser::parseDate( + QStringLiteral("Fri, 14 Aug 2026 09:30:00 +0200 (CEST)")); + QVERIFY(withComment.isValid()); + QCOMPARE(withComment.date(), QDate(2026, 8, 14)); + + const QDateTime plain = MimeParser::parseDate( + QStringLiteral("Fri, 14 Aug 2026 09:30:00 +0200")); + QVERIFY(plain.isValid()); + QCOMPARE(plain.date(), QDate(2026, 8, 14)); + + // Nothing usable is an invalid QDateTime, never a guess. + QVERIFY(!MimeParser::parseDate(QStringLiteral("last Tuesday")).isValid()); + QVERIFY(!MimeParser::parseDate(QString()).isValid()); +} + QTEST_MAIN(TestMimeParser) #include "test_mimeparser.moc" -- cgit v1.2.3 From 7e1cabf49c03c1e0c2926dcfcf82adedbf98940e Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 14 Aug 2026 12:49:37 +0200 Subject: feat(tags): hit-test a chip in the tag strip chipRectAt() is the single source of a chip's geometry, used by paintEvent and by the hit test, so the drawn chip and the clickable chip cannot drift. The +N chip yields nothing: it stands for a list of tags rather than one, so there is no single value a search could be built from. --- src/tagstrip.cpp | 54 ++++++++++++++++++++++-- src/tagstrip.h | 23 +++++++++++ tests/CMakeLists.txt | 1 + tests/test_tagstrip.cpp | 108 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 182 insertions(+), 4 deletions(-) create mode 100644 tests/test_tagstrip.cpp (limited to 'tests') diff --git a/src/tagstrip.cpp b/src/tagstrip.cpp index bad116a..7e671ce 100644 --- a/src/tagstrip.cpp +++ b/src/tagstrip.cpp @@ -18,6 +18,7 @@ #include "tagstrip.h" +#include #include #include @@ -109,6 +110,50 @@ void TagStrip::resizeEvent(QResizeEvent *event) relayout(); } +QRect TagStrip::chipRectAt(int index) const +{ + if (index < 0 || index >= m_visible.size()) + return {}; + + const QFontMetrics metrics(font()); + + // Reproduces paintEvent's own vertical placement exactly, which is derived + // from the font's height rather than from the chip's, so a chip whose text + // is shorter than the line still lands on the same baseline. + const int top = (height() - (metrics.height() + TagChip::kPaddingY * 2)) / 2; + + int x = 0; + for (int i = 0; i < index; ++i) + x += TagChip::sizeFor(metrics, m_visible.at(i)).width() + TagChip::kSpacing; + + return QRect(QPoint(x, top), TagChip::sizeFor(metrics, m_visible.at(index))); +} + +QString TagStrip::chipAt(const QPoint &point) const +{ + for (int i = 0; i < m_visible.size(); ++i) { + if (chipRectAt(i).contains(point)) + return m_visible.at(i); + } + // Deliberately nothing for the overflow chip and for empty space: the +N + // chip names a list, not a tag. + return {}; +} + +void TagStrip::contextMenuEvent(QContextMenuEvent *event) +{ + const QString tag = chipAt(event->pos()); + if (tag.isEmpty()) { + // Ignored rather than accepted, so a parent that offers its own menu + // still gets the chance to show it. + event->ignore(); + return; + } + + event->accept(); + emit tagContextMenuRequested(tag, event->globalPos()); +} + void TagStrip::paintEvent(QPaintEvent *) { if (m_visible.isEmpty()) @@ -119,12 +164,13 @@ void TagStrip::paintEvent(QPaintEvent *) const int top = (height() - (metrics.height() + TagChip::kPaddingY * 2)) / 2; int x = 0; - for (const QString &tag : m_visible) { - const QSize size = TagChip::sizeFor(metrics, tag); + for (int i = 0; i < m_visible.size(); ++i) { + const QString &tag = m_visible.at(i); + const QRect rect = chipRectAt(i); const QColor colour = m_tagColors ? m_tagColors->colourFor(tag) : TagColors().colourFor(tag); - TagChip::paint(&painter, QRect(QPoint(x, top), size), tag, colour); - x += size.width() + TagChip::kSpacing; + TagChip::paint(&painter, rect, tag, colour); + x = rect.right() + 1 + TagChip::kSpacing; } if (!m_hidden.isEmpty()) { diff --git a/src/tagstrip.h b/src/tagstrip.h index 4102bed..f59233b 100644 --- a/src/tagstrip.h +++ b/src/tagstrip.h @@ -18,10 +18,12 @@ #pragma once +#include #include #include class TagColors; +class QContextMenuEvent; /// One row of tag chips under the message pane. /// @@ -48,9 +50,30 @@ public: QStringList visibleTags() const { return m_visible; } QStringList hiddenTags() const { return m_hidden; } + /// The rect of the visible chip at `index`, empty when out of range. + /// + /// The SAME function paintEvent lays out from, so what is drawn and what + /// is clickable cannot drift. `CardDelegate::expanderRectFor` exists for + /// this reason and this follows it. + QRect chipRectAt(int index) const; + + /// The tag under `point`, empty when the point is on no chip. + /// + /// The trailing "+N" chip yields an empty string: it stands for a list of + /// tags rather than for one, so there is nothing a search could name. + QString chipAt(const QPoint &point) const; + +signals: + /// A visible chip was right-clicked. `globalPos` is where to pop a menu. + /// + /// The strip does not build the menu itself: what a tag can do belongs to + /// the window, which owns the query bar and the actions. + void tagContextMenuRequested(const QString &tag, const QPoint &globalPos); + protected: void paintEvent(QPaintEvent *event) override; void resizeEvent(QResizeEvent *event) override; + void contextMenuEvent(QContextMenuEvent *event) override; private: /// Recomputes the visible/hidden split for the current width. diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 5b0b85b..68148ee 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -55,3 +55,4 @@ add_qtmaildir_test(tagdialog) add_qtmaildir_test(tagrules) add_qtmaildir_test(rulequery) add_qtmaildir_test(searchterm) +add_qtmaildir_test(tagstrip) diff --git a/tests/test_tagstrip.cpp b/tests/test_tagstrip.cpp new file mode 100644 index 0000000..636b24b --- /dev/null +++ b/tests/test_tagstrip.cpp @@ -0,0 +1,108 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include +#include + +#include "tagstrip.h" + +/// The chip hit test. +/// +/// Asserted on rects from chipRectAt(), which is the SAME function paintEvent +/// lays out from, so a drawn chip and a clickable chip cannot drift apart. Not +/// asserted by rendering: a pixel probe cannot tell a chip that is drawn from a +/// chip that is drawn and clickable, and both halves have been broken +/// independently in this project before. +class TestTagStrip : public QObject +{ + Q_OBJECT +private slots: + void chipAtFindsEachVisibleTag(); + void chipAtMissesTheGapAndTheEdges(); + void chipAtIgnoresTheOverflowChip(); +}; + +void TestTagStrip::chipAtFindsEachVisibleTag() +{ + TagStrip strip; + strip.resize(600, 30); + strip.setTags({ QStringLiteral("inbox"), QStringLiteral("unread") }); + + const QStringList visible = strip.visibleTags(); + QCOMPARE(visible.size(), 2); + + // The guard: the geometry this test depends on must exist before the test + // can mean anything. A zero-width chip would make every lookup below miss + // and the test would pass for the wrong reason. + for (int i = 0; i < visible.size(); ++i) { + const QRect rect = strip.chipRectAt(i); + QVERIFY2(rect.width() > 0 && rect.height() > 0, + qPrintable(QStringLiteral("chip %1 has an empty rect").arg(i))); + QCOMPARE(strip.chipAt(rect.center()), visible.at(i)); + } +} + +void TestTagStrip::chipAtMissesTheGapAndTheEdges() +{ + TagStrip strip; + strip.resize(600, 30); + strip.setTags({ QStringLiteral("inbox"), QStringLiteral("unread") }); + QCOMPARE(strip.visibleTags().size(), 2); + + const QRect first = strip.chipRectAt(0); + const QRect second = strip.chipRectAt(1); + QVERIFY2(second.left() > first.right() + 1, + "the two chips must not touch, or there is no gap to test"); + + // Between the chips: no tag, so no menu entry rather than the nearest one. + const QPoint gap((first.right() + second.left()) / 2, first.center().y()); + QVERIFY(strip.chipAt(gap).isEmpty()); + + // Past the last chip, where the strip is empty space. + QVERIFY(strip.chipAt(QPoint(strip.width() - 1, first.center().y())).isEmpty()); +} + +void TestTagStrip::chipAtIgnoresTheOverflowChip() +{ + // The +N chip stands for a LIST of tags, not for a tag, so there is no + // single value a search could be built from. + TagStrip strip; + strip.resize(90, 30); + strip.setTags({ QStringLiteral("inbox"), QStringLiteral("unread"), + QStringLiteral("flagged"), QStringLiteral("attachment"), + QStringLiteral("replied") }); + + QVERIFY2(!strip.hiddenTags().isEmpty(), + "the strip must actually overflow, or this asserts nothing"); + QVERIFY2(!strip.visibleTags().isEmpty(), + "the strip must show at least one chip to test against"); + + // Every point across the strip either finds a VISIBLE tag or nothing. The + // overflow chip sits after the visible ones and must yield nothing. + for (int x = 0; x < strip.width(); x += 3) { + const QString found = strip.chipAt(QPoint(x, strip.height() / 2)); + if (!found.isEmpty()) + QVERIFY(strip.visibleTags().contains(found)); + } + + const QRect last = strip.chipRectAt(strip.visibleTags().size() - 1); + QVERIFY(strip.chipAt(QPoint(last.right() + 5, strip.height() / 2)).isEmpty()); +} + +QTEST_MAIN(TestTagStrip) +#include "test_tagstrip.moc" -- cgit v1.2.3 From f7f868ce00e01e4026be19437242537967875ce5 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 14 Aug 2026 12:53:58 +0200 Subject: feat(search): offer the header's fields for searching The menu lists what is searchable rather than hit-testing which line of a rich-text label was clicked, which breaks as soon as the label wraps. The values are collected by the pass that renders the header, so nothing parses the markup back into structure. From, To and Cc appear only for a single-message thread, sharing the condition with the header's own display: a thread's recipient differs message to message, and the menu must not offer what the header is not stating. The test fixture's Date: header named the wrong weekday, which Qt::RFC2822Date rejects outright, so no date offer would have been produced from it. --- src/messageview.cpp | 72 +++++++++++++++++++++++++++++++++ src/messageview.h | 37 +++++++++++++++++ tests/test_messageview.cpp | 99 +++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 207 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/src/messageview.cpp b/src/messageview.cpp index d2be380..8a1976d 100644 --- a/src/messageview.cpp +++ b/src/messageview.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -50,6 +51,7 @@ #include "cidschemehandler.h" #include "htmlbuilder.h" #include "requestinterceptor.h" +#include "searchterm.h" #include "tagstrip.h" #include "threadcidmap.h" #include "version.h" @@ -163,6 +165,12 @@ MessageView::MessageView(QWidget *parent) m_headerLabel->setWordWrap(true); m_headerLabel->setTextInteractionFlags(Qt::TextSelectableByMouse); + // Item 85: the header's values are searchable. CustomContextMenu rather + // than an action list, since the entries depend on what is displayed. + m_headerLabel->setContextMenuPolicy(Qt::CustomContextMenu); + connect(m_headerLabel, &QWidget::customContextMenuRequested, + this, &MessageView::showHeaderContextMenu); + // To the right of the header, per the user's decision: the summary answers // "who is this from", this answers "what actually happened to it". A button // and not only a shortcut, since "everything needs a memorized key" is the @@ -428,6 +436,11 @@ QString MessageView::headerMark(Marks::Mark mark) const void MessageView::updateHeader() { + // A stale offer list must not survive either exit: the early return below + // leaves nothing on screen to search, and the normal path rebuilds it from + // scratch a few lines down. + m_headerOffers.clear(); + if (m_items.isEmpty()) { m_headerLabel->clear(); m_detailsButton->hide(); @@ -440,6 +453,30 @@ void MessageView::updateHeader() // Re: prefixes that add nothing. const QString subject = m_items.first().message.subject; + // Collected by the pass that renders the label, from the same values, so + // nothing has to parse the rendered markup back into structure. + auto elided = [](const QString &value) { + constexpr int kMaxLabel = 40; + return value.size() > kMaxLabel + ? value.left(kMaxLabel) + QStringLiteral("...") + : value; + }; + + auto offer = [this](const QString &label, const QString &query) { + if (query.isEmpty()) + return; + m_headerOffers.append({ label, query }); + }; + + offer(tr("subject \"%1\"").arg(elided(subject)), + SearchTerm::field(QStringLiteral("subject"), subject)); + + const QDateTime sent = MimeParser::parseDate(m_items.first().message.date); + if (sent.isValid()) { + offer(tr("mail from %1").arg(sent.date().toString(Qt::ISODate)), + SearchTerm::onDate(sent.date())); + } + // Item 70's marks, beside the subject and OUTSIDE the message area. The // user asked for these two only: whether the thread is flagged and whether // it carries an attachment, which are the two states worth knowing before @@ -489,6 +526,16 @@ void MessageView::updateHeader() row(tr("From:"), message.from); row(tr("To:"), message.to); row(tr("Cc:"), message.cc); + + // Only here, sharing the condition with the header's own display: for + // a real thread these differ message to message, and the details + // dialog is where they are unambiguous. + offer(tr("sender %1").arg(elided(message.from)), + SearchTerm::field(QStringLiteral("from"), message.from)); + offer(tr("recipient %1").arg(elided(message.to)), + SearchTerm::field(QStringLiteral("to"), message.to)); + offer(tr("copied to %1").arg(elided(message.cc)), + SearchTerm::field(QStringLiteral("cc"), message.cc)); } else { text += QStringLiteral("
%1") .arg(tr("%n message(s) in thread", "", m_items.size())); @@ -497,6 +544,31 @@ void MessageView::updateHeader() m_headerLabel->setText(text); } +void MessageView::addSearchEntries(QMenu *menu, const QList &offers) +{ + for (const SearchOffer &entry : offers) { + auto *sub = menu->addMenu(tr("Search for %1").arg(entry.label)); + + auto *replace = sub->addAction(tr("Search for this")); + connect(replace, &QAction::triggered, this, + [this, entry]() { emit searchRequested(entry.query, false); }); + + auto *narrow = sub->addAction(tr("Add to search")); + connect(narrow, &QAction::triggered, this, + [this, entry]() { emit searchRequested(entry.query, true); }); + } +} + +void MessageView::showHeaderContextMenu(const QPoint &pos) +{ + if (m_headerOffers.isEmpty()) + return; + + QMenu menu(this); + addSearchEntries(&menu, m_headerOffers); + menu.exec(m_headerLabel->mapToGlobal(pos)); +} + void MessageView::showDetailsDialog() { if (m_items.isEmpty()) diff --git a/src/messageview.h b/src/messageview.h index 422869a..26ed216 100644 --- a/src/messageview.h +++ b/src/messageview.h @@ -25,8 +25,10 @@ #include "htmlbuilder.h" #include "marks.h" #include "mimeparser.h" +#include "searchterm.h" class QLabel; +class QMenu; class QPushButton; class QWebEngineView; class QWebEngineProfile; @@ -127,6 +129,17 @@ public: /// is rendered or the notice is hidden. QString staleMessageId() const { return m_staleMessageId; } + /// What the header can be searched for, given what it is currently showing. + /// + /// From, To and Cc appear only for a single-message thread, which is + /// exactly when the header displays them: for a real thread the recipient + /// differs message to message and the header says only the subject and the + /// count. The menu must never offer a value the header is not stating. + /// + /// The values come from the same pass that renders the label, never from + /// parsing it back: rich text does not survive a second parse. + QList headerSearchOffers() const { return m_headerOffers; } + public slots: void toggleHtml(); void loadRemoteContent(); @@ -158,6 +171,18 @@ signals: void staleThreadRecoveryRequested(const QString &threadId, const QString &messageId); + /// The user chose a search from one of the pane's context menus. + /// + /// `extend` narrows the current query rather than replacing it. The view + /// does not know what the query bar holds and must not: the window owns + /// that field and does the combining. + /// + /// Separate from queryRequested(), which carries a gate against a link in + /// a rendered document driving the thread list. These menus are chrome + /// built by our own code from values we extracted, so they need no gate, + /// and widening the existing signal would change what that gate protects. + void searchRequested(const QString &query, bool extend); + 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 @@ -207,6 +232,15 @@ private: /// Every attachment in the thread, in the order the messages render. QList allAttachments() const; + /// Builds and pops the header's menu at `pos`, in the label's coordinates. + void showHeaderContextMenu(const QPoint &pos); + + /// Appends a "Search for ..." submenu per offer, each holding the replace + /// and the narrow operation. + /// + /// Shared with the web view's menu in a later task so the two cannot grow + /// different wording or a different pair of operations. + void addSearchEntries(QMenu *menu, const QList &offers); QList m_items; bool m_preferHtml = true; @@ -232,4 +266,7 @@ private: QPushButton *m_detailsButton = nullptr; QWidget *m_attachmentBar = nullptr; TagStrip *m_tagStrip = nullptr; + + /// Populated by updateHeader(), consumed by the header's context menu. + QList m_headerOffers; }; diff --git a/tests/test_messageview.cpp b/tests/test_messageview.cpp index e94fb86..6403308 100644 --- a/tests/test_messageview.cpp +++ b/tests/test_messageview.cpp @@ -49,6 +49,9 @@ private slots: void detailsDialogIsOfferedForEveryThread(); void placeholderRendersAndReportsItself(); void aMessageBodyCannotRunAQuery(); + void headerOffersSubjectDateAndSenderForOneMessage(); + void headerOffersNoSenderForARealThread(); + void headerOffersNothingForAnAbsentField(); private: QWebEngineView *webViewOf(MessageView *view) const @@ -372,7 +375,7 @@ static ThreadRenderItem oneMessage() message.to = QStringLiteral("Recipient "); message.cc = QStringLiteral("Copied "); message.subject = QStringLiteral("Quarterly report"); - message.date = QStringLiteral("Mon, 4 Aug 2026 09:00:00 +0200"); + message.date = QStringLiteral("Tue, 4 Aug 2026 09:00:00 +0200"); message.plainBody = QStringLiteral("body"); ThreadRenderItem item; @@ -566,5 +569,99 @@ void TestMessageView::aMessageBodyCannotRunAQuery() QVERIFY(queries.isEmpty()); } +void TestMessageView::headerOffersSubjectDateAndSenderForOneMessage() +{ + MessageView view; + view.showThread({ oneMessage() }); + + const QList offers = view.headerSearchOffers(); + + QStringList queries; + for (const SearchOffer &offer : offers) + queries << offer.query; + const QString shown = queries.join(QStringLiteral(" | ")); + + QVERIFY2(queries.contains(QStringLiteral("subject:\"Quarterly report\"")), + qPrintable(shown)); + QVERIFY2(queries.contains( + QStringLiteral("from:\"Sender \"")), + qPrintable(shown)); + QVERIFY2(queries.contains( + QStringLiteral("to:\"Recipient \"")), + qPrintable(shown)); + QVERIFY2(queries.contains( + QStringLiteral("cc:\"Copied \"")), + qPrintable(shown)); + + // The date is offered as a one-day range. Qt::RFC2822Date checks the + // weekday against the date, so a fixture with the wrong day silently + // produces no offer at all: 2026-08-04 is a Tuesday. + QVERIFY2(queries.contains(QStringLiteral("date:2026-08-04..2026-08-04")), + qPrintable(shown)); + + // Every offer carries a label the menu shows, naming the value so the user + // can see what they are about to search for. + for (const SearchOffer &offer : offers) { + QVERIFY(!offer.label.isEmpty()); + QVERIFY(!offer.query.isEmpty()); + } +} + +void TestMessageView::headerOffersNoSenderForARealThread() +{ + // The header shows From/To/Cc only for a single-message thread, because a + // thread's recipient differs message to message. The menu shares that + // condition: it must never offer a value the header is not stating. + ThreadRenderItem first = oneMessage(); + ThreadRenderItem second = oneMessage(); + second.message.from = QStringLiteral("Recipient "); + second.message.to = QStringLiteral("Sender "); + + MessageView view; + view.showThread({ first, second }); + + QStringList queries; + for (const SearchOffer &offer : view.headerSearchOffers()) + queries << offer.query; + + // THE GUARD. A test asserting only that something is absent passes against + // no implementation whatever. Subject and date must still be offered, + // which proves the list was built before the absences below mean anything. + QVERIFY2(!queries.isEmpty(), "no offers at all: the list was never built"); + QVERIFY(queries.contains(QStringLiteral("subject:\"Quarterly report\""))); + QVERIFY(queries.contains(QStringLiteral("date:2026-08-04..2026-08-04"))); + + for (const QString &query : queries) { + QVERIFY2(!query.startsWith(QStringLiteral("from:")), + qPrintable(QStringLiteral("thread offered %1").arg(query))); + QVERIFY2(!query.startsWith(QStringLiteral("to:")), + qPrintable(QStringLiteral("thread offered %1").arg(query))); + QVERIFY2(!query.startsWith(QStringLiteral("cc:")), + qPrintable(QStringLiteral("thread offered %1").arg(query))); + } +} + +void TestMessageView::headerOffersNothingForAnAbsentField() +{ + // cc:"" parses cleanly and matches nothing, so an entry built from an + // empty header would look enabled and silently do nothing. + ThreadRenderItem item = oneMessage(); + item.message.cc.clear(); + + MessageView view; + view.showThread({ item }); + + QStringList queries; + for (const SearchOffer &offer : view.headerSearchOffers()) + queries << offer.query; + + // Guard first, then the absence. + QVERIFY2(!queries.isEmpty(), "no offers at all: the list was never built"); + QVERIFY(queries.contains(QStringLiteral("subject:\"Quarterly report\""))); + + for (const QString &query : queries) + QVERIFY(!query.startsWith(QStringLiteral("cc:"))); +} + QTEST_MAIN(TestMessageView) #include "test_messageview.moc" -- cgit v1.2.3 From 8a2e02a3e1a669a738d1de4615720378d5cc2d38 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 14 Aug 2026 12:56:53 +0200 Subject: feat(search): search for the selected body text selectedText() reads the selection with no script injection; JavaScript stays disabled in the profile. The page's standard menu is kept and the entries are added to it below a separator. The quoting is tested through a function taking the text, so it needs no live web engine: a selection is arbitrary prose and can carry quotes, newlines and query syntax, none of which notmuch reports as an error. --- src/messageview.cpp | 42 ++++++++++++++++++++++++++++++++++++++++++ src/messageview.h | 12 ++++++++++++ tests/test_messageview.cpp | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+) (limited to 'tests') diff --git a/src/messageview.cpp b/src/messageview.cpp index 8a1976d..5f7c9d2 100644 --- a/src/messageview.cpp +++ b/src/messageview.cpp @@ -153,6 +153,12 @@ MessageView::MessageView(QWidget *parent) settings->setAttribute(QWebEngineSettings::PluginsEnabled, false); settings->setAttribute(QWebEngineSettings::FullScreenSupportEnabled, false); + // Item 85: a selection in the body is searchable. CustomContextMenu so the + // page's standard entries survive and the search is added to them. + m_view->setContextMenuPolicy(Qt::CustomContextMenu); + connect(m_view, &QWidget::customContextMenuRequested, + this, &MessageView::showBodyContextMenu); + // Ctrl+wheel zoom. The filter goes on the application rather than on // m_view: the wheel event is delivered to an internal QQuickWidget the // view creates lazily, so there is no child to filter at this point and a @@ -569,6 +575,42 @@ void MessageView::showHeaderContextMenu(const QPoint &pos) menu.exec(m_headerLabel->mapToGlobal(pos)); } +SearchOffer MessageView::selectionSearchOffer(const QString &selectedText) const +{ + const QString query = SearchTerm::quote(selectedText); + if (query.isEmpty()) + return {}; + + constexpr int kMaxLabel = 40; + const QString shown = selectedText.simplified(); + return { shown.size() > kMaxLabel + ? shown.left(kMaxLabel) + QStringLiteral("...") + : shown, + query }; +} + +void MessageView::showBodyContextMenu(const QPoint &pos) +{ + // The page's own menu first: copy, select all and the rest stay exactly as + // they were. This adds to that menu rather than replacing it. + QMenu *menu = m_view->createStandardContextMenu(); + if (!menu) + menu = new QMenu(this); + menu->setAttribute(Qt::WA_DeleteOnClose); + + // selectedText() reads the selection out of the render process with no + // script injection. JavaScript is disabled in this profile and stays so. + const SearchOffer offer = selectionSearchOffer(m_view->page()->selectedText()); + if (!offer.query.isEmpty()) { + menu->addSeparator(); + addSearchEntries(menu, { offer }); + } + + // popup() rather than exec(): the menu owns itself via WA_DeleteOnClose and + // must not block this handler. + menu->popup(m_view->mapToGlobal(pos)); +} + void MessageView::showDetailsDialog() { if (m_items.isEmpty()) diff --git a/src/messageview.h b/src/messageview.h index 26ed216..3e600f0 100644 --- a/src/messageview.h +++ b/src/messageview.h @@ -140,6 +140,15 @@ public: /// parsing it back: rich text does not survive a second parse. QList headerSearchOffers() const { return m_headerOffers; } + /// The offer for a body selection, its query empty when there is nothing + /// usable selected. + /// + /// Takes the text rather than reading the page, so the quoting is testable + /// without a live web engine and a rendered document. A selection is + /// arbitrary prose and can carry quotes, newlines and query syntax, none + /// of which notmuch reports as an error. + SearchOffer selectionSearchOffer(const QString &selectedText) const; + public slots: void toggleHtml(); void loadRemoteContent(); @@ -235,6 +244,9 @@ private: /// Builds and pops the header's menu at `pos`, in the label's coordinates. void showHeaderContextMenu(const QPoint &pos); + /// Builds and pops the web view's menu, keeping its standard entries. + void showBodyContextMenu(const QPoint &pos); + /// Appends a "Search for ..." submenu per offer, each holding the replace /// and the narrow operation. /// diff --git a/tests/test_messageview.cpp b/tests/test_messageview.cpp index 6403308..f83a377 100644 --- a/tests/test_messageview.cpp +++ b/tests/test_messageview.cpp @@ -52,6 +52,7 @@ private slots: void headerOffersSubjectDateAndSenderForOneMessage(); void headerOffersNoSenderForARealThread(); void headerOffersNothingForAnAbsentField(); + void bodySelectionBecomesAQuotedSearch(); private: QWebEngineView *webViewOf(MessageView *view) const @@ -663,5 +664,38 @@ void TestMessageView::headerOffersNothingForAnAbsentField() QVERIFY(!query.startsWith(QStringLiteral("cc:"))); } +void TestMessageView::bodySelectionBecomesAQuotedSearch() +{ + // The selection reaches the query as ONE quoted term. Asserted on the + // constructed string: a query that lost its quoting is not an error to + // notmuch, it simply matches nothing, so nothing downstream would report + // this being wrong. + // + // Takes the text as an argument rather than reading the page, so the + // quoting is testable without a live web engine and a rendered document. + MessageView view; + + QCOMPARE(view.selectionSearchOffer(QStringLiteral("invoice 4471")).query, + QStringLiteral("\"invoice 4471\"")); + + // A selection spanning paragraphs arrives full of newlines. + QCOMPARE(view.selectionSearchOffer( + QStringLiteral("first line\n\nsecond line")).query, + QStringLiteral("\"first line second line\"")); + + // Query syntax in the selection is data, not syntax: it is quoted, not + // interpreted, so a selection reading "a or b" searches for that phrase. + QCOMPARE(view.selectionSearchOffer(QStringLiteral("tag:inbox or x")).query, + QStringLiteral("\"tag:inbox or x\"")); + + // Nothing selected means no entry, rather than an entry searching for "". + QVERIFY(view.selectionSearchOffer(QString()).query.isEmpty()); + QVERIFY(view.selectionSearchOffer(QStringLiteral(" \n ")).query.isEmpty()); + + // A usable offer always carries a label for the menu to show. + QVERIFY(!view.selectionSearchOffer(QStringLiteral("invoice 4471")) + .label.isEmpty()); +} + QTEST_MAIN(TestMessageView) #include "test_messageview.moc" -- cgit v1.2.3 From 2f124b22920fb290684206a2c905996f1c378fd7 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 14 Aug 2026 13:01:04 +0200 Subject: feat(details): rebuild the message details dialog as rows A text box could not carry a per-value context menu without parsing displayed text back into structure, and the user did not want a text box. Each row now holds its own value, its message index and its query, built from the parsed message. Every value label states Qt::PlainText. The QPlainTextEdit this replaced was plain by design rather than by style: header values come from strangers, and a QLabel guesses the format under AutoText. --- src/CMakeLists.txt | 1 + src/messagedetailsdialog.cpp | 153 ++++++++++++++++++++++++++++++ src/messagedetailsdialog.h | 87 +++++++++++++++++ src/messageview.cpp | 55 ++--------- tests/CMakeLists.txt | 1 + tests/test_messagedetailsdialog.cpp | 182 ++++++++++++++++++++++++++++++++++++ 6 files changed, 430 insertions(+), 49 deletions(-) create mode 100644 src/messagedetailsdialog.cpp create mode 100644 src/messagedetailsdialog.h create mode 100644 tests/test_messagedetailsdialog.cpp (limited to 'tests') diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 382f3b8..ee1f621 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -22,6 +22,7 @@ add_library(qtmaildir_lib STATIC syncmonitor.cpp threadcidmap.cpp messageview.cpp + messagedetailsdialog.cpp mainwindow.cpp querycompleter.cpp rulequery.cpp diff --git a/src/messagedetailsdialog.cpp b/src/messagedetailsdialog.cpp new file mode 100644 index 0000000..058814b --- /dev/null +++ b/src/messagedetailsdialog.cpp @@ -0,0 +1,153 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "messagedetailsdialog.h" + +#include +#include +#include +#include +#include +#include +#include + +#include "mimeparser.h" +#include "searchterm.h" + +MessageDetailsDialog::MessageDetailsDialog(const QList &items, + QWidget *parent) + : QDialog(parent) +{ + setWindowTitle(tr("Message details")); + setObjectName(QStringLiteral("messageDetailsDialog")); + + buildRows(items); + + auto *layout = new QVBoxLayout(this); + + // Scrollable: a long thread has many rows, and the dialog must not grow + // past the screen to show them. + auto *scroll = new QScrollArea(this); + scroll->setWidgetResizable(true); + auto *content = new QWidget(scroll); + auto *grid = new QGridLayout(content); + + // A monospaced value keeps a long id or address readable as the record it + // is, which is what the text box did well and is worth carrying over. + const QFont fixed = QFontDatabase::systemFont(QFontDatabase::FixedFont); + + int gridRow = 0; + int lastMessage = -1; + for (const HeaderRow &row : std::as_const(m_rows)) { + if (items.size() > 1 && row.messageIndex != lastMessage) { + lastMessage = row.messageIndex; + auto *heading = new QLabel( + tr("Message %1 of %2").arg(row.messageIndex + 1) + .arg(items.size()), + content); + heading->setTextFormat(Qt::PlainText); + QFont headingFont = heading->font(); + headingFont.setBold(true); + heading->setFont(headingFont); + grid->addWidget(heading, gridRow, 0, 1, 2); + ++gridRow; + } + + auto *label = new QLabel(row.label, content); + label->setTextFormat(Qt::PlainText); + label->setAlignment(Qt::AlignTop | Qt::AlignLeft); + grid->addWidget(label, gridRow, 0); + + // PlainText stated, not inferred. A QLabel guesses under AutoText, and + // this value came from a stranger. + auto *value = new QLabel(row.value, content); + value->setTextFormat(Qt::PlainText); + value->setFont(fixed); + value->setWordWrap(true); + value->setTextInteractionFlags(Qt::TextSelectableByMouse); + + if (!row.query.isEmpty()) { + value->setContextMenuPolicy(Qt::CustomContextMenu); + connect(value, &QWidget::customContextMenuRequested, this, + [this, value, row](const QPoint &pos) { + QMenu menu(this); + auto *replace = menu.addAction(tr("Search for this")); + connect(replace, &QAction::triggered, this, + [this, row]() { requestSearch(row, false); }); + auto *narrow = menu.addAction(tr("Add to search")); + connect(narrow, &QAction::triggered, this, + [this, row]() { requestSearch(row, true); }); + menu.exec(value->mapToGlobal(pos)); + }); + } + + grid->addWidget(value, gridRow, 1); + ++gridRow; + } + + grid->setColumnStretch(1, 1); + grid->setRowStretch(gridRow, 1); + scroll->setWidget(content); + layout->addWidget(scroll); + + auto *buttons = new QDialogButtonBox(QDialogButtonBox::Close, this); + connect(buttons, &QDialogButtonBox::rejected, this, &QDialog::reject); + layout->addWidget(buttons); + + resize(700, 400); +} + +void MessageDetailsDialog::buildRows(const QList &items) +{ + for (int i = 0; i < items.size(); ++i) { + const ParsedMessage &message = items.at(i).message; + + auto add = [this, i](const QString &field, const QString &label, + const QString &value, const QString &query) { + if (value.isEmpty()) + return; // An empty row reads as a rendering fault. + m_rows.append({ field, label, value, query, i }); + }; + + add(QStringLiteral("subject"), tr("Subject:"), message.subject, + SearchTerm::field(QStringLiteral("subject"), message.subject)); + add(QStringLiteral("from"), tr("From:"), message.from, + SearchTerm::field(QStringLiteral("from"), message.from)); + add(QStringLiteral("to"), tr("To:"), message.to, + SearchTerm::field(QStringLiteral("to"), message.to)); + add(QStringLiteral("cc"), tr("Cc:"), message.cc, + SearchTerm::field(QStringLiteral("cc"), message.cc)); + + // The raw header is shown, but the query is a one-day range: a text + // match on an RFC 2822 string would match almost nothing. + const QDateTime sent = MimeParser::parseDate(message.date); + add(QStringLiteral("date"), tr("Date:"), message.date, + sent.isValid() ? SearchTerm::onDate(sent.date()) : QString()); + + // Shown but not searchable: an id names one message, and the thread + // holding it is already on screen. + add(QString(), tr("Message-Id:"), message.messageId, QString()); + } +} + +void MessageDetailsDialog::requestSearch(const HeaderRow &row, bool extend) +{ + if (row.query.isEmpty()) + return; + emit searchRequested(row.query, extend); +} diff --git a/src/messagedetailsdialog.h b/src/messagedetailsdialog.h new file mode 100644 index 0000000..f865913 --- /dev/null +++ b/src/messagedetailsdialog.h @@ -0,0 +1,87 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#pragma once + +#include +#include +#include + +#include "htmlbuilder.h" + +/// One header of one message, as shown and as searched for. +/// +/// The query is built when the row is, from the parsed value, so nothing has +/// to parse displayed text back into structure. That is the whole reason this +/// dialog stopped being a text box. +struct HeaderRow +{ + /// notmuch's field name, or empty for a header with no searchable form. + /// Wire format, never translated. + QString field; + + /// Translated label shown at the start of the row, e.g. "From:". + QString label; + + /// The header's value, verbatim and untrusted. + QString value; + + /// The finished query, empty when the header has no searchable form. + QString query; + + /// Which message of the thread this row belongs to, zero-based. + int messageIndex = 0; +}; + +/// The full headers of every message in a thread, read-only. +/// +/// Rows rather than one text box, so a value can carry its own context menu +/// without anything parsing rendered text back into structure. The user also +/// asked not to be shown a text box. +/// +/// **Every value label is explicitly `Qt::PlainText`.** This replaced a +/// `QPlainTextEdit` whose plain-textness was a security property rather than a +/// style: header values come from strangers, and plain text cannot interpret +/// markup, so there is nothing to escape and nothing that can render. A QLabel +/// guesses under `Qt::AutoText`, so stating the format is what preserves that. +class MessageDetailsDialog : public QDialog +{ + Q_OBJECT +public: + explicit MessageDetailsDialog(const QList &items, + QWidget *parent = nullptr); + + /// The rows on display, in order. Exposed for testing without rendering. + QList rows() const { return m_rows; } + + /// Emits searchRequested for `row`, or nothing when the row carries no + /// searchable query. The menu entries call this; a test can too, without + /// popping a menu. + void requestSearch(const HeaderRow &row, bool extend); + +signals: + /// The user chose a search from a row's menu. `extend` narrows the current + /// query rather than replacing it. + void searchRequested(const QString &query, bool extend); + +private: + /// Builds the rows from the thread, one group per message. + void buildRows(const QList &items); + + QList m_rows; +}; diff --git a/src/messageview.cpp b/src/messageview.cpp index 5f7c9d2..fbd43d6 100644 --- a/src/messageview.cpp +++ b/src/messageview.cpp @@ -22,8 +22,6 @@ #include #include #include -#include -#include #include #include #include @@ -50,6 +48,7 @@ #include "cidschemehandler.h" #include "htmlbuilder.h" +#include "messagedetailsdialog.h" #include "requestinterceptor.h" #include "searchterm.h" #include "tagstrip.h" @@ -616,53 +615,11 @@ void MessageView::showDetailsDialog() if (m_items.isEmpty()) return; - QDialog dialog(this); - dialog.setWindowTitle(tr("Message details")); - - auto *layout = new QVBoxLayout(&dialog); - - auto *details = new QPlainTextEdit(&dialog); - details->setReadOnly(true); - // A monospaced font keeps a long Received chain readable as the wrapped - // record it is. - details->setFont(QFontDatabase::systemFont(QFontDatabase::FixedFont)); - details->setLineWrapMode(QPlainTextEdit::NoWrap); - - // setPlainText, and a QPlainTextEdit rather than a label: this dialog shows - // header values verbatim, and those come from strangers. Plain text cannot - // interpret markup, so there is nothing here to escape and nothing that - // could render. - QString text; - for (int i = 0; i < m_items.size(); ++i) { - const ParsedMessage &message = m_items.at(i).message; - - if (i > 0) - text += QLatin1Char('\n'); - if (m_items.size() > 1) - text += tr("--- Message %1 of %2 ---") - .arg(i + 1).arg(m_items.size()) + QLatin1Char('\n'); - - auto line = [&text](const QString &label, const QString &value) { - if (!value.isEmpty()) - text += label + QLatin1Char(' ') + value + QLatin1Char('\n'); - }; - - line(tr("Subject:"), message.subject); - line(tr("From:"), message.from); - line(tr("To:"), message.to); - line(tr("Cc:"), message.cc); - line(tr("Date:"), message.date); - line(tr("Message-Id:"), message.messageId); - } - details->setPlainText(text); - - layout->addWidget(details); - - auto *buttons = new QDialogButtonBox(QDialogButtonBox::Close, &dialog); - connect(buttons, &QDialogButtonBox::rejected, &dialog, &QDialog::reject); - layout->addWidget(buttons); - - dialog.resize(700, 400); + MessageDetailsDialog dialog(m_items, this); + // The dialog's searches are the pane's searches: one signal reaches the + // window whichever surface the user used. + connect(&dialog, &MessageDetailsDialog::searchRequested, + this, &MessageView::searchRequested); dialog.exec(); } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 68148ee..5f7bd48 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -56,3 +56,4 @@ add_qtmaildir_test(tagrules) add_qtmaildir_test(rulequery) add_qtmaildir_test(searchterm) add_qtmaildir_test(tagstrip) +add_qtmaildir_test(messagedetailsdialog) diff --git a/tests/test_messagedetailsdialog.cpp b/tests/test_messagedetailsdialog.cpp new file mode 100644 index 0000000..4e685ce --- /dev/null +++ b/tests/test_messagedetailsdialog.cpp @@ -0,0 +1,182 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include +#include +#include + +#include "htmlbuilder.h" +#include "messagedetailsdialog.h" + +/// The details dialog, which shows every header of every message in a thread. +/// +/// Rows rather than one text box since item 85, so a value can carry its own +/// context menu without anything parsing rendered text back into structure. +class TestMessageDetailsDialog : public QObject +{ + Q_OBJECT +private slots: + void showsEveryHeaderOfEveryMessage(); + void valueLabelsCannotRenderMarkup(); + void offersASearchForEachValue(); + void omitsAnEmptyHeader(); + void messageIdIsShownButNotSearchable(); + +private: + /// One message, with every header populated. The date's weekday matches + /// the date: Qt::RFC2822Date validates the two against each other, and + /// 2026-08-14 is a Friday. + ThreadRenderItem oneMessage() const + { + ThreadRenderItem item; + item.message.ok = true; + item.message.subject = QStringLiteral("Quarterly report"); + item.message.from = QStringLiteral("Sender "); + item.message.to = QStringLiteral("Recipient "); + item.message.cc = QStringLiteral("Copied "); + item.message.date = QStringLiteral("Fri, 14 Aug 2026 09:30:00 +0200"); + item.message.messageId = QStringLiteral(""); + return item; + } +}; + +void TestMessageDetailsDialog::showsEveryHeaderOfEveryMessage() +{ + ThreadRenderItem second = oneMessage(); + second.message.subject = QStringLiteral("Re: Quarterly report"); + + MessageDetailsDialog dialog({ oneMessage(), second }); + + const QList rows = dialog.rows(); + QVERIFY2(!rows.isEmpty(), "no rows: the dialog was never populated"); + + // Both messages are represented, each row knowing which one it belongs to. + QVERIFY(std::any_of(rows.cbegin(), rows.cend(), [](const HeaderRow &row) { + return row.messageIndex == 0; + })); + QVERIFY(std::any_of(rows.cbegin(), rows.cend(), [](const HeaderRow &row) { + return row.messageIndex == 1; + })); + + QStringList values; + for (const HeaderRow &row : rows) + values << row.value; + QVERIFY(values.contains(QStringLiteral("Sender "))); + QVERIFY(values.contains(QStringLiteral("Re: Quarterly report"))); + QVERIFY(values.contains(QStringLiteral(""))); +} + +void TestMessageDetailsDialog::valueLabelsCannotRenderMarkup() +{ + // The QPlainTextEdit this replaced was plain by DESIGN, not by style: + // header values come from strangers and plain text cannot interpret + // markup. A QLabel guesses under Qt::AutoText, so every label states its + // format rather than relying on escaping, which is the same protection one + // mistake away from failing. + ThreadRenderItem hostile = oneMessage(); + hostile.message.subject = + QStringLiteral("bold"); + + MessageDetailsDialog dialog({ hostile }); + + const QList labels = dialog.findChildren(); + QVERIFY2(!labels.isEmpty(), "no labels: the dialog was never populated"); + + bool sawTheSubject = false; + for (const QLabel *label : labels) { + QCOMPARE(label->textFormat(), Qt::PlainText); + if (label->text().contains(QStringLiteral("bold"))) + sawTheSubject = true; + } + + // The markup survives AS TEXT, which is the proof it was not interpreted. + QVERIFY2(sawTheSubject, "the hostile subject never reached a label"); +} + +void TestMessageDetailsDialog::offersASearchForEachValue() +{ + MessageDetailsDialog dialog({ oneMessage() }); + + QSignalSpy spy(&dialog, &MessageDetailsDialog::searchRequested); + QVERIFY(spy.isValid()); + + const QList rows = dialog.rows(); + const auto from = std::find_if( + rows.cbegin(), rows.cend(), [](const HeaderRow &row) { + return row.field == QStringLiteral("from"); + }); + QVERIFY2(from != rows.cend(), "no From row to search from"); + QCOMPARE(from->query, QStringLiteral("from:\"Sender \"")); + + // The date becomes a one-day range rather than a text match on the header. + const auto date = std::find_if( + rows.cbegin(), rows.cend(), [](const HeaderRow &row) { + return row.field == QStringLiteral("date"); + }); + QVERIFY2(date != rows.cend(), "no Date row"); + QCOMPARE(date->query, QStringLiteral("date:2026-08-14..2026-08-14")); + + // Replacing and narrowing are both offered, and the flag distinguishes them. + dialog.requestSearch(*from, false); + dialog.requestSearch(*from, true); + QCOMPARE(spy.count(), 2); + QCOMPARE(spy.at(0).at(0).toString(), from->query); + QCOMPARE(spy.at(0).at(1).toBool(), false); + QCOMPARE(spy.at(1).at(1).toBool(), true); +} + +void TestMessageDetailsDialog::omitsAnEmptyHeader() +{ + ThreadRenderItem noCc = oneMessage(); + noCc.message.cc.clear(); + + MessageDetailsDialog dialog({ noCc }); + + const QList rows = dialog.rows(); + // Guard first: an absence assertion alone passes against no implementation. + QVERIFY2(!rows.isEmpty(), "no rows: the dialog was never populated"); + QVERIFY(std::any_of(rows.cbegin(), rows.cend(), [](const HeaderRow &row) { + return row.field == QStringLiteral("from"); + })); + + for (const HeaderRow &row : rows) + QVERIFY(row.field != QStringLiteral("cc")); +} + +void TestMessageDetailsDialog::messageIdIsShownButNotSearchable() +{ + // A message id names one message, and the thread holding it is already on + // screen, so there is nothing useful to search for. It is still shown. + MessageDetailsDialog dialog({ oneMessage() }); + + const QList rows = dialog.rows(); + const auto id = std::find_if( + rows.cbegin(), rows.cend(), [](const HeaderRow &row) { + return row.value == QStringLiteral(""); + }); + QVERIFY2(id != rows.cend(), "the message id is not shown at all"); + QVERIFY(id->query.isEmpty()); + + // And asking to search it emits nothing rather than an empty query. + QSignalSpy spy(&dialog, &MessageDetailsDialog::searchRequested); + dialog.requestSearch(*id, false); + QCOMPARE(spy.count(), 0); +} + +QTEST_MAIN(TestMessageDetailsDialog) +#include "test_messagedetailsdialog.moc" -- cgit v1.2.3 From 5b18bc2123d8bf0830ebf8ab3f735c9496342e97 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 14 Aug 2026 13:36:11 +0200 Subject: feat(search): run a search asked for from the message pane The panes carry a finished query and know nothing of the query bar; the window sets the field and calls the existing runner, so the account scope and the generation counter keep working as they do for a typed query. Narrowing combines here rather than in a pane, because only the window can see what the bar currently holds. The tag strip's chips join the header, the body selection and the details dialog as a fourth surface. Also fixes the details dialog to actually close when a search is chosen: the comment above the connection already described this requirement, but nothing called accept() or reject(), so the dialog stayed open, the query ran behind it, and the modal exec() never returned. This hung the whole test suite on QT_QPA_PLATFORM=offscreen once a covering test was added. --- src/mainwindow.cpp | 18 +++++++++++++ src/mainwindow.h | 7 ++++++ src/messageview.cpp | 30 ++++++++++++++++++++-- tests/test_mainwindow.cpp | 63 ++++++++++++++++++++++++++++++++++++++++++++++ tests/test_messageview.cpp | 53 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 169 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index bb6aa43..115646e 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -55,6 +55,7 @@ #include "querycompleter.h" #include "carddelegate.h" #include "cardlayout.h" +#include "searchterm.h" #include "tagchip.h" #include "tagdialog.h" #include "savequerydialog.h" @@ -672,6 +673,8 @@ void MainWindow::buildUi() this, &MainWindow::onPlaceholderQueryRequested); connect(m_messageView, &MessageView::staleThreadRecoveryRequested, this, &MainWindow::recoverStaleThread); + connect(m_messageView, &MessageView::searchRequested, + this, &MainWindow::runSearchFromPane); m_splitter = new QSplitter(Qt::Horizontal, central); m_splitter->addWidget(m_threadView); @@ -1648,6 +1651,21 @@ void MainWindow::onPlaceholderQueryRequested(const QString &query) runCurrentQuery(); } +void MainWindow::runSearchFromPane(const QString &query, bool extend) +{ + if (query.isEmpty()) + return; + + const QString next = + extend ? SearchTerm::extend(m_queryEdit->text(), query) : query; + + // Through the query bar and the existing runner, so the account scope, the + // generation counter and the flat-mode reset all behave exactly as they do + // for a typed query. Nothing here builds a second query path. + m_queryEdit->setText(next); + runCurrentQuery(); +} + void MainWindow::showWarnings() { const QStringList warnings = m_config.warnings() + m_keyMap.warnings(); diff --git a/src/mainwindow.h b/src/mainwindow.h index a68fcd9..f54a889 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -373,6 +373,13 @@ private slots: /// Runs a query the user clicked on the placeholder pane. void onPlaceholderQueryRequested(const QString &query); + /// Runs a search asked for from the message pane. + /// + /// `extend` narrows the current query rather than replacing it. The panes + /// carry a finished query and no knowledge of the bar; the combining + /// happens here, because only the window can see what the bar holds. + void runSearchFromPane(const QString &query, bool extend); + /// Runs one tagging rule's query in the thread list, so the user can see /// which mail it collects. The rules dialog stays open; the point is to /// compare the rule against its results. diff --git a/src/messageview.cpp b/src/messageview.cpp index fbd43d6..1e6c256 100644 --- a/src/messageview.cpp +++ b/src/messageview.cpp @@ -252,6 +252,20 @@ MessageView::MessageView(QWidget *parent) m_tagStrip = new TagStrip(this); m_tagStrip->hide(); + // Item 85: a tag chip is searchable. The strip reports which chip was hit + // and where; what a tag can do is decided here, beside the other menus, so + // all three surfaces offer the same pair of operations. + connect(m_tagStrip, &TagStrip::tagContextMenuRequested, this, + [this](const QString &tag, const QPoint &globalPos) { + const QString query = SearchTerm::tag(tag); + if (query.isEmpty()) + return; + + QMenu menu(this); + addSearchEntries(&menu, { { tr("tag %1").arg(tag), query } }); + menu.exec(globalPos); + }); + auto *layout = new QVBoxLayout(this); layout->addLayout(headerRow); layout->addLayout(blockedRow); @@ -616,10 +630,22 @@ void MessageView::showDetailsDialog() return; MessageDetailsDialog dialog(m_items, this); + // The dialog's searches are the pane's searches: one signal reaches the // window whichever surface the user used. - connect(&dialog, &MessageDetailsDialog::searchRequested, - this, &MessageView::searchRequested); + // + // It CLOSES on the way out, and that is not tidiness. The dialog is modal, + // so without this the query runs and the thread list repaints behind a + // window the user still has to dismiss, making the search look like it did + // nothing. The dialog is also built from m_items, which the new query is + // about to replace, so what it displays would describe a thread the pane + // has already stopped showing. + connect(&dialog, &MessageDetailsDialog::searchRequested, this, + [this, &dialog](const QString &query, bool extend) { + emit searchRequested(query, extend); + dialog.accept(); + }); + dialog.exec(); } diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 6e300bf..8b60e37 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -88,6 +88,9 @@ private slots: void markReadTimerIsNotArmedForAReadThread(); void aConfirmedEditArmsTheAutoSync(); void autoSyncDebouncesABurstOfEdits(); + void aSearchFromThePaneReplacesTheQuery(); + void aSearchFromThePaneCanNarrowTheQuery(); + void narrowingAnEmptyQueryBarIsAPlainSearch(); void autoSyncIsNotArmedWhenDisabledOrWithNothingPending(); void autoSyncSkipsWhileABackgroundSyncIsRunning(); void aSuccessfulSyncRefreshesRatherThanRerunningTheQuery(); @@ -3725,6 +3728,66 @@ void TestMainWindow::autoSyncDebouncesABurstOfEdits() 1); } +void TestMainWindow::aSearchFromThePaneReplacesTheQuery() +{ + const Config config; + MainWindow window(config); + + QLineEdit *queryEdit = + window.findChild(QStringLiteral("queryEdit")); + QVERIFY2(queryEdit, "no query bar: the window was never built"); + + MessageView *view = window.findChild(); + QVERIFY2(view, "no message view"); + + queryEdit->setText(QStringLiteral("tag:inbox")); + emit view->searchRequested(QStringLiteral("from:\"foo@example.org\""), false); + + QCOMPARE(queryEdit->text(), QStringLiteral("from:\"foo@example.org\"")); +} + +void TestMainWindow::aSearchFromThePaneCanNarrowTheQuery() +{ + // The case the feature exists for: a query returning a thousand threads is + // narrowed by adding a condition. BOTH sides are parenthesised, because + // 'a or b AND c' binds as 'a or (b AND c)', which WIDENS a search the user + // asked to narrow, and notmuch reports no error for it. + const Config config; + MainWindow window(config); + + QLineEdit *queryEdit = + window.findChild(QStringLiteral("queryEdit")); + QVERIFY2(queryEdit, "no query bar: the window was never built"); + + MessageView *view = window.findChild(); + QVERIFY2(view, "no message view"); + + queryEdit->setText(QStringLiteral("tag:inbox or tag:flagged")); + emit view->searchRequested(QStringLiteral("from:\"foo@example.org\""), true); + + QCOMPARE(queryEdit->text(), + QStringLiteral("(tag:inbox or tag:flagged) AND (from:\"foo@example.org\")")); +} + +void TestMainWindow::narrowingAnEmptyQueryBarIsAPlainSearch() +{ + // Rather than "() AND (x)", which matches nothing. + const Config config; + MainWindow window(config); + + QLineEdit *queryEdit = + window.findChild(QStringLiteral("queryEdit")); + QVERIFY2(queryEdit, "no query bar: the window was never built"); + + MessageView *view = window.findChild(); + QVERIFY2(view, "no message view"); + + queryEdit->clear(); + emit view->searchRequested(QStringLiteral("tag:inbox"), true); + + QCOMPARE(queryEdit->text(), QStringLiteral("tag:inbox")); +} + void TestMainWindow::autoSyncIsNotArmedWhenDisabledOrWithNothingPending() { // A negative delay is the switch that restores the pre-0.16.0 behaviour, so diff --git a/tests/test_messageview.cpp b/tests/test_messageview.cpp index f83a377..edcba03 100644 --- a/tests/test_messageview.cpp +++ b/tests/test_messageview.cpp @@ -24,6 +24,7 @@ #include #include "htmlbuilder.h" +#include "messagedetailsdialog.h" #include "messageview.h" #include "mimeparser.h" @@ -53,6 +54,7 @@ private slots: void headerOffersNoSenderForARealThread(); void headerOffersNothingForAnAbsentField(); void bodySelectionBecomesAQuotedSearch(); + void aSearchFromTheDetailsDialogClosesIt(); private: QWebEngineView *webViewOf(MessageView *view) const @@ -697,5 +699,56 @@ void TestMessageView::bodySelectionBecomesAQuotedSearch() .label.isEmpty()); } +void TestMessageView::aSearchFromTheDetailsDialogClosesIt() +{ + // The dialog is modal. Without closing it, the query runs and the thread + // list repaints BEHIND a window the user still has to dismiss, so the + // search looks like it did nothing. The dialog also describes m_items, + // which the new query is about to replace. + MessageView view; + view.showThread({ oneMessage() }); + + QSignalSpy spy(&view, &MessageView::searchRequested); + QVERIFY(spy.isValid()); + + // showDetailsDialog() blocks in exec(), so the dialog has to be driven + // from a timer once it is up. + bool foundTheDialog = false; + QTimer::singleShot(0, &view, [&view, &foundTheDialog]() { + auto *dialog = view.findChild(); + if (!dialog) { + // Never leave exec() spinning: a missing dialog must fail the test, + // not hang the suite. + QApplication::exit(1); + return; + } + foundTheDialog = true; + + const QList rows = dialog->rows(); + const auto from = std::find_if( + rows.cbegin(), rows.cend(), [](const HeaderRow &row) { + return row.field == QStringLiteral("from"); + }); + if (from == rows.cend()) { + dialog->reject(); + return; + } + + dialog->requestSearch(*from, false); + }); + + view.showDetailsDialog(); + + QVERIFY2(foundTheDialog, "the details dialog never appeared"); + QCOMPARE(spy.count(), 1); + QCOMPARE(spy.at(0).at(0).toString(), + QStringLiteral("from:\"Sender \"")); + + // exec() returned, which is the assertion: the dialog closed on its own + // rather than waiting for the user to dismiss it. + QVERIFY(!view.findChild() + || !view.findChild()->isVisible()); +} + QTEST_MAIN(TestMessageView) #include "test_messageview.moc" -- cgit v1.2.3