320102 - File - Save As - Template menu item does nothing for an existing message in Local folders

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[[:Aleksej]]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8) Gecko/20051111 Firefox/1.5
Build Identifier: 2005120115

Tried restarting Fx some times, checked the folder name in account settings.

Reproducible: Always

Steps to Reproduce:
1. Select an existing message (I did in Drafts and Inbox folders in Local Folders)
2. Choose File-Save as-Template
3. Look into the Local Folders - Templates folder (if it exists)

Actual Results:  
Nothing appeared in the Local Folders - Templates folder.

Expected Results:  
The message should appear in the folder as a template.

Debian GNU/Linux testing (Etch)

Comment 1

[Bob Clary [:bc] (inactive)]

This worked on WinXP when using a templates folder on the imap server and for one in the local folders. IMAP required (I think) and Local Folders required that the templates folder be created first. IMAP was slow to show the templates but they did work.

So this bug may be Linux only.

Comment 2

[Adam Guthrie]

Aleksej, are you using IMAP? If so, this is a dupe of bug 216262.

Comment 3

[[:Aleksej]]

No IMAP. Only one POP3 account, and then one NNTP account.

Comment 4

[Magnus Melin [:mkmelin]]

->new Seeing this to on 1.5/linux, I assume you only mean mesages that already exist, not when you create a new message. Doesn't work in tb 1.0.x either. 

Comment 5

[[:Aleksej]]

Can't recheck in 1.5 now, but in 1.0, yes, and in 1.5 AFAIR yes.

Comment 6

[Magnus Melin [:mkmelin]]

I can confirm this is linux only. On windows 1.5rc it works fine.

Comment 7

[Mike Cowperthwaite]

Under Windows 2000, 1.5.0.2, this bug is not occurring; but with 2a1-0523, 
3a1-0529, I see   Save As | Template   failing for all messages, in particular those in a POP Inbox.  (xref bug 176567 comment 8 -- In Seamonkey 1.5-0518, 
I'm not seeing it.) 

Comment 8

[Álvaro González]

Under Windows XP Professional SP2 Spanish I have the same problem with Thunderbird 1.5.0.5 (20060719), es-ES localized build. When I install the program the "Templates" folder doesn't exist. The "Save as template" option does nothing.

If I first chose "Edit as new", then "Save as template", the "Templates" folder is finally created. However, it remains empty.

Comment 9

[[:Aleksej]]

Still there with version 2 beta 1 (20061121)/Linux

Comment 10

[[:Aleksej]]

Also on version 2 beta 1 (20061121)

Comment 11

[[:Aleksej]]

Also on version 2 beta 1 (20061121)

Comment 12

[[:Aleksej]]

And on Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.2pre) Gecko/20070216 Thunderbird/2.0pre ID:2007021603

Comment 13

[[:Aleksej]]

And version 2.0.0.4pre (20070514) and version 3.0a1 (20070511)

Comment 14

[Boying Lu]

The root cause is that on Linux (actually UNIX) each line is ended with '\n' instead of '\r\n' and IMAP server may not accept this obsolete syntax according to the RFC2822.

On MS Windows, each line is ended by '\r\n', so there is no such issue on MS Windows platform.

Comment 15

[Boying Lu]

Attachment: replace standalone '\n' with '\r\n'

Comment 16

[[:Aleksej]]

(In reply to comment #14)
> The root cause is that on Linux (actually UNIX) each line is ended with '\n'
> instead of '\r\n' and IMAP server 

This bug doesn't need IMAP accounts to happen.

> may not accept this obsolete syntax according to the RFC2822.

[off-topic]o_O XT performance issues in 2001? Or just poorly working compatibility?[/off-topic]

Comment 17

[Scott MacGregor]

Comment on attachment 271008 [details] [diff] [review]
replace standalone '\n' with '\r\n' 

bouncing over to David.

Comment 18

[Boying Lu]

(In reply to comment #16)
> (In reply to comment #14)
> > The root cause is that on Linux (actually UNIX) each line is ended with '\n'
> > instead of '\r\n' and IMAP server 
> 
> This bug doesn't need IMAP accounts to happen.
> 
> > may not accept this obsolete syntax according to the RFC2822.
> 
> [off-topic]o_O XT performance issues in 2001? Or just poorly working
> compatibility?[/off-topic]
> 

Can you tell me how to re-produce the bug without an IMAP account? Thanks

Comment 19

[David :Bienvenu]

Comment on attachment 271008 [details] [diff] [review]
replace standalone '\n' with '\r\n' 

yuck, I'd much rather fix this in memory, instead of on disk, i.e., as we're uploading the lines.

Comment 20

[[:Aleksej]]

(In reply to comment #18)
> Can you tell me how to re-produce the bug without an IMAP account? Thanks

Not exclusive, just the StR I've just tried (version 3.0a1pre (2007070803) Linux):
1. Create a new profile.
2. Create a POP e-mail account
3. Create a message, choose "Send later", close
4. Create "Inbox" under Local folders, move the message there
5. Select the message
6. File→Save As→Template
Actual results: none

Comment 21

[Boying Lu]

(In reply to comment #19)
> (From update of attachment 271008 [details] [diff] [review])
> yuck, I'd much rather fix this in memory, instead of on disk, i.e., as we're
> uploading the lines.
> 
I think we can't do this in memory because the mail can be very large

Comment 22

[David :Bienvenu]

You can do it a line or buffer at a time...I'll try to get to this in a bit.

Comment 23

[Boying Lu]

Attachment: Not using temporary file to do the translation

Comment 24

[Boying Lu]

Attachment: Not using temporary file to do the translation

Comment 25

[David :Bienvenu]

I forgot this is imap, where we need to know how big the message is before we start appending it.  It seems like the save as template code should make sure we have CRLF line endings, just like the FCC (copy to sent folder) code does. Let me go see why that code works, assuming it does.

Comment 26

[David :Bienvenu]

What happens without your patch if you edit a message as new, and then send it, with an imap fcc folder? Does the message get correctly appended to the imap sent folder?

The code that I was thinking about that fixes the line endings for copying messages to an imap folder is the code that gets called when you copy a local message to an imap folder, not a template or fcc msg.

Comment 27

[David :Bienvenu]

Attachment: [checked in]possible alternate fix

Brian, does this fix work for you? If I'm understanding the problem correctly, it's only happening with the plain text compose window (not the html compose window) which is why it must not be that common. If so, the simple fix is to make the plain text compose window always generate CRLF's, like we're already doing for html.

Comment 28

[Boying Lu]

(In reply to comment #27)
> Created an attachment (id=271735) [details]
> possible alternate fix
> 
> Brian, does this fix work for you? If I'm understanding the problem correctly,
> it's only happening with the plain text compose window (not the html compose
> window) which is why it must not be that common. If so, the simple fix is to
> make the plain text compose window always generate CRLF's, like we're already
> doing for html.
> 

I tried the patch, but it doesn't work for me. I think it's unrelated to composing.

The following are steps I used to reproduce the bug:
1. start thunderbird with an IMAP account ( I use Sun IMAP server)
2. select an e-mail in INBOX
3. select File->Save As -> Template

expected result:
The selected e-mail should be save in Template folder

actual result:
The mail is not saved in the Template folder.

Comment 29

[David :Bienvenu]

oh, duh. Does editing a plain text message as new, and saving that as an imap template cause the same problem?

Comment 30

[David :Bienvenu]

I suspect the problem for the file save as template case (is this an imap message you're saving?) is that the message is in the memory cache, and thus we don't realize that we should be converting to canonical line endings, which is how, in general, we avoid this line ending problem in the first place - see nsIMsgMessageUrl.canonicalLineEnding, which gets set to true when we want the saving code to end lines with CRLF.

I tried saving a local message as an imap template, and that failed badly, trunk and 2.0 branch, which is a separate issue.

Comment 31

[David :Bienvenu]

Attachment: fix saving imap message as a template

Brian, if you could indulge me, and see if this fixes the problem you were seeing saving an imap message as a template, that would be helpful.

What this patch does is prevent save as template from going through the memory cache, if you're on a platform with a one char line terminator.

Comment 32

[Boying Lu]

(In reply to comment #31)
> Created an attachment (id=271950) [details]
> fix saving imap message as a template
> 
> Brian, if you could indulge me, and see if this fixes the problem you were
> seeing saving an imap message as a template, that would be helpful.
> 

David, the patch doesn't work. The following is what I did:
1. build thunderbird with the both patches
2. start TB
3. compose a new e-mail and send it to myself
4. select the new e-mail in the inbox
5. save it as a template 

I think the root cause is that each line sent to the IMAP server in "append" command (the command used when saving as a template) should be ended with CRLF no matter what platform TB runs on

Comment 33

[David :Bienvenu]

Attachment: [checked in]fix saving imap message as draft

OK, this fixed it for me, and is much nicer than my previous patch, in that it still allows us to use the disk or memory cache. This replaces the previous patch, though I think the save as draft from the compose window patch is still needed for that case.

Comment 34

[Boying Lu]

David, I tried your patch, it doesn't work with my IMAP account.

The following is the log I dumped by setting NSPR_LOG_MODULES=IMAP:5

10[9738198]: 9738fe8:mail-apac.sun.com:S-Templates:SendData: 9 append "Templates" (\Seen \Draft) {1279+}^M
10[9738198]: 9738fe8:mail-apac.sun.com:S-Templates:SendData: Return-path: <[e-mail]>
Received: from fe-apac-05.sun.com ([192.18.19.176])
 by sedge2-mail1.singapore.sun.com
 (Sun Java System Messaging Server 6.2-6.01 (built Apr  3 2006))
 with ESMTP id <[e-mail]> for
 [e-mail]; Fri, 13 Jul 2007 11:22:13 +0800 (SGT)
Received: from conversion-daemon.mail-apac.sun.com by mail-apac.sun.com
10[9738198]: 9738fe8:mail-apac.sun.com:S-Templates:SendData: ^M
10[9738198]: ReadNextLine [stream=8a35844 nb=37 needmore=0]
10[9738198]: 9738fe8:mail-apac.sun.com:S-Templates:CreateNewLineFromSocket: 9 NO Message contains bare newlines^M

I think each line sent to the IMAP server should be ended with CRLF (i.e ^M)

Comment 35

[David :Bienvenu]

If you set a breakpoint in nsMsgSaveAsListener::OnDataAvailable, are we adding a CRLF newline, or a LF newline?

Comment 36

[Boying Lu]

(In reply to comment #35)
> If you set a breakpoint in nsMsgSaveAsListener::OnDataAvailable, are we adding
> a CRLF newline, or a LF newline?
> 

David, sorry, I missed a line in your patch. I re-added the patch and it works for me now.

Comment 37

[David :Bienvenu]

Comment on attachment 271630 [details] [diff] [review]
Not using temporary file to do the translation 

marking obsolete

Comment 38

[David :Bienvenu]

Comment on attachment 272053 [details] [diff] [review]
[checked in]fix saving imap message as draft

Thx for diagnosing this problem and testing my patch, Brian. 

Someday we might go with your approach but I'd like to avoid the extra overhead if possible :-)

Comment 39

[David :Bienvenu]

Comment on attachment 271735 [details] [diff] [review]
[checked in]possible alternate fix

want to bake this on the trunk, but we should consider this for the next .0x release.

Comment 40

[David :Bienvenu]

Comment on attachment 272053 [details] [diff] [review]
[checked in]fix saving imap message as draft

want to let this bake on trunk, but getting on radar for 2.0.0.6

Comment 41

[David :Bienvenu]

fixed on trunk.

Comment 42

[[:Aleksej]]

No luck with version 3.0a1pre (2007072003) on Linux.
Note that the problem is with saving using the 3-pane window menu.

Comment 43

[David :Bienvenu]

Aleksej, my guess is that your problem is unrelated to the issues Brian and I have been working on. I suspect your problem has something to do with us not knowing where your templates folder is. Do you have multiple accounts? Have you looked in all of them? Are you using the global inbox? Does anything appear on the error console when you do this?

Comment 44

[Boying Lu]

Aleksej, which protocol do you use IMAP or POP3?

If you use IMAP, can you do the following for us?
1. open an xterm
2. export NSPR_LOG_FILE=/tmp/imap.log
3. export NSPR_LOG_MODULES=IMAP:5
4. /usr/bin/thunderbird  
5. until you see the problem, quit from thunderbird
6. attach the imap.log here

Thanks

Comment 45

[[:Aleksej]]

version 3.0a1pre (2007072203)

POP3. The log file remains empty.

The bug happens:

1. create a POP3 account with Global Inbox
2. start a new message and choose Send Later (the message is in Unsent now)
3. create Inbox folder if it doesn't exist
(optionally) create Templates folder
4. Move the message from step 2 into Inbox
5. Select Inbox, select the message in the message list
6. File → Save As → Template

Results:
If the Templates folder exists (after restart it will anyway), it is empty.


Oh. It looks like it doesn't work in Local folders. It works if I choose a separate folder tree for the account, and save the message there. It doesn't work for the same message under Local folders then, too.

Comment 46

[Boying Lu]

Aleksej, can you set NSPR_LOG_MODULES=POP3:5 and try again? Thanks

Comment 47

[[:Aleksej]]

No change:
fxtest@chaos:/home/fb/3/mz/tb/trunk/tr2007072303$ env|grep NSPR
NSPR_LOG_MODULES=POP3:5
NSPR_LOG_FILE=/dev/shm/imap.log
fxtest@chaos:/home/fb/3/mz/tb/trunk/tr2007072303$ ls -al /dev/shm/imap.log
-rw-r--r-- 1 fxtest fxtest 0 2007-07-24 13:15 /dev/shm/imap.log

Comment 48

[David :Bienvenu]

Yes, I'm pretty sure Aleksej's problem has nothing to do with pop3 or imap, and just has to do with us not knowing where the templates folder is. I don't think it has anything to do with the message your saving either - it's strictly an identity/profile/account issue.

Comment 49

[David :Bienvenu]

function SaveAsTemplate(uri, folder)
{
  if (uri) {
    var identity = getIdentityForServer(folder.server);
    messenger.saveAs(uri, false, identity, null);
  }
}

I think this code simply doesn't work for the global inbox/local folders. It's the code that's invoked for file | save as template, when a message is selected. I'll dig a little more.

Comment 50

[David :Bienvenu]

Attachment: [checked in]fix trunk-only regression saving a message as imap template

this is trunk-only fallout from nsIFileSpec - we need to clone the file to get the file size updated. Also, there's a little bit of fallout from the changes to save attachments automatically. This only affects selecting a local message in the thread pane and picking save as template when your template folder is IMAP.

Comment 51

[David :Bienvenu]

Attachment: [checked in]fix identity problem

finally, deal with the case that we can't get the identity for the message from the account key. I made a helper routine to get the best identity for a msg hdr, and made that code, as a fallback, iterate over all the identities, if it couldn't find an identity for the server.

Comment 52

[[:Aleksej]]

Works with version 3.0a1pre (2007073103) now.
- The Templates folder is created after restart, not right after saving the template. The created folder then contains the template.
- In case the account has its own separate tree, the Templates folder is created under the account tree, not Local folders.

Comment 53

[David :Bienvenu]

marking fixed

Comment 54

[Wayne Mery (:wsmwk)]

can someone test bug 331876?
it might be fixed as well, but my local folder settings are host ATM.

Comment 55

[David :Bienvenu]

patch landed on 1.8.1.7 branch

Comment 56

[Carsten Book [:Tomcat]]

verified 1.8.1.8 using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.8pre) Gecko/20070927 Thunderbird/2.0.0.7pre Mnenhy/0.7.5.666 ID:2007092703

Comment 57

[Scott MacGregor]

Comment on attachment 271735 [details] [diff] [review]
[checked in]possible alternate fix

David, is this approval request still valid or did this get addressed by subsequent patches in this bug which landed on the 1.8.1.8 branch already.

Comment 58

[David :Bienvenu]

Comment on attachment 271735 [details] [diff] [review]
[checked in]possible alternate fix

no, I think this request isn't valid anymore...