873872 - Feed messages with image attachments and Display Inline set should show the remote image

Status: RESOLVED FIXED

(Thunderbird :: Message Reader UI, defect)

Description

[alta88]

The mime parser creates an <img> element with a src of a mailbox:// url, which doesn't apply as feed attachments are always remote links and there's nothing valid in that there part to stream.  The src should be set with the link url.

Comment 1

[alta88]

Attachment: patch

Comment 2

[alta88]

Also, I had expected Filelink image attachments to also display inline.  Is there a reason they don't (unless it's a business agreement reason).  It would be easy enough to extract a X-Mozilla-Cloud-Part url and do the same..

Comment 3

[alta88]

Attachment: updated

robustness tweak for GlodaUtils.PART_RE regex failing to take into account a ? in a url.

Comment 4

[Magnus Melin [:mkmelin]]

Do you have a sample feed url?

Comment 5

[alta88]

Since file:// urls can't be loaded in a messagepane doc, I use these; a recent guardian item had 19 media (.jpg) attachments:

http://feeds.guardian.co.uk/theguardian/sport/rss
http://news.nationalgeographic.com/index.rss

Comment 6

[alta88]

Attachment: shrinktofit update

tweak to ensure shrink to fit works nicely.

Comment 7

[Magnus Melin [:mkmelin]]

Comment on attachment 752220 [details] [diff] [review]
shrinktofit update

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

Works for me. r=mkmelin

::: mail/base/content/msgHdrViewOverlay.js
@@ +684,5 @@
>        if (!currentAttachments.length && this.mSaveHdr)
>          this.mSaveHdr.markHasAttachments(false);
> +
> +      let browser = getBrowser();
> +      let inlineAttachments = Services.prefs.getBoolPref("mail.inline_attachments");

you could just inline inlineAttachments since it's used only once

@@ +698,5 @@
> +              break;
> +            }
> +            else {
> +              // GlodaUtils.PART_RE fails to extract a partID for urls with
> +              // an embedded ?, at least.  So, check the filename too.  Frontend

please don't double space for new sentences

Comment 8

[alta88]

Attachment: for checkin

updated for comments.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/5b57f54e2cb0