diff options
| author | Danilo M. <danix@danix.xyz> | 2026-09-18 15:25:14 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-09-18 15:25:14 +0200 |
| commit | 9d4fb5444438bdf188a6149a6667788dc3fc7bc0 (patch) | |
| tree | 10d4a63f18334ed68d65805b78e16fe1b0dfe9a5 | |
| parent | 729fd1465718ef1da5f61452bcb7eb23d02c9c9a (diff) | |
| download | qtmaildir-9d4fb5444438bdf188a6149a6667788dc3fc7bc0.tar.gz qtmaildir-9d4fb5444438bdf188a6149a6667788dc3fc7bc0.zip | |
feat: complete contacts in the composer recipients
One shared QCompleter serves To, Cc and Bcc, attached with setWidget and
never setCompleter, which resets the prefix to the whole field and stops
matching after the first comma. The prefix is the comma-delimited token
under the cursor, set by hand from textEdited; accepting replaces only
that token and leaves the rest of the field alone.
Candidates match the name and the address case-insensitively. A display
name containing a comma is quoted on insertion, and splitRecipients() is
now quote-aware so the quoted name survives as one recipient.
Contacts reach the composer through setContacts() rather than a fourth
constructor argument, so every existing three-argument construction and
test stays as it was. An empty list leaves the fields behaving exactly
as before completion existed.
| -rw-r--r-- | src/composewindow.cpp | 227 | ||||
| -rw-r--r-- | src/composewindow.h | 37 | ||||
| -rw-r--r-- | tests/test_composewindow.cpp | 256 |
3 files changed, 515 insertions, 5 deletions
diff --git a/src/composewindow.cpp b/src/composewindow.cpp index 59228ba..f62f38f 100644 --- a/src/composewindow.cpp +++ b/src/composewindow.cpp @@ -36,6 +36,7 @@ #include <QCheckBox> #include <QCloseEvent> #include <QComboBox> +#include <QCompleter> #include <QDir> #include <QFile> #include <QFileDialog> @@ -49,8 +50,10 @@ #include <QMenu> #include <QMenuBar> #include <QMessageBox> +#include <QPair> #include <QPlainTextEdit> #include <QPushButton> +#include <QStandardItemModel> #include <QStatusBar> #include <QStandardPaths> #include <QTextCursor> @@ -70,18 +73,125 @@ namespace { /// rendering of it joins with ", ". A display name containing a comma has to /// be quoted by the user, exactly as it has to be in the wire format, and /// MessageBuilder is what turns each entry into a mailbox. +/// +/// QUOTE-AWARE, and that is the half that makes the completion safe. This +/// application now inserts `"Rossi, Mario" <m@example.org>` for a contact whose +/// name carries a comma, and a naive split would cut that name in half before +/// MessageBuilder ever saw it. A `"` toggles in-quote; a `\"` inside a quoted +/// span is an escaped quote and does not close it; a comma inside quotes does +/// not split. The quotes are KEPT, because GMime's parser needs them to know +/// the comma belongs to the name. QStringList splitRecipients(const QString &text) { QStringList out; - const QStringList parts = text.split(QLatin1Char(','), Qt::SkipEmptyParts); - for (const QString &part : parts) { - const QString trimmed = part.trimmed(); - if (!trimmed.isEmpty()) - out.append(trimmed); + QString current; + bool inQuote = false; + + for (int i = 0; i < text.size(); ++i) { + const QChar c = text.at(i); + + // An escaped character inside a quoted span is literal. Both characters + // are kept so the wire format survives to MessageBuilder. + if (c == QLatin1Char('\\') && inQuote && i + 1 < text.size()) { + current.append(c); + current.append(text.at(i + 1)); + ++i; + continue; + } + + if (c == QLatin1Char('"')) { + inQuote = !inQuote; + current.append(c); + continue; + } + + if (c == QLatin1Char(',') && !inQuote) { + const QString trimmed = current.trimmed(); + if (!trimmed.isEmpty()) + out.append(trimmed); + current.clear(); + continue; + } + + current.append(c); } + + const QString trimmed = current.trimmed(); + if (!trimmed.isEmpty()) + out.append(trimmed); return out; } +/// The text a completion inserts for \p contact. +/// +/// A display name containing a comma must be QUOTED, because the recipient +/// fields are comma-separated and splitRecipients() would otherwise cut the +/// name in half before the message was built. A name containing a double quote +/// is quoted too, with its quotes backslash-escaped: an unquoted `"` in an +/// address header is malformed. The backslash is escaped FIRST, or escaping the +/// quote would then double the backslashes it just introduced. +QString contactInsertionText(const Contact &contact) +{ + if (contact.name.isEmpty()) + return contact.email; + + if (contact.name.contains(QLatin1Char(',')) + || contact.name.contains(QLatin1Char('"'))) { + QString escaped = contact.name; + escaped.replace(QLatin1Char('\\'), QStringLiteral("\\\\")); + escaped.replace(QLatin1Char('"'), QStringLiteral("\\\"")); + return QStringLiteral("\"%1\" <%2>").arg(escaped, contact.email); + } + + return QStringLiteral("%1 <%2>").arg(contact.name, contact.email); +} + +/// The span an accepted completion replaces: the comma-delimited token the +/// cursor sits in, with the whitespace around it excluded so it survives. +/// +/// A token is bounded by the last comma before the cursor and the first comma +/// after it, which is the same unit splitRecipients() will read the field back +/// as. First is the offset, second the length. +QPair<int, int> recipientTokenRange(const QString &text, int cursor) +{ + cursor = qBound(0, cursor, text.size()); + + int start = 0; + for (int i = cursor - 1; i >= 0; --i) { + if (text.at(i) == QLatin1Char(',')) { + start = i + 1; + break; + } + } + + int end = text.size(); + for (int i = cursor; i < text.size(); ++i) { + if (text.at(i) == QLatin1Char(',')) { + end = i; + break; + } + } + + while (start < end && text.at(start).isSpace()) + ++start; + while (end > start && text.at(end - 1).isSpace()) + --end; + + return { start, end - start }; +} + +/// What the completer matches on: the token's text up to the cursor. Trimmed, +/// because the leading space after a comma is not part of what was typed, and +/// QCompleter would look for it literally. +QString recipientCompletionPrefix(const QString &text, int cursor) +{ + const QPair<int, int> range = recipientTokenRange(text, cursor); + const int start = range.first; + const int end = start + range.second; + const int upTo = qBound(start, cursor, end); + return text.mid(start, upTo - start).trimmed(); +} + /// Everything about a message the user can change, as one comparable string. /// /// Joined with a character no field can contain, because concatenating them @@ -578,6 +688,113 @@ void ComposeWindow::buildUi() } applySignature(seeded); }); + + // Last, because it connects to the three fields buildUi() just created. + buildContactCompleter(); +} + +void ComposeWindow::buildContactCompleter() +{ + m_contactModel = new QStandardItemModel(this); + + m_contactCompleter = new QCompleter(m_contactModel, this); + m_contactCompleter->setCaseSensitivity(Qt::CaseInsensitive); + // MatchContains over a display string carrying BOTH name and address is the + // whole matching rule: "Ali" finds the name, "alice@" finds the address, + // and QCompleter's default prefix-on-one-string could do neither. + m_contactCompleter->setFilterMode(Qt::MatchContains); + m_contactCompleter->setCompletionMode(QCompleter::PopupCompletion); + m_contactCompleter->setCompletionColumn(0); + + // setWidget, NEVER QLineEdit::setCompleter. This is the trap CLAUDE.md + // records twice already (QueryCompleter 01ba356, TagDialog): setCompleter + // hands completion to the line edit, which then overwrites + // completionPrefix with the field's ENTIRE text on every keystroke, so + // after the first comma nothing matches and the popup stops appearing. + // setWidget still gives complete() the widget it dereferences + // unconditionally; the prefix is set by hand from textEdited instead. + for (QLineEdit *field : { m_to, m_cc, m_bcc }) { + field->installEventFilter(this); + connect(field, &QLineEdit::textEdited, this, + [this, field]() { completeRecipientToken(field); }); + } + + connect(m_contactCompleter, + QOverload<const QModelIndex &>::of(&QCompleter::activated), this, + [this](const QModelIndex &index) { + acceptContactCompletion(index); + }); +} + +void ComposeWindow::setContacts(const QList<Contact> &contacts) +{ + m_contacts = contacts; + rebuildContactModel(); +} + +void ComposeWindow::rebuildContactModel() +{ + if (!m_contactModel) + return; + + m_contactModel->clear(); + for (const Contact &contact : m_contacts) { + auto *item = new QStandardItem(contactInsertionText(contact)); + item->setEditable(false); + m_contactModel->appendRow(item); + } +} + +void ComposeWindow::completeRecipientToken(QLineEdit *field) +{ + if (!m_contactCompleter || !field) + return; + + // Re-pointed here as well as on focus: a keystroke is the signal every + // platform delivers, and complete() dereferences widget() unconditionally. + m_contactCompleter->setWidget(field); + m_contactCompleter->setCompletionPrefix( + recipientCompletionPrefix(field->text(), field->cursorPosition())); + m_contactCompleter->complete(); +} + +void ComposeWindow::acceptContactCompletion(const QModelIndex &index) +{ + auto *field = qobject_cast<QLineEdit *>(m_contactCompleter->widget()); + if (!field) + return; + + const QString value = index.data(Qt::DisplayRole).toString(); + if (value.isEmpty()) + return; + + QString text = field->text(); + const QPair<int, int> range = + recipientTokenRange(text, field->cursorPosition()); + text.replace(range.first, range.second, value); + + // setText emits textChanged, not textEdited, so this cannot re-enter the + // completion handler. The caret lands after the insertion, ready for the + // comma and the next recipient. + field->setText(text); + field->setCursorPosition(range.first + value.size()); +} + +bool ComposeWindow::eventFilter(QObject *watched, QEvent *event) +{ + // One completer serves three fields, so whichever takes focus must become + // the widget it is anchored to, or the popup opens over the wrong field and + // its keys are routed to a line edit the user has left. + if (event->type() == QEvent::FocusIn && m_contactCompleter) { + for (QLineEdit *field : { m_to, m_cc, m_bcc }) { + if (watched == field) { + m_contactCompleter->setWidget(field); + break; + } + } + } + + return QMainWindow::eventFilter(watched, event); } void ComposeWindow::buildFormatToolbar() diff --git a/src/composewindow.h b/src/composewindow.h index 09b47de..c4e5cb3 100644 --- a/src/composewindow.h +++ b/src/composewindow.h @@ -27,12 +27,17 @@ #include <memory> #include "config.h" +#include "contactstore.h" #include "formattoolbar.h" // MarkdownFormat::Edit is used by value below, and // a type nested in a namespace cannot be // forward-declared from outside it. #include "types.h" class QAction; +class QCompleter; +class QEvent; +class QModelIndex; +class QStandardItemModel; class QCheckBox; class QSplitter; class QComboBox; @@ -114,6 +119,19 @@ public: /// a second location, and the tests need to not read the real one. void setSignatureDir(const QString &dir); + /// The address book the recipient fields complete from. + /// + /// MainWindow owns the load and calls this right after constructing the + /// window, because it already holds the list for its own account picker and + /// a composer must not read the store itself. Empty (the store off, or no + /// vdir) is the ordinary state and leaves the fields behaving exactly as + /// they did before completion existed: no candidates, no popup. + /// + /// A setter rather than a constructor parameter on purpose: the three- + /// argument constructor is used by every other test, and a fourth argument + /// would turn one load into forty edits. + void setContacts(const QList<Contact> &contacts); + /// Seeds the signature from config and fills the switch. /// /// Public and called by the constructor rather than private, so a test can @@ -220,9 +238,18 @@ protected: /// The one place the registry is told, whichever route closes the window. void closeEvent(QCloseEvent *event) override; + /// Re-points the shared contact completer at whichever recipient field just + /// took focus. One completer serves To, Cc and Bcc, and QCompleter anchors + /// its popup and its key handling to a single widget. + bool eventFilter(QObject *watched, QEvent *event) override; + private: void buildUi(); void buildFormatToolbar(); + void buildContactCompleter(); + void rebuildContactModel(); + void completeRecipientToken(QLineEdit *field); + void acceptContactCompletion(const QModelIndex &index); void seedFields(); /// Extracts a forwarded message's parts into m_forwardedParts and appends @@ -325,6 +352,16 @@ private: QPlainTextEdit *m_body = nullptr; QToolButton *m_sendHtml = nullptr; + /// ONE completer over ONE model, shared by To, Cc and Bcc. Owned here and + /// parented to the window; the model's rows are the insertion strings, see + /// contactInsertionText() in the .cpp. + QCompleter *m_contactCompleter = nullptr; + QStandardItemModel *m_contactModel = nullptr; + + /// The contacts MainWindow handed in, kept so setContacts() can rebuild the + /// model without reaching back for them. + QList<Contact> m_contacts; + /// Item 171. Strips remote content from the forwarded original, checked by /// default. Only created for a Forward whose original carries remote /// content, so an ordinary message gains no control. diff --git a/tests/test_composewindow.cpp b/tests/test_composewindow.cpp index 49bb390..e2673b7 100644 --- a/tests/test_composewindow.cpp +++ b/tests/test_composewindow.cpp @@ -22,6 +22,8 @@ #include <QDir> #include <QFile> #include <QFileInfo> +#include <QLineEdit> +#include <QListView> #include <QMenu> #include <QPlainTextEdit> #include <QSignalSpy> @@ -40,8 +42,41 @@ #include "composecontext.h" #include "composewindow.h" #include "config.h" +#include "contactstore.h" #include "signatures.h" +/// The popup QCompleter shows while it is offering contacts. +/// +/// activePopupWidget, not a scan of every QListView in the process: the +/// composer already owns an attachment QListWidget, which a scan would find +/// and this must not. +static QListView *contactPopup() +{ + return qobject_cast<QListView *>(QApplication::activePopupWidget()); +} + +/// The two contacts the completion tests offer. Names and addresses differ so a +/// candidate matched by the wrong half of the string is visible in the result. +static QList<Contact> twoContacts() +{ + return { { QStringLiteral("Alice Example"), QStringLiteral("alice@example.org") }, + { QStringLiteral("Bob Example"), QStringLiteral("bob@example.org") } }; +} + +/// Accepts the top suggestion by clicking it, which is the route QCompleter +/// reports as activated() without depending on how a keyboard layout delivers +/// Return. +static void acceptFirstPopupRow() +{ + QListView *popup = contactPopup(); + QVERIFY(popup); + const QModelIndex row = popup->model()->index(0, 0); + QVERIFY(row.isValid()); + popup->setCurrentIndex(row); + QTest::mouseClick(popup->viewport(), Qt::LeftButton, Qt::NoModifier, + popup->visualRect(row).center()); +} + class TestComposeWindow : public QObject { Q_OBJECT @@ -74,6 +109,14 @@ private slots: void theHtmlMenuItemTracksTheToolbarButton(); void theAgeLineFollowsTheClock(); + void completionOffersAContactOnTheFirstRecipient(); + void completionMatchesTheAddressAsWellAsTheName(); + void completionStillWorksAfterAComma(); + void insertingACommaNameQuotesItAndKeepsOneRecipient(); + void insertingANameWithAQuoteEscapesIt(); + void anEmptyNameInsertsTheBareAddress(); + void noContactsLeavesTheBehaviourUnchanged(); + private: /// A config pointing at a signatures directory holding \p files, with one /// account that can send. @@ -1154,5 +1197,218 @@ void TestComposeWindow::theHtmlMenuItemTracksTheToolbarButton() QCOMPARE(button->isChecked(), initial); } +/// Completion exists to save typing a contact the user already has. The test +/// TYPES, never setText(): QLineEdit::setText does not drive a completer at +/// all, so a test using it passes against the very bug this task exists to +/// avoid and would endorse the mutation that reintroduces it. +void TestComposeWindow::completionOffersAContactOnTheFirstRecipient() +{ + const Config config = configWithDrafts(); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + window.setContacts(twoContacts()); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + auto *to = window.findChild<QLineEdit *>(QStringLiteral("to")); + QVERIFY(to); + to->setFocus(); + + QTest::keyClicks(to, QStringLiteral("Ali")); + QVERIFY2(contactPopup() && contactPopup()->isVisible(), + "typing a contact's name must offer it"); + QCOMPARE(contactPopup()->model()->index(0, 0).data(Qt::DisplayRole).toString(), + QStringLiteral("Alice Example <alice@example.org>")); + + acceptFirstPopupRow(); + QCOMPARE(to->text(), QStringLiteral("Alice Example <alice@example.org>")); +} + +/// The candidate is matched on the ADDRESS as well as the name. A single +/// display string carrying both is what makes that true; matching only the +/// prefix of the name would leave the address unusable as a query. +void TestComposeWindow::completionMatchesTheAddressAsWellAsTheName() +{ + const Config config = configWithDrafts(); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + window.setContacts(twoContacts()); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + auto *to = window.findChild<QLineEdit *>(QStringLiteral("to")); + QVERIFY(to); + to->setFocus(); + + // "alice@" occurs in the address and nowhere in either name. + QTest::keyClicks(to, QStringLiteral("alice@")); + QVERIFY2(contactPopup() && contactPopup()->isVisible(), + "typing an address must offer the contact"); + QCOMPARE(contactPopup()->model()->index(0, 0).data(Qt::DisplayRole).toString(), + QStringLiteral("Alice Example <alice@example.org>")); + + acceptFirstPopupRow(); + QCOMPARE(to->text(), QStringLiteral("Alice Example <alice@example.org>")); +} + +/// The case this whole task exists for: `setCompleter()` would set the +/// completion prefix to the field's ENTIRE text on every keystroke, so after +/// the first comma nothing matches and the popup never appears again. The +/// prefix must be the comma-delimited token under the cursor. +void TestComposeWindow::completionStillWorksAfterAComma() +{ + const Config config = configWithDrafts(); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + window.setContacts(twoContacts()); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + auto *to = window.findChild<QLineEdit *>(QStringLiteral("to")); + QVERIFY(to); + to->setFocus(); + + QTest::keyClicks(to, QStringLiteral("alice@example.org, bo")); + QVERIFY2(contactPopup() && contactPopup()->isVisible(), + "completion stopped after the first recipient"); + // Bob, not Alice: the prefix is the token after the comma, so Alice must + // not be offered any more. + QCOMPARE(contactPopup()->model()->index(0, 0).data(Qt::DisplayRole).toString(), + QStringLiteral("Bob Example <bob@example.org>")); + + acceptFirstPopupRow(); + QCOMPARE(to->text(), + QStringLiteral("alice@example.org, Bob Example <bob@example.org>")); +} + +/// A display name containing a comma must be QUOTED on insertion, or +/// splitRecipients() cuts it in half on the way to OutgoingMessage. The name is +/// asserted as ONE entry, which is the property the quoting buys. +void TestComposeWindow::insertingACommaNameQuotesItAndKeepsOneRecipient() +{ + const Config config = configWithDrafts(); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + window.setContacts({ { QStringLiteral("Rossi, Mario"), + QStringLiteral("mario@example.org") } }); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + auto *to = window.findChild<QLineEdit *>(QStringLiteral("to")); + QVERIFY(to); + to->setFocus(); + + QTest::keyClicks(to, QStringLiteral("Rossi")); + QVERIFY(contactPopup() && contactPopup()->isVisible()); + acceptFirstPopupRow(); + + QCOMPARE(to->text(), + QStringLiteral("\"Rossi, Mario\" <mario@example.org>")); + + // The quoted name survives splitRecipients() as a single recipient. + const OutgoingMessage message = window.currentMessage(); + QCOMPARE(message.to.size(), 1); + QCOMPARE(message.to.first(), + QStringLiteral("\"Rossi, Mario\" <mario@example.org>")); +} + +/// A double quote in a display name is backslash-escaped inside the quoted +/// string, so the header the user sees is valid. The backslash is escaped +/// first, or escaping the quotes would double the backslashes just added. +void TestComposeWindow::insertingANameWithAQuoteEscapesIt() +{ + const Config config = configWithDrafts(); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + window.setContacts({ { QStringLiteral("He said \"hi\""), + QStringLiteral("q@example.org") } }); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + auto *to = window.findChild<QLineEdit *>(QStringLiteral("to")); + QVERIFY(to); + to->setFocus(); + + QTest::keyClicks(to, QStringLiteral("He")); + QVERIFY(contactPopup() && contactPopup()->isVisible()); + acceptFirstPopupRow(); + + QCOMPARE(to->text(), + QStringLiteral("\"He said \\\"hi\\\"\" <q@example.org>")); + + const OutgoingMessage message = window.currentMessage(); + QCOMPARE(message.to.size(), 1); +} + +/// A card with an address but no name completes on the address alone, with no +/// empty angle brackets. +void TestComposeWindow::anEmptyNameInsertsTheBareAddress() +{ + const Config config = configWithDrafts(); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + window.setContacts({ { QString(), QStringLiteral("plain@example.org") } }); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + auto *to = window.findChild<QLineEdit *>(QStringLiteral("to")); + QVERIFY(to); + to->setFocus(); + + QTest::keyClicks(to, QStringLiteral("plai")); + QVERIFY(contactPopup() && contactPopup()->isVisible()); + acceptFirstPopupRow(); + + QCOMPARE(to->text(), QStringLiteral("plain@example.org")); +} + +/// The store is optional. With no contacts the fields behave exactly as they +/// did before completion existed: text goes in and no popup appears. +void TestComposeWindow::noContactsLeavesTheBehaviourUnchanged() +{ + const Config config = configWithDrafts(); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + auto *to = window.findChild<QLineEdit *>(QStringLiteral("to")); + QVERIFY(to); + to->setFocus(); + + QTest::keyClicks(to, QStringLiteral("Ali")); + QVERIFY2(!contactPopup() || !contactPopup()->isVisible(), + "a composer with no contacts must offer none"); + QCOMPARE(to->text(), QStringLiteral("Ali")); +} + QTEST_MAIN(TestComposeWindow) #include "test_composewindow.moc" |
