Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.4) Gecko/20010913 Write a reply mail where the quoted text is longer than wrap column. Use Options/Rewrap. This will also apply to quoted text. So we have long quoted lines followed by short unquoted lines. This is evil. pi

This would definitely be evil. The whole point of rewrap is that it's supposed to do the right thing with quotes. But I can't reproduce this -- it works for me. Can you give some more details on exactly what the message looks like (and does it come from text or html) and exactly how you get into this mode?

OK, let me elaborate on the details. I use only the text editor. I answer to an e-mail or posting (no difference here). The quoted lines (with the quotation mark) are longer than the 70 characters I set as line length. For some (unclear) reason the line breaking for my text does no longer work and lines become very long. So I press Rewrap. Now my text is broken correctly, but also the quotes are broken to fit 70 charaters. Please let me know which details I could provide to clarify the problem. In the meantime I switched to the new version, the problem is still around: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.5) Gecko/20011012 pi

Wait, I'm really confused. Rewrap is for quoted text -- that's the whole point of it, to rewrap quoted lines that are too long or short (e.g. from people who send out mail with wrapping off entirely, or with their line length set too long). There's no need to rewrap the text you type; the edit window keeps it wrapped no matter what. If the new lines you type somehow aren't wrapping, then that's a different bug (on plaintext editing), not a rewrap bug. (There is an existing bug, assigned to jfrancis, where if you break a quoted section and start typing, or click immediately underneath quoted text and start typing, your lines won't be broken. That's the only case I know of where your typed lines don't break correctly, though.)

Sorry, I did not make myself not clear enough (not being a native speaker can be tough;-). > Rewrap is for quoted text -- that's the whole point of it, > to rewrap quoted lines that are too long or short Well, that completely fails. See screenshots before and after which I'll post in a minute. You have a comb-like structure (like Outbreak Excess produces). > There's no need to rewrap the text you type; In that case there is. Anyways, that text is wrapped using the command (as I would expect), as is also seen on the screenshot. > If the new lines you type somehow aren't wrapping, then > that's a different bug (on plaintext editing), not a rewrap bug. Right, that was not the point of my report. If you have the number for that bug, this would be helpful. Also note a third thing in the screenshots. The text doubled. I have never seen that before. But between taking the shots I have done nothing but applying options / rewrap. pi

That really shouldn't be happening (and it doesn't happen for me, but I just discovered that I do have quotesPreformatted set to the wrong value). Can you check your prefs (prefs.js in your profile directory) and see if there's a line there setting a pref called editor.quotesPreformatted ? Also, is your mailnews.wraplength set to anything other than the default? (I'm trying to test this myself with various values of editor.quotesPreformatted, but apparently plaintext quoting is broken in the build I have, so I'll have to wait a few days 'til that gets fixed.) If quotesInPre were being handled backward, that would explain this, but the code looks right ...

> Can you check your prefs and see if there's a line there > setting a pref called editor.quotesPreformatted ? No such entry. > Also, is your mailnews.wraplength set to anything other than the default? I don't know the default, but it is 70 for me. pi

I'm finally able to reproduce this on my own build, and debug it. The problem was that the algorithm sometimes made lines one word too long. It looks at a line and decides whether to break without looking at the length of the first word on the next line; if that word is too long, we can exceed the line length. I have a patch for that, which should fix your problem. I also fixed some debug prints that were made longer than they should be in one of the string API reorgs, and noticed some places where we were getting double spaces inserted into the line, so I fixed those too. Patch coming. Looking for review and sr. Kathy, any chance you can review this? Or suggest someone else? Kin, can you sr?

Comment on attachment 56197 [details] [diff] [review] Patch, should fix the problem This may just be my paranoia, but... + // Skip over initial spaces: + while (nsCRT::IsAsciiSpace(tString[posInString])) + ++posInString; Any chance this could exceed nextNewLine or length? Do we need some code here that checks for that condition, and perhaps does the right thing in terms of the outer loop? + // Trim trailing spaces: + PRInt32 lastRealChar = nextNewline; + while (nsCRT::IsAsciiSpace(tString[lastRealChar-1])) + --lastRealChar; Same here, is there any possibility that lastRealChar can become zero resulting in a -1 lookup?

I think it's unlikely (would only happen if the line was all spaces, but then going back you'd hit the newline at the end of the previous line) but it's possible, and the checks are a good idea in any case, and don't cost much. New patch on the way, with checks (which also converts more of those debug statements that unnecessarily used intermediate variables) but otherwise it's the same.

Comment on attachment 56276 [details] [diff] [review] Patch with additional checks for overruns sr=kin@netscape.com

I'll try to get this reviewed and in for 0.9.6.

Comment on attachment 56276 [details] [diff] [review] Patch with additional checks for overruns r=brade

Sorry -- I caught a bad flu and wasn't able to get this in in time for the milestone. It'll go into the next open tree I see, though.

Fixed.

Boris, can you please help verify this one? thanks..let us know if it looks fixed on your end...

I will check, but my admin won't install a version before 0.9.6. pi

Sorry, it is still broken. I'll again post before/after shots. Please ignore the rectangle in the upper right corner of the second image. pi

Akkana says that the milestone 0.9.6 was missed and this was checked in afterwards, this means the fix will be in the latest nightly builds or you'll have to wait till 0.9.7 to verify if your admin has a policy of only installing releases.

Marking fixed for now; please reopen if it doesn't work in 0.9.7.

Verifying on build 2001121803. If anyone is still seeing this, please reopen.

Sorry for the long silence. I did not have 0.9.7 to verify, now I have 0.9.8. The fix is not completely correct. My test looked like this (hopefully the lines stay correct here): >> 012345678 012345678 012345678 012345678 012345678 012345678 012345678 >> 012345678 012345678 012345678 012345678 012345678 012345678 012345678 >> 012345678 012345678 012345678 012345678 012345678 012345678 012345678 >> 012345678 012345678 012345678 012345678 012345678 012345678 012345678 >> 012345678 012345678 012345678 012345678 012345678 012345678 012345678 >> 012345678 012345678 012345678 012345678 012345678 012345678 012345678 >> 012345678 012345678 012345678 012345678 012345678 012345678 012345678 I created a reply and applied Rewrap, in the editor it looked like: >>> 012345678 012345678 012345678 012345678 012345678 012345678 >>> 012345678 012345678 012345678 012345678 012345678 012345678 >>> 012345678 012345678 012345678 012345678 012345678 012345678 >>> 012345678 012345678 012345678 012345678 012345678 012345678 >>> 012345678 012345678 012345678 012345678 012345678 012345678 >>> 012345678 012345678 012345678 012345678 012345678 012345678 >>> 012345678 012345678 012345678 012345678 012345678 012345678 012345678 >>> 012345678 012345678 012345678 012345678 012345678 012345678 The mail actually looked like this when received: >>> 012345678 012345678 012345678 012345678 012345678 012345678 >>> 012345678 012345678 012345678 012345678 012345678 012345678 >>> 012345678 012345678 012345678 012345678 012345678 012345678 >>> 012345678 012345678 012345678 012345678 012345678 012345678 >>> 012345678 012345678 012345678 012345678 012345678 012345678 >>> 012345678 012345678 012345678 012345678 012345678 012345678 >>> 012345678 012345678 012345678 012345678 012345678 012345678 012345678 >>> 012345678 012345678 012345678 012345678 012345678 012345678 Not really correct. Reopen or different bug? pi

Different bug. It's related to some existing ones, but I don't think anything covers quite this case. Please file it, against me.