From a9d1cf73a91b5eef79a9722ec9921c97b9ec5c81 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Sat, 22 Aug 2026 11:19:22 +0200 Subject: feat(compose): wire the composer into the main window, item 123 The reply family is disabled on mail that arrived at an account with no send_command, behind a ribbon in MessageView naming the account and the key to add. save_message is deliberately never disabled: it is the escape hatch for exactly that case. The ribbon is a WIDGET in the pane's layout, never markup inside the web view. Composing HTML from configuration into the one document that renders input from strangers is the wrong direction, and the header row is already a widget for the same reason. Compose itself is disabled only when NO account can send, and that state is not warned about at startup: an installation with no send_command anywhere is a valid read-only installation. Every reply resolves through messageScopeFor(), not threadFor(): a thread row means the one message its card shows. Replying to a thread is meaningless; a reply answers a message. The context is built from the DATABASE rather than the model, the rule Restore already follows, because a row whose state has not been re-queried carries stale values and a reply built from one would carry the wrong recipients. The mail root crosses from the worker as its own signal. There was no route for it at all: mailRootOf() is file-static in notmuchworker.cpp, and item 124 records that composing a destination from database.path writes into the Xapian tree under a split index. The test uses NotmuchFixture::splitIndex(), the only layout where the two accessors disagree. A thread row's path is RELATIVE to the mail root while a message row's is absolute, so the account lookup matched nothing and the reply family was dead on mail from an account that could send. Found by the positive guard test rather than the negative one, which passed throughout for the wrong reason. The quit path checks the failed-save case FIRST. In the ordinary case nothing is lost by saving; there, saving is what is already not working, so the dialog says plainly that quitting loses that text rather than offering a save that will fail again. Both dialogs name the composers, and the ordinary one asks once whatever the count, because three modals in a row is worse than a coarse answer. Its wording says drafts already saved stay in the folder, so Discard cannot read as 'delete my three messages'. The Save loop holds QPointers, not raw pointers. A deleteLater() posted while a nested exec() runs IS processed by that nested loop, measured in a standalone program: the guard nulls before the modal returns. Closing a composer while the quit dialog is up therefore freed a window the loop then called saveDraftNow() on, crashing at the exact moment the application promised to preserve that text. A compose request that matches nothing clears itself and says so. It was cleared only on a match, so a message deleted between selection and Reply left the request armed for the session: Reply did nothing, and the next ordinary click on that message opened a composer nobody asked for while the pane stayed blank. Forward carries the original's attachments, which the context has always had a field for and nothing ever filled, and seeds its HTML toggle from [compose] send_html. Only Reply seeds that from the original. save_message keeps its filename inside the chosen directory and no longer overwrites a file already there. The check was correct and untested: the test asserted through Attachment's helpers rather than through the function production calls, so deleting the containment check outright left it green. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TvwDptMWxjqhbCmjxwcSZ2 --- src/composewindow.cpp | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) (limited to 'src/composewindow.cpp') diff --git a/src/composewindow.cpp b/src/composewindow.cpp index 95b0a7b..0445c5d 100644 --- a/src/composewindow.cpp +++ b/src/composewindow.cpp @@ -18,8 +18,11 @@ #include "composewindow.h" +#include + #include "draftstore.h" #include "messagebuilder.h" +#include "mimeparser.h" #include "messagesender.h" #include "senddialog.h" @@ -124,6 +127,13 @@ ComposeWindow::ComposeWindow(const ComposeContext &context, buildFormatToolbar(); seedFields(); seedBody(); + + // AFTER buildUi(), which creates m_banner, and BEFORE + // refreshAttachmentList(), which renders m_attachments: extraction appends + // to that list, so listing first would show a Forward with no attachments + // on it, which is precisely the defect this fixes. + extractForwardedAttachments(); + refreshAttachmentList(); // Seeding is not an edit. Every field was just filled from the context, so @@ -135,6 +145,64 @@ ComposeWindow::ComposeWindow(const ComposeContext &context, m_autosaveTimer->stop(); } + +ComposeWindow::~ComposeWindow() = default; + +void ComposeWindow::extractForwardedAttachments() +{ + if (m_context.kind != ComposeContext::Kind::Forward + || m_context.originalPath.isEmpty()) { + return; + } + + MimeParser parser; + const ParsedMessage original = parser.parse(m_context.originalPath); + if (!original.ok || original.attachments.isEmpty()) + return; + + m_forwardedParts = std::make_unique(); + if (!m_forwardedParts->isValid()) { + m_forwardedParts.reset(); + m_banner->setText( + tr("The forwarded attachments could not be extracted.")); + m_banner->show(); + return; + } + + // Not auto-removed on destruction by accident: QTemporaryDir does this by + // default, and it is the whole reason the directory rather than the files + // is what this window owns. + m_forwardedParts->setAutoRemove(true); + + QStringList failed; + for (const Attachment &attachment : original.attachments) { + QString error; + // saveWithoutOverwriting, never saveTo. One message really can carry + // two parts with the same filename, and saveTo overwrites: CLAUDE.md + // records six of sixteen files lost that way, every write reporting + // success. Here it would silently forward fewer files than the + // original had. + const QString written = + attachment.saveWithoutOverwriting(m_forwardedParts->path(), &error); + if (written.isEmpty()) { + failed.append(attachment.safeFilename()); + continue; + } + m_attachments.append(written); + } + + if (!failed.isEmpty()) { + // Said out loud rather than swallowed. The composer looks entirely + // correct with an attachment missing, and the recipient gets a body + // quoting a document that is not there. + m_banner->setText( + tr("%n forwarded attachment(s) could not be extracted: %1", "", + failed.size()) + .arg(failed.join(QStringLiteral(", ")))); + m_banner->show(); + } +} + Account ComposeWindow::currentAccount() const { // The dropdown is the authority once the window is open: the context -- cgit v1.2.3