Closed Bug 754655 Opened 13 years ago Closed 13 years ago

Out of multiple identical inline images, all but 1st image break (broken image placeholder) when saving a message as draft or template multiple times (multipart numbering problem)

Categories

(Thunderbird :: Message Compose Window, defect)

defect
Not set
major

Tracking

(thunderbird14 fixed)

RESOLVED FIXED
Thunderbird 15.0
Tracking Status
thunderbird14 --- fixed

People

(Reporter: myaddons, Assigned: Bienvenu)

Details

(Keywords: dataloss, regression, Whiteboard: [needs followup bugs for comment 25])

Attachments

(5 files)

If you create a new message, insert one image multiple times and save the message multiple times as draft or template, the images get replaced by placeholder images. 1. Create a new message 2. Insert one image multiple times, e.g.: <img src="http://www.mozilla.org/favicon.ico" /> <img src="http://www.mozilla.org/favicon.ico" /> <img src="http://www.mozilla.org/favicon.ico" /> 3. Save the message as draft or template 4. Open the saved message 5. Save the message as draft or template Now the second image and the third image are replaced by placeholder images in the current editor. (However, opening the saved draft or template again shows the correct images.) I've tested this with different versions of Thunderbird: This behaviour started in Thunderbird 10. (Using Thunderbird 3.1 up to Thunderbird 9 works without problems.) Arch Linux / Thunderbird 12.0.1
(In reply to myaddons from comment #0) I followed your steps and I was able to save draft along with its images. No place holder was there. Draft was saved on an IMAP server drafts folder. User Agent: Mozilla/5.0 (X11; Linux i686; rv:12.0) Gecko/20120428 Thunderbird/12.0.1 Application Build ID: 20120428123100
Hi Hashem, thanks for looking into this! :-) I am the developer of Mail Merge, an add-on for Thunderbird, and I do have reports from some users, who could reproduce this problem easily. You did close the message compose window after you saved the message as draft the first time? And you did open the saved message and saved it as draft again? When inserting the images, can you try to use "Insert -> HTML" and insert the following HTML: <img src="http://www.mozilla.org/favicon.ico" /> <img src="http://www.mozilla.org/favicon.ico" /> <img src="http://www.mozilla.org/favicon.ico" /> So, I guess I missed one step in the description: 1. Create a new message 2. Insert one image multiple times, e.g.: <img src="http://www.mozilla.org/favicon.ico" /> <img src="http://www.mozilla.org/favicon.ico" /> <img src="http://www.mozilla.org/favicon.ico" /> 3. Save the message as draft or template 4. Close the message compose window 5. Open the saved message 6. Save the message as draft or template And again, the placeholder image does only show in the current editor of the message compose window after you have saved the message as draft the second time. If you close the message compose window afterwards and take a look at the saved message, the original image shows fine.
Confirming on TB12.0.1 on WinXP Immediately reproducable with steps from comment 2 This is the resulting code after step 6 of comment 2 (select images, then "Insert > HTML" to check) <img src="mailbox:///C:/Dokumente%20und%20Einstellungen/User/Anwendungsdaten/Thunderbird/Profiles/abc1abc2.default/Mail/pop.gmx.net/Drafts?number=12345678&amp;part=1.2&amp;filename=favicon.ico"> <img src="mailbox:///C:/Dokumente%20und%20Einstellungen/User/Anwendungsdaten/Thunderbird/Profiles/abc1abc2.default/Mail/pop.gmx.net/Drafts?number=12345678&amp;part=&amp;filename=favicon.ico"> <img src="mailbox:///C:/Dokumente%20und%20Einstellungen/User/Anwendungsdaten/Thunderbird/Profiles/abc1abc2.default/Mail/pop.gmx.net/Drafts?number=12345678&amp;part=&amp;filename=favicon.ico"> Out of the three images, only the first one still shows right, 2 and 3 are broken. I notice that the working img has part=1.2, but broken images have part= and then nothing, so there is no part number. Could that be the cause of this bug?
(In reply to Thomas D. from comment #3) > Confirming on TB12.0.1 on WinXP > Immediately reproducable with steps from comment 2 That's on a local POP account.
(In reply to myaddons from comment #2) Agreed. When you save the draft the 2nd time, only the 1st picture appears. But when you close and re-open it, all of them appear fine.
(In reply to Hashem Masoud from comment #5) > (In reply to myaddons from comment #2) > Agreed. When you save the draft the 2nd time, only the 1st picture appears. > But when you close and re-open it, all of them appear fine. That's right. The problem is, any time you save draft again manually, the other 2 pics will disappear again, and if you send from that situation, they are not sent. Hashem, did you see duplicates of this? Otherwise, we could confirm.
(In reply to Thomas D. from comment #3) ... > <img > src="mailbox:///C:/Dokumente%20und%20Einstellungen/User/Anwendungsdaten/ > Thunderbird/Profiles/abc1abc2.default/Mail/pop.gmx.net/ > Drafts?number=12345678&amp;part=1.2&amp;filename=favicon.ico"> > <img > src="mailbox:///C:/Dokumente%20und%20Einstellungen/User/Anwendungsdaten/ > Thunderbird/Profiles/abc1abc2.default/Mail/pop.gmx.net/ > Drafts?number=12345678&amp;part=&amp;filename=favicon.ico"> > <img > src="mailbox:///C:/Dokumente%20und%20Einstellungen/User/Anwendungsdaten/ > Thunderbird/Profiles/abc1abc2.default/Mail/pop.gmx.net/ > Drafts?number=12345678&amp;part=&amp;filename=favicon.ico"> > > Out of the three images, only the first one still shows right, 2 and 3 are > broken. I notice that the working img has part=1.2, but broken images have > part= and then nothing, so there is no part number. Could that be the cause > of this bug? I can confirm the same behaviour especially the "part=1.2" is replaced with only "part=" The result is not only the image not displaying correctly, but the email will not send to any recipient. Windows 7, Thunderbird 12.01 on two machines.
Similar bugs, but not exactly dupes afaics: bug 560210, bug 560877, bug 566120
Keywords: regression
The source code of that same msg in my Sent folder is also completely weird (messed up): The HTML part has <img src="cid:part1.03080104.09080808@gmx.de"> <img src="cid:part2.02000609.00090306@gmx.de"> <img src="cid:part2.02000609.00090306@gmx.de"> ...which looks wrong because the cid's should either all point to the same picture (one source for all), or, less efficiently, each to a different picture with its own source included (3 sources). part1.03080104.09080808@gmx.de is correctly included as icon source Then follows another HTML part (which itself has the ID of 2nd and 3rd graphic src from 1st HTML part above!?!) which still has references to the graphics from the draft message (which no longer exists, as this already been sent)!?? : Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-ID: <part2.02000609.00090306@gmx.de> [snip] <img src="mailbox:///C:/Dokumente%20und%20Einstellungen/User/Anwendungsdaten/Thunderbird/Profiles/abc1abc2.default/Mail/pop.gmx.net/Drafts?number=28804591&part=1.2"> <img src="mailbox:///C:/Dokumente%20und%20Einstellungen/User/Anwendungsdaten/Thunderbird/Profiles/abc1abc2.default/Mail/pop.gmx.net/Drafts?number=28804591&part=1.2"> <img src="mailbox:///C:/Dokumente%20und%20Einstellungen/User/Anwendungsdaten/Thunderbird/Profiles/abc1abc2.default/Mail/pop.gmx.net/Drafts?number=28804591&part=1.2"> This looks totally bogus to me. But the confusions seem obvious enough that it might be possible to track them down relatively "easily" for someone who knows ;)
Summary: Images get replaced by placeholder images when saving a message as draft or template multiple times → Out of multiple identical inline images, all but 1st image break (broken image placeholder) when saving a message as draft or template multiple times
(In reply to Thomas D. from comment #9) > src="mailbox:///C:/Dokumente%20und%20Einstellungen/User/Anwendungsdaten/ > Thunderbird/Profiles/abc1abc2.default/Mail/pop.gmx.net/ > Drafts?number=28804591&part=1.2"> ...I forgot to manually replace Drafts?number=28804591 with Drafts?number=12345678 as I did in comment 9 when sanitizing the source for privacy (surely the original draft numbers must have been the same).
According to comment 0, this regression happened between TB9 and TB10. Confirming as this is reproducable and has something tangible in the source code of affected messages. It might still be a duplicate, but it's too hard to check.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached file Draft1.eml (after 1st save; OK) (deleted) —
This is the source of the draft after 1st manual saving (from drafts folder). All identical images referenced correctly, using same source.
After reopening the saved draft (step 5 of comment 2) in editor, the img src for all images correctly points to the same part 1.2 of the draft in drafts folder - although I don't understand why we are using part 1.2 which does not seem to exist in the source of the draft. It seems to work though, all identical images are shown in editor.
Attachment #625361 - Attachment description: Draft1 (after 1st save; OK).eml → Draft1.eml (after 1st save; OK)
This is where editor breaks the img references: 1st img has src "...Drafts?number=12345678&amp;part=1.2&amp;filename=favicon.ico" 2nd img has src "...Drafts?number=12345678&amp;part=&amp;filename=favicon.ico" 3rd img has src "...Drafts?number=12345678&amp;part=&amp;filename=favicon.ico"
When trying to send jumbled draft4 (after step 6 of comment 2), of course we end up with something broken. However, the complete mess of parts we generate is really amazing: --------------040306020108020705050309 Content-Type: multipart/related; boundary="------------020906030102020800040206" --------------020906030102020800040206 Content-Type: text/html; charset=ISO-8859-1 [snip] img1: src="cid:part1.09030907.06090300@bar.com" img2: src="cid:part2.00080905.05070809@bar.com" img3: src="cid:part2.00080905.05070809@bar.com" (src of img2 and img3 is wrong) --------------020906030102020800040206 Content-Type: image/x-icon (the mail should end here; instead, we have another text/html part which is completely bogus at this time! Note that the wrong src references/cid's of img2 and img3 point to the content ID of this extra html part!: part2.00080905.05070809@bar.com) --------------020906030102020800040206 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-ID: <part2.00080905.05070809@bar.com> <html> <head> <meta http-equiv="content-type" content="text/html; charset=ISO-8859-1"> </head> <body bgcolor="#FFFFFF" text="#000000"> <u><img alt="" src="mailbox:///C:/Dokumente%20und%20Einstellungen/User/Anwendungsdaten/Thunderbird/Profiles/abc1abc2.default/Mail/pop.bar.com/Drafts?number=28882265&part=1.2&filename=favicon.ico" height="16" width="16"><img alt="" src="mailbox:///C:/Dokumente%20und%20Einstellungen/User/Anwendungsdaten/Thunderbird/Profiles/abc1abc2.default/Mail/pop.bar.com/Drafts?number=28882265&part=1.2&filename=favicon.ico" height="16" width="16"><img alt="" src="mailbox:///C:/Dokumente%20und%20Einstellungen/User/Anwendungsdaten/Thunderbird/Profiles/abc1abc2.default/Mail/pop.bar.com/Drafts?number=28882265&part=1.2&filename=favicon.ico" height="16" width="16"></u> </body> </html>
Considering the complete multipart garbage structure that we create and send out, this looks major to me.
Severity: normal → major
Ludo, who can find code of comment 14, where the following happens: - when user manually saves reopened draft (with multiple identical images) - in img src for subsequent identical images (except 1st img), - part no. (part=1.2) gets omitted
Confirming for trunk, too (on WinXP, Pop3). Ludo, any bug candidates within the regression range (per comment 0, between TB9 and TB10) that were tweaking drafts/multipart behaviour and could have caused this?
Summary: Out of multiple identical inline images, all but 1st image break (broken image placeholder) when saving a message as draft or template multiple times → Out of multiple identical inline images, all but 1st image break (broken image placeholder) when saving a message as draft or template multiple times (multipart numbering problem)
(In reply to Thomas D. from comment #18) > Confirming for trunk, too (on WinXP, Pop3). > > Ludo, any bug candidates within the regression range (per comment 0, between > TB9 and TB10) that were tweaking drafts/multipart behaviour and could have > caused this? Bug 692875 - editing an existing draft with inline images doesn't handle mime part renumbering when new images are added before existing images We did not test for multiple images with same URL, probably renumbered is only first one.
(In reply to Arivald from comment #19) > (In reply to Thomas D. from comment #18) > > Confirming for trunk, too (on WinXP, Pop3). > > > > Ludo, any bug candidates within the regression range (per comment 0, between > > TB9 and TB10) that were tweaking drafts/multipart behaviour and could have > > caused this? > > Bug 692875 - editing an existing draft with inline images doesn't handle > mime part renumbering when new images are added before existing images > > We did not test for multiple images with same URL, probably renumbered is > only first one. Ah, but I did test this, but only in the case of locally embedded images, and not remote images. So, for remote images in the form of <img src="http://www.mozilla.org/favicon.ico" /> it seems that the save draft/template assumes that it should be converted to an inline image. Not a bad assumption in that the moz-do-not-send="true" attribute was not set. So we should probably not make that assumption in the code and test for "http" in the image source. Anyway...if you paste in html code with that attribute, things work fine. example: <img alt="" src="http://img408.imageshack.us/img408/1973/rss14iq7.gif" moz-do-not-send="true" height="14" width="14"><br> <img alt="" src="http://img408.imageshack.us/img408/1973/rss14iq7.gif" moz-do-not-send="true" height="14" width="14"><br> <img alt="" src="http://img408.imageshack.us/img408/1973/rss14iq7.gif" moz-do-not-send="true" height="14" width="14"><br> There may be some cases where you do see placeholders instead of images. But the "remoteness" is preserved if you include that attribute.
Assignee: nobody → dbienvenu
Status: NEW → ASSIGNED
nsMsgComposeAndSend::PreProcessPart ignores the duplicate image parts because they're marked as m_bogus_attachment, which means we don't put it in the m_partNumbers array. I think this is because we weed out the duplicate parts earlier. I think when we do this, we need to make sure we have an entry in m_partsNumber for the duplicate parts.
Attached patch proposed fix (deleted) — Splinter Review
this fixes the STR's described in this case, though I think it probably needs a bit of user testing. I'll generate a try server build for JoeS and anyone else interested to try.
Attachment #627413 - Flags: review?(neil)
I requested a try server build - windows and mac builds should show up here - http://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/bienvenu@nventure.com-5ff4941ce502
Testing the try build: Mozilla/5.0 (Windows NT 5.1; rv:15.0) Gecko/15.0 Thunderbird/15.0a1 ID:20120525173756 Seems to work perfectly here. It even pre-processes the duplicate remote source into one actual embed +1 on that I didn't expect that to happen.
(In reply to David :Bienvenu from comment #22) > Created attachment 627413 [details] [diff] [review] > proposed fix > > this fixes the STR's described in this case, though I think it probably > needs a bit of user testing. I'll generate a try server build for JoeS and > anyone else interested to try. David, that's great and so quick, thank you! Confirming that tryserver build fixes the bigger issues for me. There are some smaller issues when more (same) images are added sequentially, say in different editing sessions of same draft on different days: STR 1) insert 3 identical graphics as described in comment 0 <img src="http://www.mozilla.org/favicon.ico" /> <img src="http://www.mozilla.org/favicon.ico" /> <img src="http://www.mozilla.org/favicon.ico" /> 2) save draft, close(!), reopen (edit draft) 3) insert another set of same 3 identical graphics, so you now have 6 identical graphics <img src="http://www.mozilla.org/favicon.ico" /> <img src="http://www.mozilla.org/favicon.ico" /> <img src="http://www.mozilla.org/favicon.ico" /> 4) save draft, close (!) 4b) View Source of draft (Ctrl+U) 4c) edit draft, view HTML source of draft (Ctrl+A, Insert > HTML) Actual result: 4b) (tags are less nicely formatted in real life...) <img src="cid:part1.09080207.03010505@gmx.net"> <img src="cid:part1.09080207.03010505@gmx.net"> <img src="cid:part1.09080207.03010505@gmx.net"> <br> <br> <img src="cid:part4.04030806.03000007@gmx.net"> <img src="cid:part4.04030806.03000007@gmx.net"> <img src="cid:part4.04030806.03000007@gmx.net"> ... --------------020907080805080906080004 Content-Type: image/x-icon Content-Transfer-Encoding: base64 Content-ID: <part1.09080207.03010505@gmx.net> AAABAAIAEBAAAAEACABoBQAAJgAAABAQAAABACAAaAQAAI4FAAAoAAAAEAAAACAAAAABAAgA ... --------------020907080805080906080004 Content-Type: image/x-icon; name="favicon.ico" Content-Transfer-Encoding: base64 Content-ID: <part4.05040403.07030500@gmx.net> Content-Disposition: inline; filename="favicon.ico" AAABAAIAEBAAAAEACABoBQAAJgAAABAQAAABACAAaAQAAI4FAAAoAAAAEAAAACAAAAABAAgA ... 4c) <img src="mailbox:///C:/.../Profiles/a1aaaa1a.Daily15tmp/Mail/pop.gmx.net/Drafts?number=12345&amp;part=1.2"> <img src="mailbox:///C:/.../Profiles/a1aaaa1a.Daily15tmp/Mail/pop.gmx.net/Drafts?number=12345&amp;part=1.2"> <img src="mailbox:///C:/.../Profiles/a1aaaa1a.Daily15tmp/Mail/pop.gmx.net/Drafts?number=12345&amp;part=1.2"> <br> <br> <img src="mailbox:///C:/.../Profiles/a1aaaa1a.Daily15tmp/Mail/pop.gmx.net/Drafts?number=12345&amp;part=1.3&amp;filename=favicon.ico"></u> <img src="mailbox:///C:/.../Profiles/a1aaaa1a.Daily15tmp/Mail/pop.gmx.net/Drafts?number=12345&amp;part=1.3&amp;filename=favicon.ico"> <img src="mailbox:///C:/.../Profiles/a1aaaa1a.Daily15tmp/Mail/pop.gmx.net/Drafts?number=12345&amp;part=1.3&amp;filename=favicon.ico"> Problems: A) Bug: Content-Disposition and filename gets lost -> that should be fixed > Content-Disposition: inline; > filename="favicon.ico" This part (both content-disposition and filename) gets lost! (if you save, close and reopen more often, it will get lost for all images!) B) Inefficient, minor: For 6 (2x3) identical images (albeit added in different edit sessions), *one* embedded mime source would be enough -> optional depending on work and costs For users who add multiple identical images in different editing sessions, we still create multiple embedded instances of same image. For optimum size of mail, it should be only one embedded source. I don't know how hard it is, and what the costs are, to do this: When preprocessing, test for identical mime sources: - for all inline image sources, check size of source part (assuming that's faster than doing checksums) - if size is identical - if only 2 identical image sizes: compare mime sources - if mime sources are identical: embed as single source - elseif more than 2 identical image sizes: - compare checksums (MD5 or something like that) - if checksums are identical, compare mime sources (to be sure) - if mime sources are identical, combine into single mime source and modify CID references accordingly - elseif checksums are different: nothing to optimize - elseif size is different: nothing to optimize C) Trivial: Be more coherent about part numbering -> Cosmetic, but nice to have We create multipart message: Part1: html (parent part) Part2: img (child part) part3: img (child part) In composition editor, these parts are coherently identified: Part1: html Part2: Drafts?...;part=1.2;... Part3: Drafts?...;part=1.3;... (That's a nice and intelligible numbering) The same parts, from Draft Source or when sent, confusingly numbered like this: Part1: html Part2: cid:part1.09080207.03010505@gmx.net (why part1? this is the 2nd part) Part3: cid:part4.04030806.03000007@gmx.net (why part4? there is no part 3!) A nicer numbering, more coherent with parts numbering in editor, would be this: Part1: html Part2: cid:part2.09080207.03010505@gmx.net (2nd part, that's part=1.2 in editor) Part3: cid:part3.04030806.03000007@gmx.net (3rd part, that's part=1.3 in editor)
I can confirm that the try server build fixes my initial problem! :-) However, I can also confirm the (minor) issues mentioned by Thomas in comment #22.
I can confirm that the try server build fixes my initial problem! :-) However, I can also confirm the (minor) issues mentioned by Thomas in comment #25 (not in comment #22, sorry).
thx for testing the try build! Are the minor issues regressions caused by my patch, or were they pre-existing issues?
I've just run a quick test with the current Thunderbird 12.0.1 and the (minor) issues do also happen there. So I guess they are not caused by your patch.
And some other tests: The (minor) issues in comment #25 do also happen with Thunderbird 3.1.20(!) and Thunderbird 9.
This issue: >A) Bug: Content-Disposition and filename gets lost > that should be fixed > > Content-Disposition: inline; > filename="favicon.ico" >This part (both content-disposition and filename) gets lost! Has been around for a very long time (I can't find a dup, but I swear that I filed this on Netscape 7.2 :) Let's just spin those minor bugs off after this one is reviewed and landed.
Status: ASSIGNED → NEW
(In reply to Joe Sabash from comment #31) > This issue: > >A) Bug: Content-Disposition and filename gets lost > > that should be fixed > > > > Content-Disposition: inline; > > filename="favicon.ico" > >This part (both content-disposition and filename) gets lost! I can also confirm for TB12.0.1 that issue of A) was there before this patch. > Has been around for a very long time (I can't find a dup, but I swear that I > filed this on Netscape 7.2 :) From time immemorial... *sigh*... I should have guessed that, silly me! > Let's just spin those minor bugs off after this one is reviewed and landed. That's perfectly fine with me.
Status: NEW → ASSIGNED
Attachment #627413 - Flags: review?(neil) → review+
Status: ASSIGNED → NEW
Keywords: checkin-needed
Status: NEW → ASSIGNED
Whiteboard: [needs followup bugs for comment 25]
fixed on trunk
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 15.0
Version: 12 → Trunk
Due to the dataloss nature of this bug (image data lost when sending), and the straightforward, minimal fix - can we land this on at least aurora, too?
Keywords: dataloss
Comment on attachment 627413 [details] [diff] [review] proposed fix [Triage Comment] I'm fine with landing this on Aurora, though it's more datalossy than dataloss
Attachment #627413 - Flags: approval-comm-aurora+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: