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

RESOLVED FIXED in Thunderbird 53.0

Status

Thunderbird
Message Compose Window
RESOLVED FIXED
7 months ago
6 months ago

People

(Reporter: Jorg K (GMT+2), Assigned: Jorg K (GMT+2))

Tracking

(Blocks: 1 bug)

Trunk
Thunderbird 53.0

Thunderbird Tracking Flags

(thunderbird52 fixed, thunderbird53 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

7 months ago
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?
(Assignee)

Updated

7 months ago
Blocks: 1322155
(Assignee)

Comment 1

7 months ago
Created attachment 8817835 [details] [diff] [review]
1322861-attach-only-images.patch (v1)

I think it should be like this.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #8817835 - Flags: review?(mkmelin+mozilla)
(Assignee)

Comment 2

7 months ago
The patch also corrects some indentation issues in the vicinity :-(
(Assignee)

Comment 3

6 months ago
Created attachment 8818110 [details] [diff] [review]
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?
Attachment #8817835 - Attachment is obsolete: true
Attachment #8817835 - Flags: review?(mkmelin+mozilla)
Attachment #8818110 - Flags: review?(mkmelin+mozilla)
(Assignee)

Comment 4

6 months ago
Created attachment 8818126 [details] [diff] [review]
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
Attachment #8818110 - Attachment is obsolete: true
Attachment #8818110 - Flags: review?(mkmelin+mozilla)
Attachment #8818126 - Flags: review?(mkmelin+mozilla)

Comment 5

6 months ago
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
Attachment #8818126 - Flags: review?(mkmelin+mozilla) → review+
(Assignee)

Comment 6

6 months ago
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 months ago
status-thunderbird52: --- → affected
status-thunderbird53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 53.0
(Assignee)

Updated

6 months ago
Attachment #8818126 - Flags: approval-comm-aurora+
(Assignee)

Comment 7

6 months ago
Aurora (TB 52):
https://hg.mozilla.org/releases/comm-aurora/rev/89a653de4c322b3eff8845774cd5c1cd5b6bf5f7
status-thunderbird52: affected → fixed
You need to log in before you can comment on or make changes to this bug.