Closed Bug 380372 Opened 17 years ago Closed 13 years ago

Attachment error when saving twice as draft a mail template containing an inline image

Categories

(Thunderbird :: Message Compose Window, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 10.0

People

(Reporter: jsoursou, Assigned: Bienvenu)

References

Details

(Keywords: helpwanted)

Attachments

(2 files, 4 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; fr; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3
Build Identifier: version 2.0.0.0 (20070326)

Since the bug 345501 https://bugzilla.mozilla.org/show_bug.cgi?id=345501 when you open a message template containing inline images, after 10 min writing your message, you get an attachment error concerning the image. And if you don't re insert your image again, you can't send the mail because of it. This attachment error can be reproduced by saving twice the message template as draft.

Reproducible: Always

Steps to Reproduce:
1. Open mail composer 
2. Insert an image 
3. Save it as Template
4. Close the mail composer
5. Open your mail template by double-click on it
6. Save twice your mail as Draft or keep the mail composer window opened enough time to enable two auto-save as draft (10min usually = 2*mail.compose.autosaveinterval)
Actual Results:  
"attachment error" and unable to save the message as draft again and unable to send the message.



You can go round this error by re_inserting the image in your message. 
1. delete the inline images in your message
2. re insert them
Confirmed on Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a5pre) Gecko/20070515 Thunderbird/3.0a1 ID:2007051503 [cairo]
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 2000 → All
Hardware: PC → All
Can be worked around by 
1. create new message from template
2. save new message to drafts
3. do not attach files for sending as they will be lost when error occurs
4. when error occurs cut and paste text from message into draft message before discarding the failing message
5. open draft and attach required file(s)then send.
I've found that turning off the "autosave" feature corrects the problem as well. Autosave is a nice feature, but not if it causes so many errors.
I have reproduced this bug on 2 computers, both with Thunderbird 2.0.0.6 installed. One PC runs under WinXP (unsure if SP1 or SP2) the other runs under Win Vista (but there are so many versions, I can't remember which one).
I have also reproduced this bug on 2 machines running Windows 2000 service pack 4 build 2195.  One using Thunderbird 2.0.0.6 plain, and one with 2.0.0.6 and Lightning 0.7 (build 2007102204)
Just summing (and confirming) the description and comment #4: if we have Autosave enabled, all TB takes to err is to edit a message (new or not) long enough. As the default seems to be autosave each 5 minutes, 10 minutes of editing will cause this problem if the user (majority) doesn't change this setting.
(TB 2.0.0.6 build 20070728 pt-BR over Win2k SP4 pt-BR)
Thanks
Note that, as per bug 391232 Variant 1, this error can also be generated by manually saving, not just by using autosave.
This problem also occurs whe I try to "edit as new" an old message with a img attached, and not only in autosave but also in sending content. The problem it's due to the html ref of this image: it points to mailbox saved image instead of original file.
For example, if I attached an html object like file:///c:/Documents and Settings/monari_emanuele/My Documents/firma.gif in a mail, when I reopen the message with "edit as new", the reference of this image correctly displayed and not correctly managed it's mailbox:///C|/Documents%20and%20Settings/monari_emanuele/Application%20Data/Thunderbird/Profiles/4vafokvd.default/Mail/Local%20Folders/Sent?number=51727577&part=1.1.2&filename=firma.gif

If I double click on image, and I specify the disk position on image, then I can save/send/whatever else.

I think problem it's in the incapacity of embedding for save/send images from a mailbox:///-type url.
I hope it's now simply to find solution...
(version 2.0.0.0 (20070326))
I confirm this bug and I believe that it's caused by two separate causes (you can reproduce these steps controlling the compose source with my extension EditHtml):

1) (just for windows) https://bugzilla.mozilla.org/show_bug.cgi?id=224733
2) when you save a message with an image, the path image is corrupted in this way
- create a template with an image
- open it by double-clicking --> now the path of the image is correct and points to a url when you find "Templates?number=X", where X is the key number of message template.
- save it --> now the path is wrong, because you'll find "Templates?number=Y", where Y is the key number of the saved message in Draft. Of course the image is not available anymore, because the path it's wrong.

I've insert a fix for both bugs in my extension QuoteAndComposeManager, that you can find here: https://nic-nac-project.org/~kaosmos/realborders-en.html
I can confirm that your QuoteAndComposeManager does seem to fix this problem. Finally I can re-upgrade from Thunderbird 1.5; this was driving me nuts in 2.x!

Thanks Paolo.

This is a quick work around...

1.Goto the templates folder in Thunderbird, right click your selected template and choose 'Edit as New..'
2.Goto the new composed email and select File -> Save as -> draft (You may want to update the subject first so that you can easily locate it later).
3. Shutdown this new composed e-mail. File -> close
4. Now locate the new e-mail in the drafts folder and open it up to continue composing your e-mail and it will save as often as you like.
Assignee: mscott → nobody
There is one big design flaw with Thubderbird cid: content handling. In storage, source of mails contain cid: links to other MIME parts, but when mail is viewed or edited, some code exchange cid: URL's with propertiary URL's like "mailbox:///C%7C/path-to-profile/Mail/Local%20Folders/Drafts?number=236476&part=1.2&filename=image.jpg"

This is big problem now, because if you save message (yourself or by auto-save) Then now message is contained in different folder, but image url was not changed. You can still see image, because it was already loaded, but when you try to save, serializer code try to get new copy of image using invalid url.

In my extension ("Stationery", designed to work like OE stationery function) i try to resolve this problem, but i do not find final, always working solution.
My current, development version close original composer window just after save, and reopen composer from just saved copy.
Flags: wanted-thunderbird3?
Flags: blocking-thunderbird3?
I disabled Autosave and the problem still happens, though in a rare fashion (or maybe it takes loooong).
Win2k SP4 TB 2.0.0.19 20081209 Lightning GCal SamePlace Talkback xmpp4moz
Deleting images and reinserting them did work around.
This is an annoying bug, for sure!  It wouldn't block tb3, but we'd love a patch if anyone has time to dig into this.
Flags: wanted-thunderbird3?
Flags: wanted-thunderbird3+
Flags: blocking-thunderbird3?
Flags: blocking-thunderbird3-
Keywords: helpwanted
I can confirm this bug still exists in a clean install of 2.0.0.19. Saving it as a draft, then closing the template and sending the draft works. Interestingly enough, the template will send just fine after that initial save-and-close. Sure is an irritating bug!
VERY frustrating bug, but I've had some luck if I QUICKLY make my changes and IMMEDIATELY/quickly click SEND, before an auto-save. I haven't tried to delay/extend the time between auto-saves; that might make a difference.
This bug still exists.  It is not only related to templates but also to e-mails sent with inline images that are replied back to by the recipient that the original sender tries to reply back to.

There has been a lot of bad advice offered in the MozillaZine Forums to try to fix this.  Idiotic things from deleting .msf files to creating new profiles.  The problem is in the code.
Strangely, if you send an e-mail with inline pictures and that e-mail is replied to by the recipient and you want to reply back to the reply, instead of hitting REPLY hit FORWARD.  Using FORWARD works fine.

This is the quickest workaround.

Of course, someone who can program should fix the REPLY issue.
(In reply to comment #16)
> "mailbox:///C%7C/path-to-profile/Mail/Local%20Folders/Drafts?number=236476&part=1.2&filename=image.jpg"

Sounds problem as Bug 461785 Comment #26. Can you reproduce problem with newest nightly build?
> http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/latest-comm-1.9.1/
I forgot to mention forwarding must be set to inline for the above workaround to work.

I am running 2.0.0.23.  Will running the nightly .exe mess up any 2.0 settings?
(In reply to comment #24)
> I am running 2.0.0.23.  Will running the nightly .exe mess up any 2.0 settings?

Tb 3.0(bX, rcX, pre) alters prefs.js content. You are better to test with newly created profile.
The nightly build appears to have corrected this issue.
(In reply to comment #26)
> The nightly build appears to have corrected this issue.

I'm not entirely clear which issue you are referring to, but having just tested the latest nightly build, the bug has not been fully resolved (tested against variant 1 of bug 391232, which is a duplicate of this bug), even if part of it may have been.
*** Bug 539900 has been marked as a duplicate of this bug. ***
I disagree comment #26

my two cents for this bug :
my way to reproduce it :

Take a message with an image incorporated
Edit as New
After a first saving (manually, or a automacally save) the mail
contain a bad reference to the image and cannot be saved or send anymore

Reproducible: Always

Steps to Reproduce:
1 - Select a mail in your inbox with a image inside. 
2 - Click on right button and "Edit as New"
3 - Now you are in the compose window
4 - Save your mail as a draft (first time)

At this point all is OK, but : 

5 - Save your mail as a draft a second time, 
and now you have a very nice error message :
"Unable to save ..."
Notice that you cannot send your message at this point.

But if you close your compose window, and reopen the saved Draft
you can send it normally.

Actual config : TB 3.0 on XP SP2
Got the EXACT bug, but any solutions? :(
I have this same issue on both of my XP-based systems. I've searched for a solution and while it appears that there is a couple of work-arounds, no real fix has been found for this very irritating and rather common problem -- that appears to have existed since at least 2007! 

I'm very surprised that this issue has not been resolved and suggest that a higher priority is placed on solving this problem.
Bug 542872 may be related, where the cause was an extension. I don't see any mention of using safe mode in the comments ...
(In reply to comment #33)
> Bug 542872 may be related, where the cause was an extension. I don't see any
> mention of using safe mode in the comments ...

You are wrong. This bug happens on clean TB profile too.
I'm surprised this has not been solved in TB core. The problem was first reported May 15, 2007! As a TB devotee, I'd appreciate some word from the developers as to why I must go to an extension writer to get the solution.
The same thing happens (I hope I'm right assuming it is the same bug) when copy/pasting images, tried it with TB 3.1.11 on Windows XP SP3 and TB 3.1 on Windows 7. A simple way to trigger it is this:

1. Create a new message (Ctrl-M)
2. Paste any image. For example, Alt-PrintScreen to take a snapshot, Ctrl-V to paste it into the message.
3. Save the message as draft (Ctrl-S).
4. Close the message
5. Open the saved draft
6. Select the previously pasted image, cut (or copy) and then paste it back into the message. A "broken image" icon appears and the draft cannot be saved anymore, it says "Unable to save your message as draft. There was an error attaching. Please check if you have access to the file." 

When I try to view the clipboard contents for the cut/copied image, it looks something like this:

Version:0.9
StartHTML:00000120
EndHTML:00000372
StartFragment:00000154
EndFragment:00000336
SourceURL:about:blank
<html><body>
<!--StartFragment--><img src="mailbox:///C%7C/Documents%20and%20Settings/Xyz/Application%20Data/Thunderbird/Profiles/3jfdkgtg.default/Mail/Local%20Folders/Drafts?number=386360280&amp;part=1.2" alt=""><!--EndFragment-->
</body>
</html>
Just install my "Stationery" extension, even if You did not want to use HTML files as templates. It fixes many problems like this above.

For image, it os probably case with broken serializing <=> deserializing. TB serialize path to file as "/C%7C/Documents..." (decoded "/C|/Documents..."), but parser (or image handler) no longer recognize "|" as replacement for ":".

My extension fix such paths to "/C%3A/Documents..." (decoded "/C:/Documents...") and images start working properly.


Also earlier fix (comment #16) works: when Stationery detects that message after saving become broken, it close current composer window, and reopen it from Drafts. It happen only once per message, version opened from drafts can be saved again many times. On the cons side, this operation break undo history...
Attached patch proposed fix (obsolete) — Splinter Review
this should fix several of the issues with editing messages with embedded content, especially when IMAP is involved.
Assignee: nobody → dbienvenu
try server builds here for TB 7 beta + this fix, if anyone wants to try them.

http://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/bienvenu@nventure.com-0dc12ff7a441/
(In reply to David :Bienvenu from comment #40)
> try server builds here for TB 7 beta + this fix, if anyone wants to try them.
> 
> http://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/
> bienvenu@nventure.com-0dc12ff7a441/

Seems to work fine if all the images are newly inserted during the editing session.
However if you are editing a previously composed message that has an inline image, you end up with this: (the pre-composed image disappears from the compose window)
<img alt="" src="file:///C:/saved/1a-1.gif" height="28" width="28"><br><br><br></font>
<img alt="" src="mailbox:///C%7C/Documents%20and%20Settings/Administrator/Application%20Data/Thunderbird/Profiles/zll4rpfc.default/Mail/incoming.verizon.net/Drafts?number=17884&amp;part=1.2.2&amp;filename=1h.gif?part=1.2.2&amp;filename=1h.gif" height="28" width="28">

But using the EditHtml extension, and modifying the second image url to match the first one's file url, it reappears and seems to work fine.

So it seems it is actually saved OK and the answer for a complete fix would be to apply the file url format to precomposed inline images.

IOW the second should be:
<img alt="" src="file:///c:/saved/1h.gif" height="28" width="28">
(In reply to Joe Sabash from comment #41)
Thx for trying this. Can you write do the steps you did to create the issue you saw? I'm not sure what a precomposed message is. Is it a hand-crafted message you've put in a templates folder, or a message you've created with the UI and saved?
Attached patch with uuid rev (obsolete) — Splinter Review
Attachment #561333 - Attachment is obsolete: true
Attachment #561925 - Flags: superreview?(mbanner)
Attachment #561925 - Flags: review?(neil)
(In reply to David :Bienvenu from comment #42)
> (In reply to Joe Sabash from comment #41)
> Thx for trying this. Can you write do the steps you did to create the issue
> you saw? I'm not sure what a precomposed message is. Is it a hand-crafted
> message you've put in a templates folder, or a message you've created with
> the UI and saved?

Yeah, I did ramble a bit there
Steps
1 compose a message with an inline image
2 save it to a local folder template/outbox
3 edit as new with the save as draft enabled
4 add another inline image
5 wait for it to be saved

Result
The original image disappears from the composition window
The added image stays there just fine

Hypothesis
The added image was actually saved correctly, but the image link was not updated.
Modifying the new image link to the "src="file:///C:/saved/xxx" format seems to fix it.
(I did that on the fly in the composition window using the EditHtml extension.)
Well that insertion of "src="file:///C:/saved/xxx" was actually a reference to an existing folder/image location on my HD
So in essence it equates to re-inserting the previously inserted images.
Sorry for the spam.
But the fact remains that previously inserted images now get lost after saving of the draft.
(In reply to Joe Sabash from comment #44)
> (In reply to David :Bienvenu from comment #42)
> > (In reply to Joe Sabash from comment #41)
> > Thx for trying this. Can you write do the steps you did to create the issue
> > you saw? I'm not sure what a precomposed message is. Is it a hand-crafted
> > message you've put in a templates folder, or a message you've created with
> > the UI and saved?
> 
> Yeah, I did ramble a bit there
> Steps
> 1 compose a message with an inline image
> 2 save it to a local folder template/outbox
> 3 edit as new with the save as draft enabled
> 4 add another inline image
> 5 wait for it to be saved
> 
> Result
> The original image disappears from the composition window
> The added image stays there just fine
> 
> Hypothesis
> The added image was actually saved correctly, but the image link was not
> updated.
> Modifying the new image link to the "src="file:///C:/saved/xxx" format seems
> to fix it.
> (I did that on the fly in the composition window using the EditHtml
> extension.)

This is not right. If you already have image, it is already attached to message. But its source file on disk may not exists anymore. So You can NOT change "mailbox:///" URL back to "file:///: URL!
It should remain "mailbox:///" URL, but valid one.

To test this case, try with image pasted from clipboard, or drag'n'dropped. TB will insert it as "data:" URL. But if You save it and reopen again it will become "mailbox://" URL, because TB already attached this image to message.


Your broken URL:
"mailbox:///C%7C/Documents%20and%20Settings/Administrator/Application%20Data/Thunderbird/Profiles/zll4rpfc.default/Mail/incoming.verizon.net/Drafts?number=17884&amp;part=1.2.2&amp;filename=1h.gif?part=1.2.2&amp;filename=1h.gif"

Should be fixed to 
"mailbox:///C%7C/Documents%20and%20Settings/Administrator/Application%20Data/Thunderbird/Profiles/zll4rpfc.default/Mail/incoming.verizon.net/Drafts?number=17884&amp;part=1.2.2&amp;filename=1h.gif"

By removing "?part=1.2.2&amp;filename=1h.gif" from end of string. Or rather by not adding it in first place.
this worked fine for me - are you really seeing the url look like this? &amp;filename=...? It should be &filename=, without the entity replacement.
(In reply to David :Bienvenu from comment #47)
> this worked fine for me - are you really seeing the url look like this?
> &amp;filename=...? It should be &filename=, without the entity replacement.

I have 2 ways to view the source while in compose.
Edithtml extension (I think stationery extension also) and yt doing Ed>>Select all>>Insert Html
Both methods show &amp;filename=...

Are you sure that Tryserver build is good?
That was built against Beta branch, and would not have the changes mentioned here: https://bugzilla.mozilla.org/show_bug.cgi?id=679476#c3 included
(In reply to Joe Sabash from comment #48)

> I have 2 ways to view the source while in compose.
> Edithtml extension (I think stationery extension also) and yt doing
> Ed>>Select all>>Insert Html
> Both methods show &amp;filename=...
When I use the debugger to look at the url that's in the image src, it looks fine. I suspect you'd see the same thing in a build without my patch.

> Are you sure that Tryserver build is good?
> That was built against Beta branch, and would not have the changes mentioned
> here: https://bugzilla.mozilla.org/show_bug.cgi?id=679476#c3 included
Why would that matter? The fix is independent of those changes. I just made a beta try server build because I was hoping that would make more people willing to try the build.

I've been using insert image to insert the image, not drag and drop - don't know if that would matter.
yeah, drag drop creates a data url for the image, so it's not involved in this bug.
Yeah, I've been using insert image to test here also.
So here is what is happening in my case.
There seems to be a difference in how the part=x.x.x. is formed between the draft folder and all others.
To prove this to myself here's what I did:
Compose a message with an inserted image
Immediately save it as a draft (without auto-save happening)
Open it in compose and observe the part= number (I see a two digit number part=1.2)
Open that same message from a local folder and observe part=1.2.2

So in the case where I lose the image links after a save, editing the part= from 1.2.2 to 1.2, 1.3 etc restores the links and saving to draft works from that point on.

I have no idea why I'm seeing this symptom, and you are not.
I'm testing with 32 bit windows OS here win2k and winxp.

Hope this is clue, sorry for the spam if it is not.
Neither of my trunk builds seem to want to display or edit a draft - none of the cid: URIs are getting resolved into mailbox: URLs.
(In reply to neil@parkwaycc.co.uk from comment #52)
> Neither of my trunk builds seem to want to display or edit a draft - none of
> the cid: URIs are getting resolved into mailbox: URLs.

You mean with and w/o my patch? Can you attach a sanitized message that you can't edit? This is working for me with various multipart/mixed messages with embedded images. I can try a SeaMonkey build later today.
(In reply to David :Bienvenu from comment #40)
> try server builds here for TB 7 beta + this fix, if anyone wants to try them.
>
> http://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/
> bienvenu@nventure.com-0dc12ff7a441/

I try this binary, and I have similar problem like Joe Sabash in comment 41.
But instead checking HTML, i just double-click on image to get imege editor dialog, and i read image URL there.

steps:

1) I have template message, with some text and image bellow. I open it.
2) I drag'n'drop other image above existing one.
so far so good.
3) I saved as draft [CTRL+S].

effects:
1) existing image URL become mailbox:///xxxxxx/Mail/Local%20Folders/Drafts?number=8396094&part=1.2?part=1.2

2) existing image changed! Somehow now it looks like just dropped image!

As I understand, number in "part=x.x" in URL depends on number and order of inline attachments in message. Inserting image before existing one created new attachment, so number should be changed to 1.3 But I do not know how to get new number (there may be more inserted images, or some images may be deleted).


I got similar problem, when I try to fix this problem in my extension. I can update URL of image, changing it from "Templates?number=xxxxx" to "Drafts?number=yyyy" is easy. But I can't find good way to reliable and properly update part number.

So far, most reliable way to fix this problem in my extension, is to close original composer window after saving as draft, and open new composer with just saved message.


By the way, if I opened just saved draft, both images are correct. Second image indeed get part number 1.3
Attached patch fix multiple url params (obsolete) — Splinter Review
GetUrlForUri in the imap case strips off the url params but not in the local mail case, so it's better to strip that stuff off before calling GetUrlForUri...I'll try to generate at try server build with this version of the patch. I couldn't ever reproduce the issue where the images didn't show or the message didn't load, but maybe this will help.
Attachment #561925 - Attachment is obsolete: true
Attachment #562496 - Flags: superreview?(mbanner)
Attachment #562496 - Flags: review?(neil)
Attachment #561925 - Flags: superreview?(mbanner)
Attachment #561925 - Flags: review?(neil)
new try server build requested with the second fix - http://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/bienvenu@nventure.com-1c5478b47b03

though we seem to have a bit of a backlog for try server builds so it might not show up until tomorrow.
try server builds should be available now.
Status: NEW → ASSIGNED
Some improvement, in that the part=1.x.x is no longer duplicated in the img src html but pre-inserted images still disappear from the composition window after the first save to draft. After that happens, if you attempt to exit compose and use the save as draft option, I get an forever "attaching" message in the status bar.

Incidentally, the first saved draft is usable, and correctly show the composition changes up to that point. 2nd saves and after won't save anything.

Nothing relevant in the error console, but there is this:
 Error: [Exception... "Component returned failure code: 0x80004001 (NS_ERROR_NOT_IMPLEMENTED) [nsIRequest.name]"  nsresult: "0x80004001 (NS_ERROR_NOT_IMPLEMENTED)"  location: "JS frame :: jar:file:///C:/tbtrunktemp/thunderbird/omni.jar!/components/nsLoginManager.js :: <TOP_LEVEL> :: line 293"  data: no]
Source File: jar:file:///C:/tbtrunktemp/thunderbird/omni.jar!/components/nsLoginManager.js
Line: 293

Which I don't think has any significance.
Status: ASSIGNED → NEW
I'm working on this, so I'd like to leave it as assigned...

Thx for trying the new build. I don't think the duplicated part stuff was actually causing a problem, so I'm not shocked that you're still seeing the problem. I wonder if it's an escaping issue in the url. My url starts off like this: mailbox:///C|/Users/David%20Bienvenu, i.e., the | is not escaped, according to the image properties dialog.

As we know from bug 224733, you can get that issue when you copy an image from another message and paste it into the compose window. But I'm not seeing it just from editing a template or draft...
Status: NEW → ASSIGNED
result of my test in new version.

initially, when I open message with image, image URL is:

mailbox:///C|/xxx/Mail/Local%20Folders/Templates?number=84551&part=1.2&filename=fancy.gif

After first save, it become:

mailbox:///C|/xxx/Mail/Local%20Folders/Drafts?number=8531528?part=1.2&filename=fancy.gif

it is quite good, except one error: "?part" should be "&part". character "?" should appear only once.


Now i do second save. URL become:

mailbox:///C|/xxx/Mail/Local%20Folders/Drafts?number=8551120?filename=fancy.gif

As You can see, "part" is missing. URL points to nowhere, image is not shown, and even its attachment is removed (message size is much lower now).



I think problem with second save is caused by error in updating URL in first save, error that "&part" become "?part". To confirm, i start test again, but this time i changed "?part" to "&part" after first save. And it fixed second save, it go correctly.

I look at Your patch, and I think this code is cause:

440 if (restOfUrl.CharAt(0) == '&')
441   restOfUrl.SetCharAt('?', 0);


Second of my problems (image data replaced by copy of new image data) still happen.
I attached image to show my problem.
(In reply to neil@parkwaycc.co.uk from comment #52)
> Neither of my trunk builds seem to want to display or edit a draft - none of
> the cid: URIs are getting resolved into mailbox: URLs.
Sorry, I was using a profile that I'd previously used to test the "Show All Body Parts" feature, which I'd left still turned on. Oops.
Attached patch fix ? and & issue (obsolete) — Splinter Review
this should fix Arivald's issue but not Joe's issue. I don't know how to reproduce his issue. Did you override the default profile directory for the account with the issue? I'm trying to figure out why the '|' in your path is escaped, but it's not escaped for anyone else that I know...
Attachment #562496 - Attachment is obsolete: true
Attachment #562861 - Flags: superreview?(mbanner)
Attachment #562861 - Flags: review?(neil)
Attachment #562496 - Flags: superreview?(mbanner)
Attachment #562496 - Flags: review?(neil)
Joe S, do you think you could send me your prefs.js so I can see if there's anything odd about the way your paths are specified? Thx.
(In reply to David :Bienvenu from comment #63)
> Joe S, do you think you could send me your prefs.js so I can see if there's
> anything odd about the way your paths are specified? Thx.

Sent, but I don't think that "apparent" escaping I'm seeing is relevant.
The difference between the Part=1.2.2 (3 digit) and part=1.2 (two digits) in the drafts folder Plus the ? versus & that Arivald was seeing.
well, you can try http://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/bienvenu@nventure.com-9272677acbce when it's available

postini didn't like your e-mail, but I've retrieved it, and I don't see anything weird in your prefs.
(In reply to David :Bienvenu from comment #65)
> well, you can try
> http://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/
> bienvenu@nventure.com-9272677acbce when it's available
> 
> postini didn't like your e-mail, but I've retrieved it, and I don't see
> anything weird in your prefs.

Yeah, That was sent with a pre- bug 688991 build.
2 copies of html  Now updated.

We'll see what happens on the try build.
(In reply to David :Bienvenu from comment #62)
> I'm trying to figure out why the '|' in your path is
> escaped, but it's not escaped for anyone else that I know...

It is not escaped in image properties dialog. 

It may escaped if You try access HTML of messgage (np: nsIEditor.outputToString() ).
You can access HTML if You select image in message, then select from menu "Insert/HTML"


Also "&amp;part" is used in HTML output, while image properties dialog shows "&part".


So this escaping may be correct, as part of HTML serialization.
(In reply to David :Bienvenu from comment #65)
> well, you can try
> http://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/
> bienvenu@nventure.com-9272677acbce when it's available

Win32 build ends with error, no binary is available.
yeah, try server is broken for windows. I'll try to get some help from the folks that maintain the try servers.

editor html serialization does escape the '|' as I've found from investigating bug 224733.
Comment on attachment 562861 [details] [diff] [review]
fix ? and & issue

>+        nsString restOfUrl(Substring(objURL, restOfUrlIndex, objURL.Length() - restOfUrlIndex));
Isn't the third parameter optional (defaults to the rest of the string)?
Also, it looks odd to have this here instead of later when you use it, although that's possibly due to the next block of code being misplaced.

>+        nsCOMPtr<nsIMsgMessageService> msgService;
>+        rv = GetMessageServiceFromURI(newURI, getter_AddRefs(msgService));
>+        if (NS_FAILED(rv))
>+          continue;
>+        nsCOMPtr<nsIURI> newUrl;
>+        rv = msgService->GetUrlForUri(newURI.get(), getter_AddRefs(newUrl), nsnull);
>+        if (!newUrl)
>+          continue;
>+        nsCString spec;
>+        newUrl->GetSpec(spec);
>+        nsString newSrc;
>+        // mailbox urls will have ?number=xxx; imap urls won't. We need to
>+        // handle both cases because we may be going from a mailbox url to
>+        // and imap url, or vice versa, depending on the original folder,
>+        // and the destination drafts folder.
>+        PRBool specHasQ = (spec.FindChar('?') != kNotFound);
Shouldn't this block belong before the loop?

>+        AppendUTF8toUTF16(spec, newSrc);
Can write this as
NS_ConvertUTF8toUTF16 newSrc(spec);
Attachment #562861 - Flags: review?(neil) → review+
I believe this addresses Neil's comments - I moved a big chunk of code out of the loop, and fixed a PRBool -> bool issue.
Attachment #562861 - Attachment is obsolete: true
Attachment #563574 - Flags: superreview?(mbanner)
Attachment #563574 - Flags: review+
Attachment #562861 - Flags: superreview?(mbanner)
Comment on attachment 563574 [details] [diff] [review]
fix addressing Neil's comments

feel free to punt on the sr if you think just adding a member var to the idl isn't worthy of sr.
Attachment #563574 - Flags: superreview?(mbanner) → superreview+
fixed on trunk - http://hg.mozilla.org/comm-central/rev/637ba790eb12
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 10.0
Testing with tinderbox: Built from http://hg.mozilla.org/mozilla-central/rev/15bfad783467
Summary:
Starting from an original composition with an inserted image works fine
Starting with a draft or a Template works
That's the good news, now the bad

If you start with a locally saved message stored other than draft or template, the image links get busted. This happens also on a reply to a message with an inline image.

The problem seems to be that part=x.x.x format is different in the drafts folder from a reply to a pop3 inbox or a local folder.

When you reply or "edit as new" a locally stored image the format is 
Part=1.2.2 Part=1.2.3 Part=1.2.4 sequentially up for each inline image.
When saved in the draft folder the parts are numbered 1.2, 1.3, 1.4 etc.

So now after the second save, we can't find Part=1.2.2 any longer 'cause it resides there as Part=1.2

End result:
Original compositions work
Drafts and Templates work

Edit as new to previously saved messages are broken.
Replies to messages with inline images can't be saved.

I don't have IMAP set up here, so can't test there.
by locally saved message, do you mean as a .eml file on disk? If so, there are already existing bugs about that not behaving correctly w.r.t. embedded images. I just haven't gotten to those yet.
Do not look at other bugs, problem (In reply to Joe Sabash from comment #73)
> Testing with tinderbox: Built from
> http://hg.mozilla.org/mozilla-central/rev/15bfad783467

> End result:
> Drafts and Templates work

Not really. There is still one problem, related to "&part=x.x": if You insert image *before* any already existing image, all images after inserted one will be corrupted. See my image attached to comment #60.

Simply this path does nothing with "&part=x.x" fragment of image URL, while it should somehow discower new part number for every saved image. This number seems to be bound to order of MIME parts in encoded message. 
Fixing "&part=x.x" will also fix problem from comment #73.


I have idea how it may be fixed: instead of doing set of small updates to image URLs, maybe it will be possible to stream out HTML of just saved message, then just replace whole HTML of edited messge with HTML of saved message?


IMHO setting Resolution to FIXED was too early, not all is fixed yet.
(In reply to Arivald from comment #75)

> Not really. There is still one problem, related to "&part=x.x": if You
> insert image *before* any already existing image, all images after inserted
> one will be corrupted. See my image attached to comment #60.

I'll file a new bug for this issue. It was an existing issue, right? I don't the patch in this bug helped or hurt. I haven't been able to reproduce the issue, personally, but I believe you that we don't handle part renumbering.
Yes, it is already existing problem. But before this path, TB simply throw error, and do not save message. With thia path, it do save message, with broken images.

So, in a way, current state is worse.
Bug 692875 addresses the part numbering issue.
Similar issues still present: Bug 501298, Bug 754655
Same problem, when sending a email with long time tempo an pasted images.
--> Worse problem ! Last week, the mail has been sent with another image than the one i had pasted (used screenpresso to copy & paste)
You need to log in before you can comment on or make changes to this bug.