Image from web page not attached despite "Attach this image to the message" (moz-do-not-send="false")
Categories
(Thunderbird :: Message Compose Window, defect)
Tracking
(thunderbird_esr91+ fixed, thunderbird92+ fixed)
People
(Reporter: zoe, Assigned: rnons)
References
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
(deleted),
text/x-phabricator-request
|
wsmwk
:
approval-comm-beta+
wsmwk
:
approval-comm-esr91+
|
Details |
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Firefox/78.0
Steps to reproduce:
Visit https://www.iana.org/domains/reserved or any other website with images in Firefox.
Right-click onto the logo image and select "Copy Image".
Start a new composition in TB and paste the image. Double-click the image and tick:
"Attach this image to the message".
Send the message.
Result: Image is not attached.
Unrelated: It works with pref mail.compose.attach_http_images set to true.
Looks like the logic around "moz-do-not-send" logic from here
https://searchfox.org/comm-central/rev/9ea240eeda6e4301fbf528982c5d8a4d02afcd82/mailnews/compose/src/nsMsgSend.cpp#1127-1133,1223,1237
was re-implemented here:
https://searchfox.org/comm-central/rev/9ea240eeda6e4301fbf528982c5d8a4d02afcd82/mailnews/compose/src/MessageSend.jsm#1144
but the line #1223 of the original wasn't replicated here:
https://searchfox.org/comm-central/rev/9ea240eeda6e4301fbf528982c5d8a4d02afcd82/mailnews/compose/src/MessageSend.jsm#1167-1169
Looks like || mozDoNotSend.toLowerCase() == "false"
needs to be added.
Apparently there isn't a test either.
Reporter | ||
Comment 1•3 years ago
|
||
Fixed here: https://github.com/Betterbird/thunderbird-patches/blob/main/91/bugs/1727164-moz-do-not-send.patch
Reporter | ||
Updated•3 years ago
|
Assignee | ||
Comment 2•3 years ago
|
||
Updated•3 years ago
|
Assignee | ||
Comment 3•3 years ago
|
||
Copying images from web seems to be a common use case, we should not require users to manually toggle the "Attach this image" checkbox.
Depends on D123408.
Assignee | ||
Comment 4•3 years ago
|
||
Thanks for the report and patch.
Reporter | ||
Comment 5•3 years ago
|
||
(In reply to Ping Chen (:rnons) from comment #4)
Thanks for the report and patch.
Please keep the conditions in mind (https://github.com/Betterbird/thunderbird-patches/blob/main/LICENSE): resulting revisions must be attributed to the Betterbird Project. This seems to be achieved by adding your own changes to a second patch. We can't see from the Phabricator link who is registered as the author of the patch, it should be done like here: https://hg.mozilla.org/comm-central/rev/76ece33df0c5eb3879abda5acf23ae4f15bb5b7d. BTW, the attribution comment also applies to bug 760412.
Regarding the second patch: Will the result be that "Copy Image" will now always result in a data: URL, so the users can no longer reference the image via a link as before. That might be an undesired restriction, especially for large images.
Comment 6•3 years ago
|
||
Patch authors should be the person driving the patch, and handling reviews and such. Noting the link where the base patch came from seems appropriate.
Reporter | ||
Comment 7•3 years ago
|
||
(In reply to Magnus Melin [:mkmelin] from comment #6)
Noting the link where the base patch came from seems appropriate.
Agreed, that's why we state:
When You apply source code patches from this product, resulting revisions must be attributed to the
Betterbird Project in an appropriate form (changeset header or commit message) by means of Your
version control system.
That leaves it open how you want to specify it exactly.
Assignee | ||
Updated•3 years ago
|
Updated•3 years ago
|
Comment 8•3 years ago
|
||
Setting leave-open was a bit pointless when Pulsebot is down. Checked in part 1:
https://hg.mozilla.org/comm-central/rev/c8f3f063681c1666f58ec152efff4e23e4e140a3
Assignee | ||
Comment 9•3 years ago
|
||
Thanks, removing the checkin-needed as well.
Updated•3 years ago
|
Updated•3 years ago
|
Reporter | ||
Comment 10•3 years ago
|
||
From the end user pov these images are there, and showing exactly like they should. We didn't have any complaints about this, and we have a hidden pref for people who would want it (mail.compose.attach_http_images).
And may we add: Users who care about this can check the box and the image will be attached. There is no reason for users on an intranet to automatically attach images.
Updated•3 years ago
|
Assignee | ||
Comment 11•3 years ago
|
||
Comment on attachment 9237606 [details]
Bug 1727164 - Fix attaching http images when moz-do-not-send is false. r=mkmelin
[Approval Request Comment]
Regression caused by (bug #): bug 1211292
User impact if declined: http images not attached even though moz-do-not-send is set to false
Testing completed (on c-c, etc.): c-c
Risk to taking this patch (and alternatives if risky): low
Assignee | ||
Updated•3 years ago
|
Comment 12•3 years ago
|
||
Comment on attachment 9237606 [details]
Bug 1727164 - Fix attaching http images when moz-do-not-send is false. r=mkmelin
[Triage Comment]
Approved for beta
Comment 13•3 years ago
|
||
bugherder uplift |
Thunderbird 92.0b5:
https://hg.mozilla.org/releases/comm-beta/rev/c83b79c175a4
Comment 14•3 years ago
|
||
Comment on attachment 9237606 [details]
Bug 1727164 - Fix attaching http images when moz-do-not-send is false. r=mkmelin
[Triage Comment]
Approved for esr91
Comment 15•3 years ago
|
||
bugherder uplift |
Description
•