1509709 - Control showing text attachments inline in Thunderbird message display: Implement pref mail.inline_attachments.text = false

Status: RESOLVED FIXED

(MailNews Core :: MIME, enhancement)

Description

[:aceman]

If the user sets 'display attachments' inline, TB shows images and also text/* (.g. plain text, HTML, CSV) attachments inline. For images it makes sense. For text attachments it is questionable. E.g. for long text files (like logs) or CSV data dumps sent within enterprises, showing them inline only inflates the displayed message height, but the user does not want to view them immediately.

We have some possibilities here:
1. do not display attachments with text/* mimetype inline ever
2. only display text/* inline if below some file size (say 10KB).
3. have a pref whether the user wants to display those

Comment 1

[Magnus Melin [:mkmelin]]

Only 3 is probably doable

Comment 2

[Alfred Peters [:infofrommozilla]]

See also Bug 564051.
Especially Bug 564051 comment 20

Comment 3

[:aceman]

Thanks, yes it seems to be the same problem. IF the text attachment is very large it may slow down TB for something the user will surely not read inline.

Comment 4

[:aceman]

Attachment: 1509709.patch

This works for me.

Comment 5

[:aceman]

OK, test mailnews/mime/test/unit/test_message_attachment.js needs update for this, but we can test behaviour with both values of the pref now.

Comment 6

[Jorg K (CEST = GMT+2)]

Comment on attachment 9039434 [details] [diff] [review]
1509709.patch

Review of attachment 9039434 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/mime/src/mimei.cpp
@@ +828,5 @@
>     But Content-Disposition is ignored for all containers except `message'.
>     (including multipart/mixed, and multipart/digest.)  It's not clear if
>     this is to spec, but from a usability standpoint, I think it's necessary.
>     */
> +printf("ACE: 4: show=%i, quote=%i, force=%i\n", opts->show_attachment_inline_p, opts->quote_attachment_inline_p, forceInline);

Please remove debug or is this left for review purposes?

Comment 7

[:aceman]

Attachment: 1509709.patch v1.1

Sorry, leftover debugging. Now with fixed and improved tests.

So with this patch and having the "show attachments" inline option set, attachmed images will still be displayed inline in the message body, but text attachments (e.g. .txt or .csv) will not be. I have intentionally kept HTML attachments displaying as those probably aren't used as data exchange with megabyte-sized attachment.

Comment 8

[Richard Marti (:Paenglab)]

Comment on attachment 9039542 [details] [diff] [review]
1509709.patch v1.1

Maybe this could be extended to attached messages in a new bug. They could also be very long and it can irritate when the message text and the attached message text are shown together.

Comment 9

[Magnus Melin [:mkmelin]]

Comment on attachment 9039542 [details] [diff] [review]
1509709.patch v1.1

Review of attachment 9039542 [details] [diff] [review]:
-----------------------------------------------------------------

Why not make html attachments attachments too? They mess up the display of the main message pretty badly (which is another bug).

Comment 10

[:aceman]

Attachment: 1509709.patch v1.2

Sure, we can do that until HTML pages are displayed properly (e.g. confined in some iframe to not affect the msg body).

Comment 11

[Magnus Melin [:mkmelin]]

Comment on attachment 9040996 [details] [diff] [review]
1509709.patch v1.2

Review of attachment 9040996 [details] [diff] [review]:
-----------------------------------------------------------------

Yep, LGTM! r=mkmelin

::: mailnews/mailnews.js
@@ +12,5 @@
>  pref("mailnews.logComposePerformance", false);
>  
>  pref("mail.wrap_long_lines",                true);
>  pref("mail.inline_attachments",             true);
> +pref("mail.inline_attachments.text",        false);

Please add inline documentation about what it does

Comment 12

[:aceman]

Attachment: 1509709.patch v1.3

Yes, thanks.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/43c9c600cf84
add a pref to disallow showing text attachments inline. r=mkmelin

Comment 14

[gene smith]

FYI, I mention this bug here: Bug 184869 comment 16. I don't see that the fix for this bug is causing a problem. With or without this fix, the two text sub-parts of Bug 184869 don't display.