Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #5051 by always showing inline attachments in list of attachments #9150

Closed
wants to merge 1 commit into from

Conversation

bohwaz
Copy link

@bohwaz bohwaz commented Sep 21, 2023

When a message has attachments with a Content-Id or a Content-Location, they are added as inline, expecting the attachment to show up in the message HTML body.

--==_=_650c1f3a7e07d-b7578f07ad
Content-Type: image/png; name="2023-09-21 12_35_30-XXXX Pierre (Membres actifs) - Chromium.png"
Content-Transfer-Encoding: base64
Content-Disposition: inline; filename="2023-09-21 12_35_30-XXX Pierre (Membres actifs) - Chromium.png"
Content-ID: <f_lmt1smiu1>

But, in some cases the attachments are not referred to in the HTML body.
And when you are viewing emails as plaintext only, you still won't see the attachments.

It's weird as the message is seen as having attachments in the messages list, but no attachments show up in the message view:

image

This patch always adds the inline attachment in the list of attachements, as is the case in most email software, so that you can see and download the attachments.

@alecpl
Copy link
Member

alecpl commented Oct 8, 2023

What other email software? Thunderbird for example does not list such images as attachments when they are used in the HTML body. And I don't like a solution proposed here.

The discrepancy between list and preview is indeed an issue, but for performance reasons we don't do an extensive check on the message to find out whether there's a real attachment or not. We do a simple content-type check instead. I doubt this will change.

@bohwaz
Copy link
Author

bohwaz commented Oct 22, 2023

For now, the attachements are just hidden and the user experience is poor and confusing.

I checked with Zimbra, Claws Mail, GMail and Mutt, they all display the attachments and don't leave them hidden like Roundcube does.

@Neustradamus
Copy link

@alecpl: Any progressed on this @bohwaz PR?

Thanks in advance.

@bohwaz
Copy link
Author

bohwaz commented Dec 17, 2023

On my side, I'm using this patch in production since September with success :)

But it is annoying having to apply the patch after each update.

I'm considering pushing the patch in the Debian package if there is no progress here.

@alecpl
Copy link
Member

alecpl commented Jan 26, 2024

The proposed solution is wrong.

@alecpl alecpl closed this Jan 26, 2024
@bohwaz
Copy link
Author

bohwaz commented Jan 26, 2024

The proposed solution is wrong.

Why?

@alecpl
Copy link
Member

alecpl commented Jan 26, 2024

Because it adds all inline attachments to the attachments list.

@bohwaz
Copy link
Author

bohwaz commented Jan 26, 2024

And how is that an issue?

@alecpl
Copy link
Member

alecpl commented Jan 26, 2024

In Roundcube inline images (used in HTML content) aren't listed as attachments. It's been this way always. Precisely "Content-Disposition: inline" makes the message part a non-attachment. What to do with such parts could be discussed, but adding them to the attachments list is wrong. As I said before: Thunderbird for example does not list such images as attachments when they are used in the HTML body.

Unfortunately a better fix is not an easy fix.

@bohwaz
Copy link
Author

bohwaz commented Jan 26, 2024

Thunderbird is just one example, other MUA allow to list all attachments.

@bohwaz
Copy link
Author

bohwaz commented Jan 26, 2024

I don't see how giving users more control on the attached files is a bad thing?

bohwaz added a commit to bohwaz/roundcubemail that referenced this pull request Jan 28, 2024
* In plaintext mode: all inline images will be displayed below the message body
* In HTML mode: all inline images that are **not** referenced in the message HTML body will be listed below the message body

This doesn't add more CPU usage as it is using the already called HTML filtering function to discover if an inline part is referenced or not.

This fixes issue roundcube#5051 and is a bit better than roundcube#9150
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants