diff options
| -rw-r--r-- | CHANGELOG.md | 5 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 44 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 2 | ||||
| -rw-r--r-- | src/composecontext.cpp | 63 | ||||
| -rw-r--r-- | src/composecontext.h | 13 | ||||
| -rw-r--r-- | src/composewindow.cpp | 29 | ||||
| -rw-r--r-- | src/keymap.cpp | 1 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 123 | ||||
| -rw-r--r-- | src/mainwindow.h | 13 | ||||
| -rw-r--r-- | src/mimeparser.cpp | 1 | ||||
| -rw-r--r-- | src/mimeparser.h | 7 | ||||
| -rw-r--r-- | src/types.h | 17 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 289 | ||||
| -rw-r--r-- | translations/qtmaildir_it_IT.ts | 16 |
14 files changed, 617 insertions, 6 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 19e52d8..1ee21a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,11 @@ point at which they are stable. reply follows what the message being answered used. - Drafts autosave to the account's `drafts` folder as ordinary Maildir files, so mbsync carries them to the server and another client can pick one up. +- **Drafts can be opened and finished.** Double-click one, or use Edit draft + in the Message menu. The composer takes ownership of the file, so saving + replaces the draft rather than leaving a second copy, and the Bcc list the + draft carries is read back rather than dropped. The action is offered only + on a message that really is in a drafts folder. - **A Drafts filter** in the query row, beside Sent and Trash. It matches each account's `drafts` folder, so it finds what the composer actually writes rather than trusting a flag. An account that configures no drafts folder diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md index 133a761..0f7181b 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md @@ -7348,3 +7348,47 @@ what a drafts folder is; they are separate readers and neither should start importing the other. --- + +## 153. A draft cannot be opened for editing, so it is write-only + +**Observed (user, 2026-08-24).** Found the moment item 138 gave drafts a +button: "Double clicking on a draft should open the message in the editor +window", and "there's no edit action anywhere, a draft is useless as is". + +**Cause (verified in code).** `DraftStore` had a `write()` and no reader, and +nothing anywhere opened a composer from an existing message. A draft therefore +rendered in the message pane like ordinary mail and could never be finished. + +**Outcome.** `ComposeContextBuilder::forDraft()`, a `Kind::Draft` that seeds +every field verbatim, `edit_draft` in the Message menu, and double-click. + +**Three things worth keeping.** + +**A resumed draft must own its file.** Maildir has no in-place edit, so an +autosave writes a new file and unlinks the old; a composer that did not know +its own path would leave the original behind and one message would become two. +`ComposeContext::draftPath` carries it into `m_draftPath`, which the autosave +already knew how to replace. + +**`MimeParser` had no `bcc` and nothing had ever needed one.** +`MessageBuilder` writes Bcc into the draft file deliberately and says why, so a +resumed draft that ignored it would drop every blind recipient from the message +the user then finishes and sends, silently. That is the failure this item was +most likely to ship. + +**The gate is not cosmetic.** `edit_draft` is offered only on a file inside a +configured drafts folder, matched on the PATH: a `draft` tag is not enough, +since notmuch surfaces the Maildir D flag as one and a message flagged by +another client sits in the inbox. Offered on ordinary mail, the composer would +own a file it did not write and the first autosave would DELETE a received +message. + +**And a live defect found on the way, which is the reason this took as long as +it did.** `updateComposeActions()` ran only from `onSelectionChanged`. Both +signals fire for an ordinary click, so nothing had ever noticed; but running a +query and setting the current index emits `currentRowChanged` ALONE, so the +enablement was computed against the previously selected row. Measured: Edit +draft stayed disabled on a draft selected that way, and the reply family had +the same blind spot without a test that could see it. It is now connected to +both signals. Reading `currentRowChanged` is safe here for the reason +`CLAUDE.md` gives: it answers "which row is current", and no count is read. diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md index 79320bd..e203856 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md @@ -222,7 +222,7 @@ taking that too literally. | 150 | The receive-only ribbon stays up after the message that raised it is gone | defect | S | **done** 2026-08-24, unreleased. One line in `MessageView::clear()`, beside the blocked-content bar, the stale notice and the attachment bar it already reset by hand. Only `setReceiveOnlyAccount()` hid the ribbon, which every SELECTION change reaches, so a row-to-row move was never the reproducer: it survived the FOUR routes that blank the pane without one (`clear_pane`, `clear_selection`, a new query, a multi-row selection). The first test written for it passed against the defect for exactly that reason | | 151 | The message-pane bars blend into the UI and carry no severity | presentation | S | **done** 2026-08-24, unreleased. Two severities as the user asked: yellow for a warning that only explains (the receive-only ribbon), blue for one offering an action (remote content blocked, stale thread), each with its own light and dark set read off `QPalette::Base` as `HtmlBuilder` does. The blocked row had to become a WIDGET first: it was a bare `QHBoxLayout`, which has nothing to paint a ground on, and its six `hide()` sites then had to move to the wrapper or a painted empty strip would show. Both action bars put the button right of a stretch | | 152 | Signatures are not managed at all | v2 | ? | open, 2026-08-24, from the notes, and the user added a constraint the same day: a signature is **not tied to an account**, and is switched from a control in the composer's editor bar. That rules out the obvious `[account.*] signature` key as the whole answer. Still unspecified in the rest: where the text is stored, how it interacts with the quote, and whether the HTML part gets its own form | -| 153 | A draft cannot be opened for editing, so it is write-only | defect | M | open, 2026-08-24, from the notes, found by the user the moment item 138 gave drafts a button. Verified: `DraftStore` has a `write()` and no reader, nothing anywhere calls anything like `loadDraft`, and no action opens a composer from an existing message. A draft therefore renders in the message pane like ordinary mail and can never be finished or sent. `ComposeContextBuilder` already parses a file into recipients and a body for Reply and Forward, so the parsing exists; what is missing is a context KIND that owns the original (a resumed draft must replace its file on save, not accumulate a second one) and a way in. **The two halves of the note are one item:** double-click on a draft, and an Edit action for every other route | +| 153 | A draft cannot be opened for editing, so it is write-only | defect | M | **done** 2026-08-24, unreleased. `ComposeContextBuilder::forDraft()` reads a draft back into a context; a new `Kind::Draft` seeds the fields verbatim, takes the body with no quote framing, and carries `draftPath` so the autosave REPLACES the file instead of leaving a second copy. `MimeParser` gained `bcc`, which nothing read before: `MessageBuilder` writes Bcc into the draft deliberately, so a resumed draft that ignored it would silently drop every blind recipient. Reachable by double-click and by an `edit_draft` action, gated on the file being in a configured drafts folder because opening ordinary mail this way would make the first autosave DELETE a received message. Found a live defect on the way, see the section | | 154 | No read confirmation | v2 | ? | open, 2026-08-24, from the notes. `Disposition-Notification-To`, which is a header `MessageBuilder` would add and a request the message pane would have to honour or ignore on the receiving side. Unspecified: whether this is send-side only, and what the reader is asked | | 155 | No urgency switch on an outgoing message | v2 | S | open, 2026-08-24, from the notes: low, regular, high. `X-Priority` and `Importance`, headers `MessageBuilder` adds; regular writes neither. A control in the composer, and the same question item 144 answered for the HTML toggle applies to where it sits | | 156 | No delivery confirmation | v2 | ? | open, 2026-08-24, from the notes. Distinct from 154: this is a DSN (`Return-Receipt-To`, or the ESMTP NOTIFY parameter), which is the sending server's to honour rather than the reader's client. Whether it can be requested at all depends on the `send_command`, so this may not be this application's to offer | diff --git a/src/composecontext.cpp b/src/composecontext.cpp index 251a028..d0406fc 100644 --- a/src/composecontext.cpp +++ b/src/composecontext.cpp @@ -486,6 +486,69 @@ QString ComposeContextBuilder::forwardSubject(const QString &original) return QStringLiteral("Fwd: ") + original; } +ComposeContext ComposeContextBuilder::forDraft(const Config &config, + const QString &path) +{ + ComposeContext context; + + MimeParser parser; + const ParsedMessage draft = parser.parse(path); + if (!draft.ok) + return context; // Kind::New and empty: the caller reports the failure. + + context.kind = ComposeContext::Kind::Draft; + context.originalPath = path; + // The file this composer OWNS. Without it the first autosave writes a + // second draft and leaves this one behind, so one message becomes two. + context.draftPath = path; + + const auto addresses = [](const QString &header) { + QStringList out; + for (const Recipient &recipient : parseAddressHeader(header)) + out.append(recipient.rendered); + return out; + }; + context.to = addresses(draft.to); + context.cc = addresses(draft.cc); + // Written into the draft file by MessageBuilder, which explains why. Read + // back or every blind recipient is dropped from the finished message, + // silently. + context.bcc = addresses(draft.bcc); + + // Verbatim, both of them. A draft is the message itself: its subject takes + // no Re:/Fwd: prefix and its body is not a quote. + context.subject = draft.subject; + context.body = draft.plainBody; + + context.seedHtml = draft.hasHtml(); + + // The account the draft SAYS it is from, which is the user's own earlier + // choice, rather than whichever account owns the folder the file sits in. + // + // Matched on the bare ADDRESS, never on the rendered form: a display name + // may contain an address-looking substring, which is the trap Recipient + // exists to keep apart. + const QList<Recipient> from = parseAddressHeader(draft.from); + if (!from.isEmpty()) { + const QString address = from.first().address; + for (const Account &account : config.accounts()) { + if (account.canSend() + && account.address.compare(address, Qt::CaseInsensitive) == 0) { + context.accountKey = account.key; + break; + } + } + } + // The From address names no account that can send any more, which happens + // when an account is renamed or its send_command removed between saving + // the draft and resuming it. Fall back through the ordinary chain rather + // than opening a composer that cannot send. + if (context.accountKey.isEmpty()) + context.accountKey = accountForNew(config, QString()); + + return context; +} + QString ComposeContextBuilder::quoteBody(const ParsedMessage &message) { QStringList quoted; diff --git a/src/composecontext.h b/src/composecontext.h index 4027af0..6f311c7 100644 --- a/src/composecontext.h +++ b/src/composecontext.h @@ -165,8 +165,21 @@ QString accountForNew(const Config &config, const QString &selectedAccount); /// treating it as a prefix means a genuine first reply gets no `Re:` and /// threads nowhere. See the patterns in composecontext.cpp for the measurement. QString replySubject(const QString &original); + QString forwardSubject(const QString &original); +/// Builds the context that RESUMES a draft from its file. +/// +/// Unlike a reply, nothing here is derived: the recipients, the subject and +/// the body are the draft's own, read back verbatim. The account comes from +/// the From header rather than from which account owns the file, because a +/// draft states who it is from and that is the user's own earlier choice. +/// +/// The returned context carries `draftPath`, which the composer seeds into +/// the path its autosave replaces. Returns a context whose kind is New, with +/// nothing filled in, when the file cannot be read. +ComposeContext forDraft(const Config &config, const QString &path); + /// The `>`-prefixed original, with an attribution line. /// /// Takes a ParsedMessage, NOT a MessageNode: the node carries no body and no diff --git a/src/composewindow.cpp b/src/composewindow.cpp index 5a92fa2..a64736f 100644 --- a/src/composewindow.cpp +++ b/src/composewindow.cpp @@ -587,16 +587,27 @@ void ComposeWindow::seedFields() m_to->setText(m_context.to.join(QStringLiteral(", "))); m_cc->setText(m_context.cc.join(QStringLiteral(", "))); + // Only a resumed draft carries one, and dropping it would remove every + // blind recipient from the message the user then finishes and sends. + m_bcc->setText(m_context.bcc.join(QStringLiteral(", "))); m_subject->setText(m_context.subject); + // The draft file this composer is resuming, so the next autosave REPLACES + // it rather than writing a second one beside it. + m_draftPath = m_context.draftPath; + // New and Forward seed from [compose] send_html; Reply and Reply-all seed // from whether the original carried a text/html part, ignoring the config // value. An HTML part in the original is a fact about the sender's // software, not a guess about their taste. - const bool isReply = m_context.kind == ComposeContext::Kind::Reply - || m_context.kind == ComposeContext::Kind::ReplyAll; - m_sendHtml->setChecked(isReply ? m_context.seedHtml - : m_config.compose().sendHtml); + // A DRAFT seeds from itself for the same reason a reply seeds from the + // original: the user already made this choice, and the config default is a + // guess that would silently overrule it. + const bool fromContext = m_context.kind == ComposeContext::Kind::Reply + || m_context.kind == ComposeContext::Kind::ReplyAll + || m_context.kind == ComposeContext::Kind::Draft; + m_sendHtml->setChecked(fromContext ? m_context.seedHtml + : m_config.compose().sendHtml); } void ComposeWindow::revealCcBccIfUsed() @@ -613,6 +624,16 @@ void ComposeWindow::revealCcBccIfUsed() void ComposeWindow::seedBody() { + // A resumed draft is the message ITSELF, so it goes in exactly as it was + // left: no attribution, no quote markers, no blank lines added and no + // cursor moved to make room for a reply that is already written. + if (m_context.kind == ComposeContext::Kind::Draft) { + m_body->setPlainText(m_context.body); + m_body->moveCursor(QTextCursor::End); + m_body->document()->clearUndoRedoStacks(); + return; + } + if (m_context.quotedBody.isEmpty()) return; diff --git a/src/keymap.cpp b/src/keymap.cpp index 0df8450..6cd965a 100644 --- a/src/keymap.cpp +++ b/src/keymap.cpp @@ -63,6 +63,7 @@ QStringList KeyMap::knownActions() QStringLiteral("reply_all"), QStringLiteral("reply_no_quote"), QStringLiteral("forward"), + QStringLiteral("edit_draft"), QStringLiteral("save_message"), QStringLiteral("focus_query"), QStringLiteral("complete_query"), diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index da18869..eb41da9 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -875,6 +875,19 @@ void MainWindow::buildUi() &QItemSelectionModel::currentRowChanged, this, &MainWindow::onThreadSelected); + // Also on currentRowChanged, and NOT only from onSelectionChanged, which + // is where the reply family is answered. Both signals fire for an ordinary + // click, but a selection that does not CHANGE emits only this one: running + // a query and setting the current index reaches here and never the other, + // so the enablement was computed against the previously selected row. + // Measured: Edit draft stayed disabled on a draft selected that way. + // + // Safe on currentRowChanged, which CLAUDE.md restricts to "which row is + // current": that is exactly the question here, and no count is read. + connect(m_threadView->selectionModel(), + &QItemSelectionModel::currentRowChanged, this, + [this]() { updateComposeActions(); }); + // Separate from currentRowChanged: a selection can grow without current // moving at all. Ctrl+click adds a row and leaves current where it was, and // selectAll() emits no currentRowChanged whatsoever (verified against @@ -970,6 +983,28 @@ void MainWindow::composeReply(ComposeContext::Kind kind, bool quote) requestMessageForCompose(scope.messageIds.first(), kind, quote); } +void MainWindow::editDraft() +{ + editDraftAt(m_threadView->currentIndex()); +} + +void MainWindow::editDraftAt(const QModelIndex &index) +{ + // messageScopeFor(), like composeReply(): a thread row means the one + // message its card shows. + const ActionScope scope = m_model->messageScopeFor({ index }); + if (scope.messageIds.isEmpty()) { + showTransientStatus(tr("No message is selected")); + return; + } + + // Through the worker for its path, never from the model: the model's path + // comes from the query, and a draft is rewritten by every autosave, so a + // row that has not been re-queried names a file that no longer exists. + requestMessageForCompose(scope.messageIds.first(), + ComposeContext::Kind::Draft, false); +} + void MainWindow::requestMessageForCompose(const QString &messageId, ComposeContext::Kind kind, bool quote) @@ -990,6 +1025,24 @@ void MainWindow::requestMessageForCompose(const QString &messageId, void MainWindow::openComposerFor(const MessageRef &ref, ComposeContext::Kind kind, bool quote) { + // A draft is RESUMED rather than answered: nothing is derived from it, + // and the composer takes ownership of its file. Handled before the reply + // machinery below, none of which applies (item 153). + if (kind == ComposeContext::Kind::Draft) { + const ComposeContext draft = + ComposeContextBuilder::forDraft(m_config, ref.filePath); + if (draft.kind != ComposeContext::Kind::Draft) { + showTransientStatus(tr("That draft could not be read")); + return; + } + if (draft.accountKey.isEmpty()) { + showTransientStatus(tr("No account is configured to send")); + return; + } + openComposer(draft); + return; + } + MimeParser parser; const ParsedMessage original = parser.parse(ref.filePath); if (!original.ok) { @@ -1155,6 +1208,50 @@ void MainWindow::markComposersDirtyForTest() } } +bool MainWindow::currentMessageIsADraft() const +{ + return indexIsADraft(m_threadView->currentIndex()); +} + +bool MainWindow::indexIsADraft(const QModelIndex ¤t) const +{ + if (m_mailRoot.isEmpty()) + return false; + + if (!current.isValid()) + return false; + + QString path; + if (m_model->isMessageRow(current)) + path = m_model->messageAt(current).filePath; + else + path = m_model->threadFor(current).firstMessagePath; + if (path.isEmpty()) + return false; + + // ThreadSummary::firstMessagePath is RELATIVE to the mail root and + // MessageNode::filePath is ABSOLUTE, the asymmetry accountForCurrentMessage() + // documents. Compared as a resolved absolute path against each account's + // drafts folder. + const QString absolute = QDir::isAbsolutePath(path) + ? path + : QDir(m_mailRoot).absoluteFilePath(path); + + for (const Account &account : m_config.accounts()) { + if (account.drafts.isEmpty()) + continue; + const QString folder = QDir(m_mailRoot).absoluteFilePath( + account.maildir + QLatin1Char('/') + account.drafts); + // A path comparison with a separator, never startsWith() on the bare + // folder: "/mail/acct/Drafts-old/cur/x" starts with "/mail/acct/Drafts" + // and is a different folder. This is the rule the attachment save path + // already follows. + if (absolute.startsWith(folder + QLatin1Char('/'))) + return true; + } + return false; +} + QString MainWindow::accountForCurrentMessage() const { if (m_mailRoot.isEmpty()) @@ -1214,6 +1311,17 @@ void MainWindow::updateComposeActions() action->setEnabled(canReply); } + // Edit draft is offered only on a row that IS a draft. On ordinary mail + // it would open a composer owning a file it did not write, and the first + // autosave replaces that file: editing a received message would delete it. + // + // Answered from the path, like accountForCurrentMessage() above, because + // the folder is what makes a draft a draft. A `draft` tag is not enough: + // notmuch surfaces the Maildir D flag as one, and a message flagged by + // another client sits in the inbox rather than in the drafts folder. + if (QAction *edit = m_actions.value(QStringLiteral("edit_draft"))) + edit->setEnabled(currentMessageIsADraft()); + // The ribbon appears only when an account was identified AND it cannot // send. An unidentified account is not a receive-only one: it is a message // whose file no account owns, and naming no account in a ribbon that @@ -1717,6 +1825,9 @@ void MainWindow::registerActions() addAction(QStringLiteral("forward"), tr("&Forward"), tr("Forward the displayed message"), [this]() { composeReply(ComposeContext::Kind::Forward, true); }); + addAction(QStringLiteral("edit_draft"), tr("&Edit draft"), + tr("Open the selected draft in a composer to finish it"), + [this]() { editDraft(); }); addAction(QStringLiteral("save_message"), tr("Sa&ve message as..."), tr("Write the raw message to a file"), [this]() { saveDisplayedMessage(); }); @@ -1764,6 +1875,7 @@ void MainWindow::buildMenus() messageMenu->addAction(m_actions.value(QStringLiteral("reply_all"))); messageMenu->addAction(m_actions.value(QStringLiteral("reply_no_quote"))); messageMenu->addAction(m_actions.value(QStringLiteral("forward"))); + messageMenu->addAction(m_actions.value(QStringLiteral("edit_draft"))); messageMenu->addSeparator(); messageMenu->addAction(m_actions.value(QStringLiteral("save_message"))); messageMenu->addSeparator(); @@ -1875,6 +1987,7 @@ void MainWindow::buildMenus() { QStringLiteral("select_all"), QStringLiteral("edit-select-all") }, { QStringLiteral("clear_pane"), QStringLiteral("edit-clear") }, { QStringLiteral("clear_selection"), QStringLiteral("edit-clear-all") }, + { QStringLiteral("edit_draft"), QStringLiteral("document-edit") }, { QStringLiteral("toggle_html"), QStringLiteral("text-html") }, { QStringLiteral("load_remote"), QStringLiteral("image-loading") }, { QStringLiteral("message_details"), QStringLiteral("dialog-information") }, @@ -4111,6 +4224,16 @@ void MainWindow::onRowDoubleClicked(const QModelIndex &index) if (!index.isValid()) return; + // A draft opens in the COMPOSER, not in a thread view of itself: it is an + // unfinished message, and looking at one rendered is not what the gesture + // means (item 153). Checked before the thread route below, which is what + // every other row does. + // The CLICKED row, which is not necessarily the current one. + if (indexIsADraft(index)) { + editDraftAt(index); + return; + } + // The whole thread in every case, and the double-clicked row's own message // in the pane. A reply therefore drills to its THREAD with itself selected, // never to itself alone: "double click on a reply in a thread should still diff --git a/src/mainwindow.h b/src/mainwindow.h index 909b04b..4b1ef8f 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -722,6 +722,19 @@ private: /// model's paths and tags come from the query, so a row that has not been /// re-queried carries stale values and a reply built from one would go to /// the wrong recipients. + /// Opens the selected draft in a composer to finish it (item 153). + /// Whether the current row's file sits in a configured drafts folder. + bool currentMessageIsADraft() const; + + /// Whether \p index names a file in a configured drafts folder. + bool indexIsADraft(const QModelIndex &index) const; + + void editDraft(); + + /// Opens the draft named by \p index, which double-click uses: + /// the clicked row is not necessarily the current one. + void editDraftAt(const QModelIndex &index); + void requestMessageForCompose(const QString &messageId, ComposeContext::Kind kind, bool quote); diff --git a/src/mimeparser.cpp b/src/mimeparser.cpp index c1198b8..6a7c5b1 100644 --- a/src/mimeparser.cpp +++ b/src/mimeparser.cpp @@ -415,6 +415,7 @@ ParsedMessage MimeParser::parse(const QString &filePath) const out.replyTo = headerText(message, "Reply-To"); out.to = headerText(message, "To"); out.cc = headerText(message, "Cc"); + out.bcc = headerText(message, "Bcc"); out.date = headerText(message, "Date"); out.references = headerText(message, "References"); out.messageId = QString::fromUtf8( diff --git a/src/mimeparser.h b/src/mimeparser.h index 64c4585..536b781 100644 --- a/src/mimeparser.h +++ b/src/mimeparser.h @@ -130,6 +130,13 @@ struct ParsedMessage QString to; QString cc; + + /// Present only on a message this application wrote: a DRAFT or a sent + /// copy carries its Bcc list in the file (see messagebuilder.cpp, which + /// explains why), and a resumed draft has to read it back or a blind + /// recipient is silently dropped from the message the user finishes. + /// Received mail does not carry one. + QString bcc; QString date; QString messageId; diff --git a/src/types.h b/src/types.h index 99c271d..7464586 100644 --- a/src/types.h +++ b/src/types.h @@ -247,7 +247,7 @@ struct DatabaseStats /// recipients. This is the same rule Restore already follows. struct ComposeContext { - enum class Kind { New, Reply, ReplyAll, Forward }; + enum class Kind { New, Reply, ReplyAll, Forward, Draft }; QString accountKey; ///< Which account sends. Plain data here; the resolution rules live with whatever builds this context. Kind kind = Kind::New; @@ -256,8 +256,23 @@ struct ComposeContext QStringList references; ///< The original's References plus its Message-ID. QStringList to; ///< Pre-filled, the user's own addresses already stripped. QStringList cc; + QStringList bcc; ///< Only a resumed draft has one; see MimeParser::bcc. QString subject; ///< Re:/Fwd: prefixed, an existing prefix not doubled. QString quotedBody; ///< The >-prefixed original. Empty when the action does not quote. + + /// The body as the user last left it, for a resumed draft ONLY. + /// + /// Separate from quotedBody because it is not a quote and must not be + /// framed like one: no attribution, no blank lines added, no cursor moved + /// to make room. It is the message itself. + QString body; + + /// The draft file this composer OWNS, empty for every other kind. + /// + /// Seeded into ComposeWindow::m_draftPath so the next autosave REPLACES + /// the file rather than leaving the original beside it. Without it a + /// resumed draft becomes two drafts on the first autosave. + QString draftPath; bool seedHtml = false; ///< Did the original carry a text/html part. QStringList attachments; ///< Carried forward for Forward, empty otherwise. }; diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 3dbb227..a6aa7f5 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -53,6 +53,9 @@ #include "carddelegate.h" #include "composewindow.h" #include "senddialog.h" +#include "composecontext.h" +#include "messagebuilder.h" +#include "draftstore.h" #include "messagesender.h" #include <QCheckBox> #include <QPlainTextEdit> @@ -118,6 +121,9 @@ public: QString trash; QString sendCommand; QString address; + /// Written only when non-empty, like trash: an account without one + /// offers no Drafts filter and no Edit draft (items 138 and 153). + QString drafts; }; /// Writes several accounts, for the compose cases. @@ -194,6 +200,8 @@ public: // send_command is receive-only, which is the shape under test. if (!account.sendCommand.isEmpty()) out << "send_command=" << account.sendCommand << "\n"; + if (!account.drafts.isEmpty()) + out << "drafts=" << account.drafts << "\n"; } } file.close(); @@ -478,6 +486,11 @@ private slots: // fixture. void aComposerOpensClean(); void ctrlWClosesTheComposer(); + void aDraftReopensWithItsOwnContent(); + void editDraftIsOfferedOnlyForADraft(); + void doubleClickingADraftOpensTheComposer(); + void aResumedDraftReplacesItsFileRatherThanAddingOne(); + void aResumedDraftKeepsItsBlindRecipients(); void theComposerSplitsItsToolbarByScope(); void ccAndBccHideBehindADisclosure(); void ccAndBccAreRevealedWhenTheyCarryAValue(); @@ -12449,6 +12462,282 @@ void TestMainWindow::removeAttachmentAppearsOnlyWithAttachments() "Remove attachment is not offered with a file attached"); } +namespace { + +/// Writes a draft the way ComposeWindow's autosave does, and returns its path. +/// +/// Built through MessageBuilder rather than by hand, so the test resumes the +/// bytes the application really writes: a draft assembled from a string +/// literal could disagree with the builder and the round trip would prove +/// nothing about the real file. +QString writeDraftFile(const QString &folder, const OutgoingMessage &message, + const Account &account) +{ + QDir().mkpath(folder + QStringLiteral("/cur")); + QDir().mkpath(folder + QStringLiteral("/new")); + QDir().mkpath(folder + QStringLiteral("/tmp")); + const MessageBuilder::Result built = MessageBuilder::build(message, account); + if (!built.ok()) { + qWarning("draft fixture: build failed: %s", qPrintable(built.error)); + return {}; + } + const DraftStore::Result written = + DraftStore::write(folder, built.bytes, QStringLiteral("D")); + if (!written.ok()) + qWarning("draft fixture: write failed: %s", qPrintable(written.error)); + return written.path; +} + +} // namespace + +void TestMainWindow::doubleClickingADraftOpensTheComposer() +{ + // The user's own words: "Double clicking on a draft should open the + // message in the editor window." Every other row opens its thread, which + // for an unfinished message means looking at it rendered and being unable + // to touch it. + WorkerComposeFixture fixture; + QVERIFY(fixture.backed.fixture().addMessage( + QStringLiteral("acct/Drafts"), QStringLiteral("draft2@example.org"), + QStringLiteral("Unfinished"), QStringLiteral("you@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Half a thought."))); + QVERIFY2(fixture.seed({ { QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash"), + QStringLiteral("/bin/true"), + QStringLiteral("you@example.org"), + QStringLiteral("Drafts") } }, + QStringLiteral("acct/inbox")), + qPrintable(fixture.backed.error())); + + MainWindow window(fixture.backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + queryEdit->setText(QStringLiteral("id:draft2@example.org")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1 + && !window.mailRootForTesting().isEmpty(), + 15000); + + const QModelIndex row = model->index(0, 0, QModelIndex()); + view->setCurrentIndex(row); + + const int before = window.openComposerCount(); + emit view->doubleClicked(row); + + // Through the worker, so the composer arrives on a later turn. + QTRY_VERIFY_WITH_TIMEOUT( + window.openComposerCount() == before + 1, 15000); +} + +void TestMainWindow::editDraftIsOfferedOnlyForADraft() +{ + // A draft renders like any other message, so the action has to say which + // rows it applies to. Offered on ordinary mail it would open a composer + // that owns a file it did not write, and the first autosave would then + // delete a received message. + WorkerComposeFixture fixture; + // The draft goes in BEFORE the window opens: the worker holds the database + // open, so a message added afterwards is not in the index it queries. + QVERIFY(fixture.backed.fixture().addMessage( + QStringLiteral("acct/Drafts"), QStringLiteral("draft1@example.org"), + QStringLiteral("Half written"), QStringLiteral("you@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body."))); + QVERIFY2(fixture.seed({ { QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash"), + QStringLiteral("/bin/true"), + QStringLiteral("you@example.org"), + QStringLiteral("Drafts") } }, + QStringLiteral("acct/inbox")), + qPrintable(fixture.backed.error())); + + MainWindow window(fixture.backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + auto *edit = window.findChild<QAction *>(QStringLiteral("edit_draft")); + QVERIFY(model && view && queryEdit && edit); + + // Selected by id, not through selectTheMessage(): there are two messages + // here, and this test is about which FOLDER each sits in. + const auto selectById = [&](const QString &id) { + queryEdit->setText(QStringLiteral("id:") + id); + queryEdit->returnPressed(); + bool ready = false; + for (int attempt = 0; attempt < 150 && !ready; ++attempt) { + ready = model->rowCount(QModelIndex()) == 1 + && !window.mailRootForTesting().isEmpty(); + if (!ready) + QTest::qWait(100); + } + if (!ready) + return false; + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + return true; + }; + + QVERIFY2(selectById(QStringLiteral("compose1@example.org")), + "the inbox message was not found"); + QVERIFY2(!edit->isEnabled(), + "Edit draft is offered on a message in the inbox"); + + // The guard, and it is the half that matters: an action disabled + // everywhere passes the assertion above while the feature does not exist. + QVERIFY2(selectById(QStringLiteral("draft1@example.org")), + "the draft was not found"); + QVERIFY2(edit->isEnabled(), + "Edit draft is not offered on a message in the drafts folder"); +} + +void TestMainWindow::aDraftReopensWithItsOwnContent() +{ + // Item 153. A draft was write-only: DraftStore had a write() and no + // reader, and nothing opened a composer from an existing message, so a + // draft rendered as ordinary mail and could never be finished. + ComposeFixture fixture; + QVERIFY(fixture.build()); + + OutgoingMessage message; + message.accountKey = QStringLiteral("acct"); + message.to = { QStringLiteral("someone@example.org") }; + message.cc = { QStringLiteral("copied@example.org") }; + message.subject = QStringLiteral("A half-written note"); + message.markdownBody = QStringLiteral("The first half.\n\nAnd more."); + + const QString folder = fixture.mailRoot() + QStringLiteral("/acct/Drafts"); + const QString path = writeDraftFile(folder, message, + fixture.config().account( + QStringLiteral("acct"))); + QVERIFY2(!path.isEmpty(), "the draft fixture was not written"); + + ComposeContext context = + ComposeContextBuilder::forDraft(fixture.config(), path); + QVERIFY2(context.kind == ComposeContext::Kind::Draft, + "forDraft did not produce a Draft context"); + + ComposeWindow window(context, fixture.config(), fixture.mailRoot()); + auto *to = window.findChild<QLineEdit *>(QStringLiteral("to")); + auto *cc = window.findChild<QLineEdit *>(QStringLiteral("cc")); + auto *subject = window.findChild<QLineEdit *>(QStringLiteral("subject")); + auto *body = window.findChild<QPlainTextEdit *>(QStringLiteral("body")); + QVERIFY(to && cc && subject && body); + + QVERIFY2(to->text().contains(QStringLiteral("someone@example.org")), + qPrintable(QStringLiteral("To reads '%1'").arg(to->text()))); + QVERIFY2(cc->text().contains(QStringLiteral("copied@example.org")), + qPrintable(QStringLiteral("Cc reads '%1'").arg(cc->text()))); + QCOMPARE(subject->text(), QStringLiteral("A half-written note")); + + // The body VERBATIM: no attribution, no quote markers, and no blank lines + // added. A draft is the message itself, not something being answered, so + // seedBody()'s quote framing must not touch it. + QVERIFY2(body->toPlainText().contains(QStringLiteral("The first half.")), + qPrintable(QStringLiteral("the body reads '%1'") + .arg(body->toPlainText()))); + QVERIFY2(!body->toPlainText().contains(QStringLiteral("wrote:")), + "the draft body was framed as a quote"); + QVERIFY2(!body->toPlainText().startsWith(QLatin1Char('>')), + "the draft body was quote-marked"); + QVERIFY2(!body->toPlainText().startsWith(QStringLiteral("\n\n")), + "blank lines were prepended to a draft, as if it were a reply"); +} + +void TestMainWindow::aResumedDraftReplacesItsFileRatherThanAddingOne() +{ + // The half that makes resuming safe rather than merely possible. Maildir + // has no in-place edit, so an autosave writes a new file and unlinks the + // old one; a resumed draft that did not know its own path would leave the + // original behind and the user would have two drafts of one message. + ComposeFixture fixture; + QVERIFY(fixture.build()); + + OutgoingMessage message; + message.accountKey = QStringLiteral("acct"); + message.to = { QStringLiteral("someone@example.org") }; + message.subject = QStringLiteral("Resumed"); + message.markdownBody = QStringLiteral("Body."); + + const QString folder = fixture.mailRoot() + QStringLiteral("/acct/Drafts"); + const QString path = writeDraftFile(folder, message, + fixture.config().account( + QStringLiteral("acct"))); + QVERIFY(!path.isEmpty()); + + const ComposeContext context = + ComposeContextBuilder::forDraft(fixture.config(), path); + QCOMPARE(context.draftPath, path); + + ComposeWindow window(context, fixture.config(), fixture.mailRoot()); + auto *body = window.findChild<QPlainTextEdit *>(QStringLiteral("body")); + QVERIFY(body); + + const auto draftCount = [&folder]() { + return QDir(folder + QStringLiteral("/cur")) + .entryList(QDir::Files).size(); + }; + QCOMPARE(draftCount(), 1); + + body->setPlainText(QStringLiteral("Body, continued.")); + + // Through the real timer, which is what production uses: the edit above + // starts it, and firing it here runs the same autosave() a pause would. + auto *timer = window.findChild<QTimer *>(QStringLiteral("autosave")); + QVERIFY2(timer, "the composer has no autosave timer"); + QVERIFY2(timer->isActive(), + "editing the body did not arm the autosave timer"); + timer->setInterval(0); + QTRY_VERIFY_WITH_TIMEOUT(!timer->isActive(), 5000); + + QCOMPARE(draftCount(), 1); + QVERIFY2(!QFile::exists(path), + "the original draft file survived the autosave, so the message " + "now exists twice"); +} + +void TestMainWindow::aResumedDraftKeepsItsBlindRecipients() +{ + // MessageBuilder writes Bcc into the draft file deliberately, and says + // why. A resumed draft that did not read it back would drop every blind + // recipient silently: the user finishes the message, sends it, and the + // people they addressed blindly never receive it and nothing reports so. + ComposeFixture fixture; + QVERIFY(fixture.build()); + + OutgoingMessage message; + message.accountKey = QStringLiteral("acct"); + message.to = { QStringLiteral("someone@example.org") }; + message.bcc = { QStringLiteral("blind@example.org") }; + message.subject = QStringLiteral("With a blind copy"); + message.markdownBody = QStringLiteral("Body."); + + const QString folder = fixture.mailRoot() + QStringLiteral("/acct/Drafts"); + const QString path = writeDraftFile(folder, message, + fixture.config().account( + QStringLiteral("acct"))); + QVERIFY(!path.isEmpty()); + + const ComposeContext context = + ComposeContextBuilder::forDraft(fixture.config(), path); + ComposeWindow window(context, fixture.config(), fixture.mailRoot()); + + auto *bcc = window.findChild<QLineEdit *>(QStringLiteral("bcc")); + QVERIFY(bcc); + QVERIFY2(bcc->text().contains(QStringLiteral("blind@example.org")), + qPrintable(QStringLiteral("Bcc reads '%1', so a blind recipient " + "was dropped").arg(bcc->text()))); + + // And it is VISIBLE, per item 145: a hidden field holding an address is a + // message going somewhere the sender cannot see. + QVERIFY2(!bcc->isHidden(), + "the resumed draft hid a Bcc it actually carries"); +} + void TestMainWindow::ctrlWClosesTheComposer() { // Item 148. Ctrl+W closes a window in every application the user runs, and diff --git a/translations/qtmaildir_it_IT.ts b/translations/qtmaildir_it_IT.ts index 9a8f7cf..f34b493 100644 --- a/translations/qtmaildir_it_IT.ts +++ b/translations/qtmaildir_it_IT.ts @@ -620,6 +620,14 @@ Il messaggio È stato inviato. Non inviarlo di nuovo.</translation> <translation>Nessun messaggio selezionato</translation> </message> <message> + <source>That draft could not be read</source> + <translation>Non è stato possibile leggere quella bozza</translation> + </message> + <message> + <source>No account is configured to send</source> + <translation>Nessun account è configurato per inviare</translation> + </message> + <message> <source>That message could not be read</source> <translation>Impossibile leggere quel messaggio</translation> </message> @@ -848,6 +856,14 @@ Il messaggio È stato inviato. Non inviarlo di nuovo.</translation> <translation>Inoltra il messaggio visualizzato</translation> </message> <message> + <source>&Edit draft</source> + <translation>&Modifica bozza</translation> + </message> + <message> + <source>Open the selected draft in a composer to finish it</source> + <translation>Apri la bozza selezionata in un editor per completarla</translation> + </message> + <message> <source>Write the raw message to a file</source> <translation>Scrive il messaggio grezzo su un file</translation> </message> |