summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/composewindow.cpp227
-rw-r--r--src/composewindow.h37
-rw-r--r--tests/test_composewindow.cpp256
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"