In todays build (10/15), replying to a message using html always fails. The compose window comes up for me but the editor window is always blank. Actually it's worse than blank. It actually captures whatever content was underneath the compose window. Lisa, can we look into adding a reply to test to our mailnews smoketest stuff? I think we missed this today and the tree opened. I'm trying to email yesterday's hook now....

Interesting. Just checked with Laurel - she can reply in html using today's release (2nd respin) builds.

Rich, if I back out your changes to nsMimeXULEmitter.cpp, I at least get a blank editor window instead of one that shows garbage (grabbing content from underneath the window). So this is a starting point. Seth sees the same problem on Linux.

My XUL emitter change it not the problem here. In either case, you are not getting an Editor instantiated, the "garbage pixels" is just a random behavior. - rhp

Are you sure Rich? Why is it with your change, I get garbage pixels...without your change, I get a blank compose window. Something is fishy somewhere that backing that out made a difference. Maybe it's influencing a race condition somewhere. Obviously backing out your change doesn't fix the problem as I still get a blank compose window, it just isn't garbage bits.

The XUL emitter change is the one where I'm doing the nsEscapeHTML() call now, right? - rhp

Hey Rich, I was just stepping through your change to nsMimeXULEmitter and I saw a small optimization that would cut down on the number of strings you allocate. You had: nsString workString(curName); char *tName; workString.Trim("\""); tName = workString.ToNewCString(); workName = nsEscapeHTML(tName); if (workName) { PR_FREEIF(tName); } else { workName = tName; } } I think you'll reduce at least one copy of the string if you use a nsAutoCString here instead and pass that directly to nsEscapeHTML instead of converting to a nsString then having it convert back to a char * just to pass into nsEscapeHTML. So the code snippet could look like: nsCAutoString workString(curName); char *tName; workString.Trim("\""); workName = nsEscapeHTML(workString); if (!workName) { workName = workString; } This saves an extra allocation/free and it shortens the code too. Just my little post code reviewer tip... Anyway, I'm stepping through the debugger now...

IF you start typing into the body, you can see xxx wrote: but everything is displayed in double, event what your are typing (this part should be an Editor problem). When I output the content as HTML or XIF, the original body is missing!

Rich, I don't know why backing out that file made me get a legit blank screen. I set a breakpoint and that code doesn't even get called, when replying to a message. So it's definetly something else.

Well looks like nothing changed in compose yesterday and hardly anything changed in mime. So it looks like it something outside of mailnews that caused the regression. I'm scanning editor changes and webshell changes now.

the generated file (nscomp.html) that we load into ender seems ok for me. By ok I mean is like it always been. But when I load it into the browser, nothing is displayed. same for ender. I will attach the file...

The attached file load correctly in 4.x. Seems to be a layout problem!

this is a layout bug, in that the browser cannot load this document either. the HTML in the test case is aweful, but Nav can lay it out so we should too in quirks mode. changing product to parser. Marking P1, blocker, regression.

I'd like to help narrow the problem down for the layout guys a bit more. We know that sometime yesterday layout lost the ability to load our reply-to message files. But we do know that we can view messages in the message pane. Rich/Jean-Francois, can you list here for rickg's benefit how the html content differs between displaying a message and replying to a message. i.e. Doesn't compose insert some of its own information into the file on top of the html generated by the mime parser?

We just take the whole original body (HTML or plain text) and we add arround the following. <br><br>chris kritzer test3 wrote:<br><BLOCKQUOTE TYPE=CITE><html> ...then we put the original body here... </html></BLOCKQUOTE> The result is that we can have several HTML tag! When we quote an plain text body, we also add a <PRE> tag around the body.

Rick, just a little extra info to help narrow this down. Both viewer and apprunner fail to load this page for me. On a build from 10/14 (morning), both apprunner and viewer can load Jean-Francois's attachement.

Got it!! On a hunch, I backed on Radha's webshell changes because they involved altering document load notifications. Sure enough that did it. replying to messages now works again. I'm going to re-assign this bug back to me and take rickg off of this since it really doesn't involve him. I'll work with radha to figure out how her stuff broke us.

Re-assigning to me, cc'ing radha since her changes in the webshell broke this. I'll try to work with her to figure out where the breakage is coming from.

Radha, in your changes to nsWebShell::DoLoadURL yesterday, you introduced a new code path that broke things I think. The code used to do the following: 1) If the url has a dest anchor and a pres context then do some stuff andcall OnEndDocumentLoad and return. 2) Or, if aType == nsISessionHistory::LOAD_HISTORY then do some stuff and call OnEndDocumentLoad 3) If neither of these cases is true, then just fall through and load the url. After your change the code flow looks like: 1) If url has a destination anchor do some stuff 2) if aType == nsISessionHistory::LOAD_HISTORY do some stuff. 3) call OnEndDocumentLoad and return Notice that now, even if (1) or (2) isn't true, we still call OnEndDocumentLoad and return. This is what broke mailnews today. I'm surprised we didn't get caught elsewhere. Anway, there are two options as I see them 1) We can back out the change to nsWebShell involving ::DoLoadURL. It looked to me that the change was having OnendDocumentLoad and the return call after these two if clauses like I illustrated above. 2) Or if you want, I've modified the methods such that it looks more like it used to (same effect as backing it out) where we call OnEndDocumentLoad and return inside each of the if clauses so we don't do that for all urls we try to load. This is a P1 stopper and we really need to get this fixed before Monday morning when the QA builds go off. So if you can look at this and let me know how you'd like me to proceed before then, I'd appreciate it.

Adding Leaf to the cc list. Leaf, I haven't been ble to get a hold of radha yet to figure out how she wants to handle this bug. I have a fix in my tree but would need her to review it. We certainly want to keep the tree closed tomorrow mornig if she doesn't get a chance to look at it before then before we open the tree up. Thanks.

I'm looking at my changes. I'll reply soon. Can you send me your patches?

Radha, I'm attaching my patch to this bug report now. Although to my eye, my changes make the function look pretty much like it did before you changed it. So it could be just as safe to back out the changes to nsWebShell::DoLoadURL.

scott, I have attached patches which moves the OnStartDocumentLoad() also inside the if so that it affects only the browser. Please try my changes and let me know. I 'll check it in. Thanks, Radha

please don't checkin until the tree opens, or until we decide to respin for this fix.

Thanks Radha, I've actually been trying to save and apply this patch on my windows box but am having line feed termination problems. It's really strange. I'm playing with it now.

Radha, did you attach the patch as a text file? I'm having difficulaties applying the patch on both my linux and windows machines. Running the actual patch routine is failing.

Can you email me the patch instead? I just can't seem to get this patch to work on either machine for me. Thanks.

Radha, can you diff the file again but this time pipe it directly into a file? i.e. do cvs diff -c > myFilePatch.txt I'm guessing that you selected the diff code in the console window and tried to paste that into a file which doesn't work. If you try to select the patch text and copy/paste it, the patch comes out bogus because extra line returns are entered in the pasted text based on how wide your terminal window was when you pasted the text. Thanks.

I have checked in the patches

Radha checked in the fix this afternoon. Marking as fixed.

This is working for me using 1999-10-19-08m11 commercial builds. However, since I didn't have this extreme problem originally, I'm going to ask someone else to confirm it...mscott?

Sure thing Laural. Marking fixed on 10/19 build...