From 28e6ea321d742d26fa07d3ae6f403e6bd9baacbe Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 7 Aug 2026 10:44:55 +0200 Subject: fix(mime): a body part carrying a Content-Id no longer renders blank collectParts() filed any part with a Content-Id into inlineParts and returned before the text/plain and text/html branches. Setting a Content-Id on the text/html body is legal and common in bulk-sender output, and such a message parsed with both body slots empty, so hasHtml() was false, HtmlBuilder fell through to an empty plain body, and the pane rendered nothing. Both halves of the report, the blank message and "no HTML part", came from that one ordering. A content id makes a part referenceable, not undisplayable. The two are independent. The branch now registers the part and falls through rather than returning, so the body still fills its slot. Registering first keeps a part that is both the body and a cid: target reachable under its id for any sibling referencing it. Content-Disposition is deliberately not used as the discriminator: it is absent far more often than it is correct, and a body part commonly carries none. The existing attachment check remains the only test for "not a body", and the first-one-wins isEmpty() guard still stops an inline image displacing a real body, since an image matches neither text branch. Verified against a hand-written fixture whose text/html part carries a Content-Id, asserting the body renders, the id still resolves, and the sibling image is unaffected. Load-bearing by mutation: restoring the early return fails the test. The user could not relocate the message that prompted the report, so the end-to-end path is unconfirmed. Closes item 41. Co-Authored-By: Claude Opus 5 --- .../plans/2026-08-03-post-0.1.0-usability.md | 2 +- src/mimeparser.cpp | 9 ++++++- tests/fixtures/body_with_content_id.eml | 20 ++++++++++++++ tests/test_mimeparser.cpp | 31 ++++++++++++++++++++++ 4 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 tests/fixtures/body_with_content_id.eml 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 0091d27..d4465ce 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 @@ -88,7 +88,7 @@ taking that too literally. | 38 | `test_mainwindow` fails when a real sync holds the lock | testing | XS | **done** | | 39 | Thread list cannot be sorted by clicking a column header | workflow | S | open | | 40 | No live filter over the current view | workflow | M | open | -| 41 | A message whose HTML body carries a `Content-Id` renders blank | correctness | S | open | +| 41 | A message whose HTML body carries a `Content-Id` renders blank | correctness | S | **done** (fixture-verified; the reported message was not relocated) | | 42 | "Syncing..." says nothing about what is being synced | feedback | S | open | | 43 | No "Mark all read" for the current view | workflow | S | open | | 44 | No way to manage the filters applied at sync time | workflow | ? | open, unspecified | diff --git a/src/mimeparser.cpp b/src/mimeparser.cpp index f5ef38a..4ad617e 100644 --- a/src/mimeparser.cpp +++ b/src/mimeparser.cpp @@ -130,13 +130,20 @@ void collectParts(GMimeObject *object, ParsedMessage &out) return; } + // A content id makes a part referenceable; it does not make it + // undisplayable. The two are independent, so register it and then fall + // through to the body branches: setting a Content-Id on the text/html body + // is legal and common in bulk-sender output, and returning here left such a + // message with both body slots empty and a blank pane. + // + // Register before assigning, so a part that is both the body and a cid: + // target stays reachable under its id for any sibling referencing it. if (contentId) { // Strip the angle brackets so the key matches a cid: URL body. QString id = QString::fromUtf8(contentId); if (id.startsWith(QLatin1Char('<')) && id.endsWith(QLatin1Char('>'))) id = id.mid(1, id.size() - 2); out.inlineParts.insert(id, InlinePart{ mimeType, decodePart(part) }); - return; } if (mimeType == QLatin1String("text/plain") && out.plainBody.isEmpty()) { diff --git a/tests/fixtures/body_with_content_id.eml b/tests/fixtures/body_with_content_id.eml new file mode 100644 index 0000000..dd0f6be --- /dev/null +++ b/tests/fixtures/body_with_content_id.eml @@ -0,0 +1,20 @@ +From: Bulk Sender +Subject: Body part carrying a content id +Date: Sat, 01 Aug 2026 10:00:00 +0000 +Message-ID: +MIME-Version: 1.0 +Content-Type: multipart/related; boundary="REL" + +--REL +Content-Type: text/html; charset=utf-8 +Content-ID: + +

Body text.

+--REL +Content-Type: image/png +Content-Transfer-Encoding: base64 +Content-ID: + +iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNk+M9QDwADhgGAWjR9 +awAAAABJRU5ErkJggg== +--REL-- diff --git a/tests/test_mimeparser.cpp b/tests/test_mimeparser.cpp index f1bbc8a..d16735f 100644 --- a/tests/test_mimeparser.cpp +++ b/tests/test_mimeparser.cpp @@ -31,6 +31,7 @@ private slots: void prefersHtmlWhenAvailable(); void fallsBackToPlainWhenHtmlDisabled(); void collectsInlineCidParts(); + void aBodyCarryingAContentIdStillRenders(); void decodesQuotedPrintableAttachment(); void decodesEncodedHeaders(); void malformedMessageDoesNotCrash(); @@ -109,6 +110,36 @@ void TestMimeParser::collectsInlineCidParts() QVERIFY(part.data.startsWith(QByteArray("\x89PNG", 4))); } +void TestMimeParser::aBodyCarryingAContentIdStillRenders() +{ + // Reported by the user: a bulk sender's message opened blank, with the app + // saying it had no HTML part. + // + // A Content-Id makes a part referenceable, not non-displayable, and setting + // one on the text/html body is legal and common. collectParts filed any + // part with an id into inlineParts and returned before the body branches, + // so such a message parsed with both body slots empty. + MimeParser parser; + const ParsedMessage msg = + parser.parse(fixture(QStringLiteral("body_with_content_id.eml"))); + + QVERIFY(msg.ok); + + // The body fills its slot despite the id. + QVERIFY2(msg.hasHtml(), "the html body was swallowed by its own content id"); + QVERIFY(msg.htmlBody.contains(QStringLiteral("Body text."))); + + // And it stays reachable under that id, so a sibling referencing it still + // resolves. Register first, then assign: the two are independent. + QVERIFY(msg.inlineParts.contains(QStringLiteral("body@example.org"))); + QCOMPARE(msg.inlineParts.value(QStringLiteral("body@example.org")).mimeType, + QStringLiteral("text/html")); + + // The genuinely inline image is untouched by the change. + QVERIFY(msg.inlineParts.contains(QStringLiteral("logo@example.org"))); + QCOMPARE(msg.inlineParts.size(), 2); +} + void TestMimeParser::decodesQuotedPrintableAttachment() { MimeParser parser; -- cgit v1.2.3