1322861 - Link content gets downloaded and attached to message when mail.compose.attachHttp is true

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Jorg K (CEST = GMT+2)]

I have a signature like so:
<a href="https://wiki.mozilla.org/Modules/Thunderbird#Thunderbird_Council">Thunderbird Council</a>
<img moz-do-not-send="true" src="http://www.jorgk.com/images/make-tb-great-again.png" border="0">

When mail.compose.attachHttp is set to true, the link content gets downloaded and attached.

Can we rename the preference to mail.compose.attachHttpImages and only attach images?

Comment 1

[Jorg K (CEST = GMT+2)]

Attachment: 1322861-attach-only-images.patch (v1)

I think it should be like this.

Comment 2

[Jorg K (CEST = GMT+2)]

The patch also corrects some indentation issues in the vicinity :-(

Comment 3

[Jorg K (CEST = GMT+2)]

Attachment: 1322861-attach-only-images.patch (v2)

Oops, this wasn't right. You removed the "shortcut processing" above (why?), so an image with moz-do-not-send="true" set individually would still have been sent with the preference set to true. That's not right. The preference shouldn't override individual settings. I added the shortcut back in. It also saves CPU cycles, since if you already know that we don't want it, why check further?

Comment 4

[Jorg K (CEST = GMT+2)]

Attachment: 1322861-attach-only-images.patch (v3)

This needed some more cleanup:
- if it's not a DOM *element*, return early.
- if moz-do-not-sent=true, return early.
- fixed error handling for links to match other elements.
- only attach images based on preference mail.compose.attachHttpImages

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 8818126 [details] [diff] [review]
1322861-attach-only-images.patch (v3)

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

Wow, yes this needed fixing! r=mkmelin

::: mailnews/mailnews.js
@@ +775,5 @@
>  
>  pref("mail.compose.dontWarnMail2Newsgroup", false);
>  
> +// Attach http image resources to composed messages.
> +pref("mail.compose.attachHttpImages", false);

while we're chanining, maybe make it snake_style instead of camelCase, as that's more common for prefs

Comment 6

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/comm-central/rev/e27aac8e06f47087b44858975f21712793839701
Landed with a few changes:
1) https://hg.mozilla.org/comm-central/rev/e27aac8e06f47087b44858975f21712793839701#l1.27
   Checking for empty or "false" rather than "true" to make it consistent with other
   checking.
2) Used mail.compose.attach_http_images as discussed.

Comment 7

[Jorg K (CEST = GMT+2)]

Aurora (TB 52):
https://hg.mozilla.org/releases/comm-aurora/rev/89a653de4c322b3eff8845774cd5c1cd5b6bf5f7