Closed Bug 56129 Opened 24 years ago Closed 24 years ago

Save As Draft and Save to Sent folder are broken

Categories

(MailNews Core :: Composition, defect, P3)

All
Windows NT
defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: phil, Assigned: jefft)

References

Details

(Keywords: regression, Whiteboard: [rtm++])

Attachments

(1 file)

Using 2000-10-11-08 RTM branch build on NT

1. Compose a mail message
2. Save as draft

Expected: draft saved
Actual: error msg: Send was successful but saving to Sent folder failed. Draft
not saved
rtm keyword
Keywords: rtm
Status: NEW → ASSIGNED
Target Milestone: --- → M18
QA Contact: esther → pmock
Changing platform to all... :(

This problem is reproducible on today builds on all platforms.
 win32 commercial seamonkey build 2000-101109-mn6 installed on P500 Win98
 linux commercial seamonkey build 2000-101109-mn6 installed on P200 RedHat 6.2
 macos commercial seamonkey build 2000-101110-mn6 installed on G3/400 OS 9.04
Hardware: PC → All
Adding regression keyword.  Can anyone figure out what recent change broke this?
 (adding some people to the CC who may be able to help answer that question)
Keywords: regression
Whiteboard: [rtm need info]
For me, I can send mail, but can't FCC either. Which makes sense since I bet
saving to Sent and saving to Drafts use some of the same code. Resummarizing.

BTW, both my Drafts and Sent folder are on the IMAP server.
Summary: Save As Draft is broken → Save As Draft and Save to Sent folder are broken
I'll take a look as soon my build is done...
 It's failing here:

nsMsgComposeAndSend::MimeDoFCC line 4091

   if (!inputFile.readline(ibuffer, ibuffer_size))
    {
      status = NS_ERROR_FAILURE;
      goto FAIL;
    }

this status then gets returned and we bail on our current action.  Interestingly
enough, we have a default error case that we hit in

nsMsgComposeAndSend::NotifyListenersOnStopCopy that puts up the copy to sent
message folder failed alert. This probably shouldn't happen in the Save as Draft
case.
*** Bug 56108 has been marked as a duplicate of this bug. ***
I am working on it...
cc'ing rhp in case he has an idea.
Couple problems found here:

1) nsFileStream::readline() falsefully returns bufferNotLarge enough if it could 
find "\r\n" token, another thing the implementation of readline is really time 
consuming

2) for some reason when writing out the final temp rfc822 message file we no 
longer writing our "\r\n" for the last line.
I meant "could NOT find \r\n".
nsMsgComposeAndSend::MimeDoFcc() is what is failing.

I'll continue to debug.  (my dollar is on the noxif stuff.)
nsInputStream::read() has a false login there. It set_at_eof(PR_TRUE) only when
the mInputStream->Read() result is equal to 0 bytes. In the case of reading the
last remaining bytes of the file we failed to set the eof flag.
Whiteboard: [rtm need info] → [rtm need info] has a safe fix .. need reviews
R=ducarroz
reassign to jeff
Assignee: ducarroz → jefft
Status: ASSIGNED → NEW
Whiteboard: [rtm need info] has a safe fix .. need reviews → [rtm need info] has a safe fix .. need super review
r=sspitzer.

the fix looks good, and fixes the problem for me.

we should get more eyes on it.

who owns the input stream code these days?  (warren?)
I thought I had already cc'ed warren on this. I think bugzilla dropped my
comments =(.

I'd like to wait until Warren does a code review on this before we check it in.
He knows the nsInputStream code best of all.
dougt? I think.
Does anyone know why this just started happening? Would be good to pinpoint the 
cause, before changing something so fundamental as the streaming code.
the noxif landing appears to have exposed this problem.  but the noxif landing
did not cause the bug.

before the noxif landing, the file was terminated with "\r\n"
after the noxif landing, the file was not terminated with "\r\n" which exposed a
problem with read / readline.

mail news uses the input stream code, and the failures bubble up and cause weird
bugs.
Adding jst to cc list. jst or jfrancis, can you review the patch please? We 
should definitely understand why this broke, and what else could be broken.
I'm not the right person to review this.  I looked at it and it lokos ok to me, 
but we need someone else.  
mscott: Actually I don't know much about this code. It's the old mcmullen stuff 
that's really obsolete now.
*** Bug 56336 has been marked as a duplicate of this bug. ***
Okay if there's no owner then I'll super review it because the change makes
sense and looks correct to me. sr=mscott
marking rtm+, since we have r= and sr=.

Whiteboard: [rtm need info] has a safe fix .. need super review → [rtm+]
Marking rtm-double-plus. Please land on branch asap.

I spoke to jst, and he doesn't think he's the guy to be reviewing this (re:
Phil's comment above).
...but he did offer to help with the landing this on the branch if no one is
around tonight to do so.
Whiteboard: [rtm+] → [rtm++]
I checked in the attached patch so that this problem is fixed in tomorrows builds.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Verified as fixed on branch builds of win32, linux, and macos using the builds:
 win32 commercial seamonkey build 2000-101810-mn6 installed on P500 Win98
 linux commercial seamonkey build 2000-101809-mn6 installed on P200 RedHat 6.2
 macos commercial seamonkey build 2000-101808-mn6 installed on G3/400 OS 9.04
Keywords: vtrunk
Verified Fixed on trunk builds
linux 101808 RedHat 6.2
win32 101804 NT 4
mac 101804 Mac OS9
Setting bug to Verified and removing vtrunk keyword
Status: RESOLVED → VERIFIED
Keywords: vtrunk
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: