fix(reader): render inline cid: images in HTML emails
Inline images in rich HTML emails (embedded via Content-ID and <img src="cid:...">, e.g. USPS Informed Delivery digests) were listed under Attachments with a download button and never rendered in the body. Two bugs combined; both are fixed here. 1. Misclassification: ImapClient classified any part with a filename as an attachment, sweeping inline images (which carry a filename AND a Content-ID under Content-Disposition: inline) into the list. A part is now a downloadable attachment only when its disposition is attachment, or it has a filename but no Content-ID; an inline image is collected separately and excluded from the displayed list (AttachmentDao filters contentId IS NULL). The Content-ID is read via MimePart.getContentID() so it resolves from IMAP BODYSTRUCTURE rather than a per-part header fetch that Angus leaves unpopulated. 2. No rendering path: HtmlBody's WebViewClient now overrides shouldInterceptRequest to resolve cid:<id> to the matching part's bytes (backing the CSP's existing cid: allowance). Content-ID is threaded end-to-end through AttachmentPart, Attachment, AttachmentEntity, and MailRepository.inlineImages(); ReaderViewModel surfaces the cid->bytes map to the WebView. Schema: adds attachments.contentId (v16 -> v17, MIGRATION_16_17). Tests: MIME-part classification (inline+cid excluded, real/disposition/ filename-only kept), a GreenMail multipart/related round-trip, the cid->bytes resolver, repository inlineImages(), the DAO display filter, and the v16->v17 migration. Closes #133 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -96,6 +96,25 @@ class LibreMailDatabaseTest {
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun observeForMessageHidesInlineImagesWhileGetForMessageKeepsThem() = runBlocking {
|
||||
val messageDao = db.messageDao()
|
||||
val attachmentDao = db.attachmentDao()
|
||||
messageDao.insertNew(listOf(message("acct:1")))
|
||||
attachmentDao.insert(
|
||||
listOf(
|
||||
AttachmentEntity("acct:1", 0, "logo.png", "image/png", 4, contentId = "logo1"),
|
||||
AttachmentEntity("acct:1", 1, "invoice.pdf", "application/pdf", 10, contentId = null),
|
||||
),
|
||||
)
|
||||
|
||||
// The displayed list excludes inline cid: images (issue #133) ...
|
||||
val displayed = attachmentDao.observeForMessage("acct:1").first()
|
||||
assertEquals(listOf("invoice.pdf"), displayed.map { it.filename })
|
||||
// ... while the full read keeps them so their bytes can back a cid: request.
|
||||
assertEquals(2, attachmentDao.getForMessage("acct:1").size)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun searchRowsAreNotInboxAndAreCleared() = runBlocking {
|
||||
val messageDao = db.messageDao()
|
||||
|
||||
@@ -169,6 +169,37 @@ class MigrationTest {
|
||||
db.close()
|
||||
}
|
||||
|
||||
/** v16 -> v17 (issue #133): `attachments.contentId` appears defaulting to NULL; cached rows survive. */
|
||||
@Test
|
||||
fun migrate16To17_addsNullContentIdToAttachments() {
|
||||
helper.createDatabase(TEST_DB, 16).apply {
|
||||
// v16 dropped the account tables, so a message (no FK to accounts) plus its attachment is
|
||||
// all that's needed to exercise the attachments table rebuild.
|
||||
execSQL(
|
||||
"INSERT INTO messages (id, accountId, sender, senderEmail, subject, snippet, body, isHtml, " +
|
||||
"timestampMillis, isRead, isStarred, folder, inInbox, bodyFetched, uid) VALUES " +
|
||||
"('acct:INBOX:1', 'acct', 'Ada', 'ada@example.org', 'Hi', '', '', 0, 1000, 0, 0, " +
|
||||
"'INBOX', 1, 1, 1)",
|
||||
)
|
||||
execSQL(
|
||||
"INSERT INTO attachments (messageId, partIndex, filename, mimeType, sizeBytes) " +
|
||||
"VALUES ('acct:INBOX:1', 0, 'report.pdf', 'application/pdf', 2048)",
|
||||
)
|
||||
close()
|
||||
}
|
||||
|
||||
val db = helper.runMigrationsAndValidate(TEST_DB, 17, true, MIGRATION_16_17)
|
||||
|
||||
db.query("SELECT filename, contentId FROM attachments WHERE messageId = 'acct:INBOX:1'").use { c ->
|
||||
assertTrue("the pre-upgrade attachment row must survive", c.moveToFirst())
|
||||
assertEquals("report.pdf", c.getString(0))
|
||||
assertTrue("existing attachments read a null contentId (treated as ordinary downloads)", c.isNull(1))
|
||||
assertFalse("only the one pre-upgrade attachment row must survive", c.moveToNext())
|
||||
}
|
||||
assertEquals("the cached message must be untouched by 16->17", 1, db.count("messages"))
|
||||
db.close()
|
||||
}
|
||||
|
||||
/** The newest schema JSON exported to app/schemas (shipped to the test APK as assets). */
|
||||
private fun latestExportedSchemaVersion(): Int {
|
||||
val schemaFolder = checkNotNull(LibreMailDatabase::class.java.canonicalName)
|
||||
|
||||
@@ -11,6 +11,7 @@ import org.libremail.domain.model.Attachment
|
||||
import org.libremail.domain.model.Draft
|
||||
import org.libremail.domain.model.Folder
|
||||
import org.libremail.domain.model.ImapConnectionParams
|
||||
import org.libremail.domain.model.InlineImage
|
||||
import org.libremail.domain.model.Message
|
||||
import org.libremail.domain.model.OutboxMessage
|
||||
import org.libremail.domain.model.OutgoingMessage
|
||||
@@ -120,6 +121,8 @@ class FakeMailRepository(
|
||||
},
|
||||
)
|
||||
|
||||
override suspend fun inlineImages(messageId: String): List<InlineImage> = emptyList()
|
||||
|
||||
override suspend fun downloadAttachment(messageId: String, partIndex: Int): Result<File> =
|
||||
Result.failure(UnsupportedOperationException("not used in UI tests"))
|
||||
|
||||
|
||||
Reference in New Issue
Block a user