Opening a message from the drafts folder for editing does not remove added linebreaks

Yes, It seems that when:

user_pref("mailnews.send_plaintext_flowed", true);

the plain text editor supports some sort of internal formatting which when saved
as draft, do Rewrap, copy and paste a quoted text (with ">" in front) is loosed
or it isn't rebuilded in some manner. This way causing the insert of a blank
character before ">" (one of the things which is most annoying) this way
breaking up the format of the quoted text.

I confirm that this bug exists in Mozilla 1.0 [Mozilla/5.0 (Windows; U; Win 9x
4.90; en-US; rv:1.0.0) Gecko/20020530]. The way I see it is: messages with
"format=flowed" are correctly sent, received and displayed, but when I select
"Message / Edit As New" on ANY such message (in Inbox, Drafts or Sent, for
example) the flowed formatting is lost at that point and it appears in the
message editor with hard line breaks and with ">" quoting characters shown
verbatim rather than as a continuous vertical bar.

Rather than submit a new bug, I will comment here that in particular this makes
multiple iterations of drafting difficult. On opening a saved draft, even though
it looks flowed in the Drafts folder, the flowing is lost. It also makes
re-saved edits look bizarre when viewed flowed, because entirely new paragraphs
or lengthened lines will flow whereas entirely old ones no longer will. So (with
indentation and shortened width for clarity), the message

 One very long a paragraph.

 Two very long b paragraph.

could become

 One very long
 a paragraph.

 Two very long
 edit
 b paragraph.

 Three very long c paragraph.

Yes, this bugs exists and still around. In BuildID 2002111716 (trunk) compiled
and running on Red Hat Linux 7.3, "Edit draft" (and "Edit as new" probably as
well) will break paragraphs into separate lines.

P.S. The quotations problem (mentioned in comment #1) was fixed in bug 110949.

Uisng trunk build 20021121 on winxp, macosx and linux.
Aleksey, I don't see the problem with text being typed not wrapping on Edit as
New or Edit draft messages. Here is what I did to test this. Please let me
know your scenario so I can test it too.

1. Launch mail, have my mail account set for plain text editing.
2. Composed and sent myself a plain text message (wrapped set at 72 char) with 3
lines that wrapped.
3. Got the message and then clicked Reply, started typing text where the cursor
landed or in a bread in the quoted area after pressing <Enter> (note: if you
don't press <Enter> you are in the quoted text, if it's within the quoted text
area the text will type in a blue color), then I saved the message as a draft to
continue typing later.
4. Selected the Draft message and clicked on Edit, then continued typing where I
left off so it would hit the 72 char limit. I hit the limit and the text wrapped.
If your scenario includes typing within the quoted area without pressing <Enter>
to break the quote then this is the same a bug 161969.

For Edit as New I selected that same message that I sent in step 1 above and
clicked Message|Edit as New. I continued typing text and it wrapped at 72
characters.

> I don't see the problem with text being typed not wrapping

There is no problem with *newly* typed text. The problem is with the text that
was there in the *original* message (but not the quoted text) - any long
paragraphs become broken into short lines on "Edit as new", "Edit draft".

Steps:

0) Set up Mozilla to use plaintext compose and to wrap long lines.
1) Compose a new message to yourself. Type in a long paragraph that would wrap.
2) Use "Edit as new" on the message.
3) Send right away (w/o editing).

Expected: both copies are displayed identically, with long paragraphs flowing
(e.g. broken into line dynamically according to the window width).
Actual: first copy is displayed with flowed paragraphs, but the second copy is
sent and displayed with paragraphs broken into separate line (e.g. "soft" line
breaks become "hard" ones).

If in steps (1) and (2) you do "Save as Draft" and "Edit traft" instead, the
result will still be the same (paragraphs broken into lines).

If in step (3) you add another long paragraph (w/o touching the original text
that you typed in step (1) ), then the result is *really* ugly - the original
text is broken into lines and the new text is shown flowed!

*** Bug 175953 has been marked as a duplicate of this bug. ***

Reference bug 45268.

See also bug 220575.

*** Bug 221474 has been marked as a duplicate of this bug. ***

*** Bug 222750 has been marked as a duplicate of this bug. ***

*** Bug 225267 has been marked as a duplicate of this bug. ***

*** Bug 236701 has been marked as a duplicate of this bug. ***

*** Bug 247856 has been marked as a duplicate of this bug. ***

*** Bug 231562 has been marked as a duplicate of this bug. ***

*** Bug 270718 has been marked as a duplicate of this bug. ***

*** Bug 259121 has been marked as a duplicate of this bug. ***

*** Bug 271651 has been marked as a duplicate of this bug. ***

Hm, just wondering. Has anybody worked on this bug in last two years (other
than marking bug reports as duplicate of this bug). This bug seems so old that
I haven't found it when I was filling my version of this very same bug report
(could have saved me some typing ;-) ).

Anyhow, I find that this bug is making Draft feature of Thunderbird rather
unusable. I believe that severity of this bug should be raised to at least
"major", because it severely criples one of the basic features of mail client.

(In reply to comment #19)
Concur. This, and the bug where the mail engine stops sending/receving mail
makes this product usable for many. I would vote 'show stopper' for both.

This bug still exists in Thunderbird version 1.0 (20041206).

*** Bug 274167 has been marked as a duplicate of this bug. ***

(In reply to comment #19)
The incredible number of duplicates speaks for itself. I can not use the draft
feature in Thunderbird (for any message over one line in length) due to this
bug. Any insights on the development process? TB 1.0 still has this issue.

*** Bug 292322 has been marked as a duplicate of this bug. ***

*** Bug 304342 has been marked as a duplicate of this bug. ***

This bug is assigned to Ducarroz, but he has not made even one comment in the 3.5 years this bug has existed (nor in any of many other f=f bugs since ~2002). Perhaps mozilla.org is not even aware of this very annoying bug.
-> CC'ing <email address hidden> to give mozilla.org a "heads-up"

Can someone at least changed the status from NEW? After 3.5 years, and dozens of duplicate reports, I think it is safe to say this is a confirmed issue.

"NEW" *means* "confirmed".

Okay, thanks.
Note also that this issue contributes to longer URL's getting broken across two lines. I just ran into this again a few seconds ago.

I don't consider this bug to be "minor". I re-edit previously stored messages very often (Edit Draft, Edit as New), and encounter this bug daily. It make messages hard to read and ugly. A very visible Thunderbird deficiency IMO.

Nominating for Thunderbird 2.0

Please, add this to the tracker of Rewrap issues (bug 192905) as:

Bug 155622 blocks 192905

(In reply to comment #31)
> Please, add this to the tracker of Rewrap issues (bug 192905) as:
>
> Bug 155622 blocks 192905
>
Bug 192905 is tracking the issues with the "Edit -> Rewrap" command, which is not what this bug is about.

*** Bug 290472 has been marked as a duplicate of this bug. ***

This is a workaround for bug 155622: You select your response text in the preview pane, then press Ctrl-C (copy), then edit the message, then select the your response text in the edit window, then press Ctrl-V (paste).

http://groups.google.com/groups?<email address hidden>

*** Bug 356836 has been marked as a duplicate of this bug. ***

the issue still is there in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.2) Gecko/20070222 SeaMonkey/1.1.1

This bug appears to have been fixed in Seamonkey.

However the Seamonkey solution comes at the loss of Format=Flowed text (See http://www.ietf.org/rfc/rfc2646.txt for details). Seamonkey sends plain text messages in traditional format, which cannot be correctly word wrapped on the receiving end no matter what.

HTML formatted messages, while malformed by Seamonkey, do not suffer the problem.

Sorry, but i don't see that it is fixed there.

I've played a bit with sources and it seems that this patch solves the problem:

Index: mailnews/compose/src/nsMsgCompose.cpp
===================================================================
RCS file: /cvsroot/mozilla/mailnews/compose/src/nsMsgCompose.cpp,v
retrieving revision 1.460.2.28
diff -u -r1.460.2.28 nsMsgCompose.cpp
--- mailnews/compose/src/nsMsgCompose.cpp 8 Feb 2007 22:21:08 -0000 1.460.2.28
+++ mailnews/compose/src/nsMsgCompose.cpp 25 Mar 2007 15:44:41 -0000
@@ -3829,7 +3829,15 @@
   // replace '\n' with <br> so that the line breaks won't be lost by html.
   // if mailtourl, do the same.
   if (m_composeHTML && (mType == nsIMsgCompType::New || mType == nsIMsgCompType::MailToUrl))
- body.ReplaceSubstring(NS_LITERAL_STRING("\n").get(), NS_LITERAL_STRING("<br>").get());
+ body.ReplaceSubstring(NS_LITERAL_STRING(NS_LINEBREAK).get(),
+ NS_LITERAL_STRING("<br>").get());
+
+ if (!m_composeHTML) {
+ body.ReplaceSubstring(NS_LITERAL_STRING(" \n").get(),
+ NS_LITERAL_STRING(" ").get());
+ body.ReplaceSubstring(NS_LITERAL_STRING(" \r\n").get(),
+ NS_LITERAL_STRING(" ").get());
+ }

   nsString empty;
   rv = ConvertAndLoadComposeWindow(empty, body, tSignature,

Actually the first change ("\n" to NS_LINEBREAK) is not related and should be not considered. Sorry for noise. (It is buggy a bit - it probably will not work if say on Windows you will work with letter saved on Unix. This is the reason why i double the replace with " \n" and " \r\n" to make sure it will work with letters saved on any platform.)

Here is the final patch without this change:

diff -u -r1.460.2.28 nsMsgCompose.cpp
--- mailnews/compose/src/nsMsgCompose.cpp 8 Feb 2007 22:21:08 -0000 1.460.2.28
+++ mailnews/compose/src/nsMsgCompose.cpp 26 Mar 2007 05:26:16 -0000
@@ -3831,6 +3831,13 @@
   if (m_composeHTML && (mType == nsIMsgCompType::New || mType == nsIMsgCompType::MailToUrl))
     body.ReplaceSubstring(NS_LITERAL_STRING("\n").get(), NS_LITERAL_STRING("<br>").get());

+ if (!m_composeHTML) {
+ body.ReplaceSubstring(NS_LITERAL_STRING(" \n").get(),
+ NS_LITERAL_STRING(" ").get());
+ body.ReplaceSubstring(NS_LITERAL_STRING(" \r\n").get(),
+ NS_LITERAL_STRING(" ").get());
+ }
+
   nsString empty;
   rv = ConvertAndLoadComposeWindow(empty, body, tSignature,
                                    PR_FALSE, m_composeHTML);

Andriy: please add patches as attachments. (Use the "add an attachment" link.)

Created an attachment (id=259665)
Possible bugfix

The same patch that is in https://bugzilla.mozilla.org/show_bug.cgi?id=155622#c39 but in separate file.

Sorry, the fix does not work correctly with flowed quoted text.

(From update of attachment 259665)
Sorry, the fix does not work correctly with flowed quoted text.

Created an attachment (id=259776)
Improved fix

This patch works correctly with flowed quotes.

(From update of attachment 259776)
You'll need to set 'review?' in the patch 'details' on someone to get attention; the recent parts of http://bonsai.mozilla.org/cvslog.cgi?file=/mozilla/mailnews/compose/src/nsMsgCompose.cpp will probably suggest reasonable review victims.

>+ char quote=0;

boolean; PRBool, PR_TRUE/FALSE

>+ for (int i=0; i < body.Length(); i++) {

signedness; PRUint32, check compiler warnings

>+ quote=1;

style; spaces

>+ if (body[j] == '\r')

if body[0] == '\n' you'd be looking at body[-1]

Created an attachment (id=259815)
improved fix2

Thank you Tuukka for detailed review. Here is adjusted patch.

(From update of attachment 259815)
Thx very much for working on this!

We don't really need to put bug #'s in the code - cvsblame will allow us find the bug, because the checkin comment will refer to the bug number.

we prefer spaces around operators, so i=0 should be i = 0, i==0 should be i == 0, or !i, i-1 should be i - 1.

What happens if the line terminator is '\r'? Or does the mac not use '\r' anymore? From lxr, it looks like it uses '\n', so your code should be ok.

The var name "j" is not meaningful - it would make the code a lot more readable if the var name indicated what "j" was - I think it's the index of the last char in the line, and if that char is a ' ', you're replacing the line terminating char(s) with ' '. So the code is basically doing this:

Look for unquoted lines - if we have an unquoted line that ends in a space, replace the end of line char(s) with spaces.

It would be good to have a comment that says that...

If that's correct, could you attach a new patch that addresses these comments? Thx!

Created an attachment (id=259823)
fix3

Thank you David for detailed review and suggestions. Here is the adjusted by your comments patch.

Created an attachment (id=259824)
fix3.1

Sorry. Brace indentation fixed.

Created an attachment (id=259874)
fix3.2

Comments adjusted.

(From update of attachment 259874)
thx for adding the comments. One final question, instead of replacing the CRLF's with spaces, should we just delete the CRLFs in this case? The previous line already ends with a space...

David, actually (as it is said in comment) CRLFs are just deleted. What the code does is the replace of " \r\n" with " ". I did it this was because the line

body.Replace(j, i-j + 1, ' ');

seemed to me more clear then

body.Replace(j + 1, i - j, '');

But this raise such questions then probably i was wrong and second line is more clear. What do you think?

Also one more thought: i think it would be good to check also whether wrapping is enabled at all (by analyzing wrapping length setting) and if it is not (wrapping length is 0) - do not perform this joining of lines. What do you think?

Also i noticed that even if wrapping is off (length is 0) saved mail has format=flowed and because of this it looks incorrect in preview - is this correct behavior? If one just switched off wrapping and while writing mails makes spaces at the end of some lines (say by mistake) - the mails in such a case will have format=flowed and will be looked different in preview. What do you think?

Thank you.

using Cut() would be much more clear (if Cut() is available - it's hard to keep up with our string classes).

yes, checking if wrapping is turned on would be a good idea, I think. I'm not an expert on all the different wrapping behaviours, however.

Created an attachment (id=260183)
fix4 with checking wether wrapping is enabled

Ok, then here is the adjusted patch with Cut() and checking whether wrapping is enabled.

I don't understand what the wrapping-enabled check is doing. It looks like you're referring to the plain-text wrap-column value. That should make no difference at all when reading the text in. I think that check can be dropped.

You said in comment 52 that with wrapping=0, "it looks incorrect in preview"
-- could you expand on this, please? I want to make sure that you're not adding a hack to fix a problem that is better addressed elsewhere, especially when I am so eagerly anticipating get the meat of this problem fixed.

Note that there are known problems with wrapping=0 and plain-text mail -- see bug 261467. Don't work around that bug by adding this check in here, it'll just break things in a way that will make it more difficult to fix later on.

|mailnews.wraplength=0| is primarily a hack that people use because
|mail.compose.wrap_to_window_width| is neither well-known nor does it work that well. Ideally, when f=f is turned on, wrap_to_window_width would be automatically enabled as well, *and* the editor would display that correctly, but for now it doesn't work that way.

Mike,

1) I think that check on wrapping-enabled is worth here. What is the reason to restore flowed text wrapping of the saved mail if one is switched wrapping off? I think this is related to bug#261467. If one does not use automatic wrapping - there is no reason to change his text behind his back - he should see the same text with the same behavior with the same CRLFs at the end of lines, right?

2) It seems to me that my last paragraph in https://bugzilla.mozilla.org/show_bug.cgi?id=155622#c52
comment 52 is exactly the reason of https://bugzilla.mozilla.org/show_bug.cgi?id=261467
bug 261467. I think if automatic wrapping is off (wrap length is 0) there should not be format=flowed attribute in saved/sent mails (as it is now). That is the attribute that makes mail viewers try to treat the mail text as flowed that actually is not true for mails manually wrapped (possibly with spaces at the end of lines, that cause joining of lines in flowed text viewers).

My fix is not related to this bug (bug#261467) - i just mentioned the strange behavior of composer i noticed that seems to be the reason of bug#261467.

*** Bug 377386 has been marked as a duplicate of this bug. ***

(From update of attachment 260183)
this looks good to me, Tuukka and David have already given it a pretty good review.

I haven't had a chance to test it myself though...

Thanks for the patch!

> What is the reason to restore flowed text wrapping of the saved mail if one
> is switched wrapping off?

Well, like I said: wrap=0 is itself a bit of a hack. The behavior is distinct from f=f, and some people may be using wrap=0 for that specific behavior. I can't say if there's a case where someone wants f=f support *and* wrap=0, but f=f changes more than just wrapping.

I don't use wrap=0, so I don't really mind if you put the check in; I'm just not convinced that the check doesn't break something for someone else who might be using wrap=0. Further, I don't see that executing the line-break removal for the wrap=0 case prevents this bug from being fixed.

This is a nice complement to bug 125928. If only the trunk didn't have the loathsome display of body text in the thread pane, I'd be eagerly anticipating downloading a version with these bugs fixed; but Alas! The trunk is not useable due to that misfeature. :/

(In reply to comment #59)
> I'm just not convinced that the check doesn't break something for someone else
> who might be using wrap=0.

It won't, because the code executed only if wraplen != 0.

*** Bug 387318 has been marked as a duplicate of this bug. ***

What are we waiting for here? Looks like the patch is reviewed and ready to checkin ...

If the r+ can be transfered from v3.2 to v4 of the patch, this is ready for checkin, but you'll have to ask David about it.

(From update of attachment 260183)
sorry, this one got past me. Looks good, thx for the patch.

Checking in mailnews/compose/src/nsMsgCompose.cpp;
/cvsroot/mozilla/mailnews/compose/src/nsMsgCompose.cpp,v <-- nsMsgCompose.cpp
new revision: 1.530; previous revision: 1.529
done

Thank you!

Verified fixed with TB 3a1p-1013, Win2K

Thank you thank you thank you, Andriy! You have nailed my top two most irritating bugs!

Unfortunately the fix does not take into account signature delimiter "-- " and as result behaves incorrectly with it. I will try to fix this ASAP.

Mike, do you know whether there is new bug opened for this already? Thank you.

and "- -- " case as well (see bug 99922 for details, "- -- " is signature delimiter in PGP-signed mails)

Created an attachment (id=289216)
patch to handle "-- " (signature delimiter) correctly

Here is the patch to already checked in fix4 that handle correctly "-- " and "- -- " delimiters.

Created an attachment (id=289225)
patch5.1 to handle "-- " (signature delimiter) correctly

patch5 adjusted a bit

*** Bug 409580 has been marked as a duplicate of this bug. ***

To clearify, Bug 409580 was only an Duplicate to the regression Andriy reported in comment c#68.
The review-awaiting patch 5.1 works fine almost under Linux, so is there any chance to get an positiv review from David Bienvenu for this? Would be very great.

(From update of attachment 289225)
I'm willing to give it a try - if you could just make the braces in this part:

+ if (body[i] == '>') {
+ quote = PR_TRUE;
+ continue;
+ }

so that the brace after the if () is on it's own line, that would be great.

thx for the patch!

Created an attachment (id=294946)
patch5.2: style adjusted a bit in patch5.1

Thanks David. Here it is.

(From update of attachment 294946)
You don't need any more reviews for this.

Checking in mailnews/compose/src/nsMsgCompose.cpp;
/cvsroot/mozilla/mailnews/compose/src/nsMsgCompose.cpp,v <-- nsMsgCompose.cpp
new revision: 1.538; previous revision: 1.537
done

*** Bug 435972 has been marked as a duplicate of this bug. ***

This bug is not yet solved:

Build Identifier: Version 2.0.0.14 (20080421)

Thunderbird does not correctly handle messages that are continued from a draft.
While line wrapping works as expected when writing a message it does not when a
message is saved to "Drafts" and later continued. It seems like only formerly
written lines are saved "wrapped" and newly added lines are not handled at all.

Reproducible: Always

Steps to Reproduce:
1. Open "new" message
2. Write some lines without return
3. Close message and save to "Drafts"
4. Continue message with some more lines and send message
5. Check "Sent" for message or check received message
Actual Results:
Message is not wrapped correctly.

Expected Results:
Message should be wrapped after 72 characters (like the saved part of the
message is).

Standard (default) theme was used. Standard MacBook was used. Enigmail Add-on
used. Error occurs without having installed Enigmail, too.

To see the fix, you need a nightly or the latest 3.0 alpha
http://www.mozillamessaging.com/en-US/thunderbird/early_releases/

Is this likely to end up in Thunderbird 2? I couldn't easily find any roadmap info stating plans for Thunderbird 2 and 3.

No this is tb3 (and seamonkey2) only.

*** Bug 456640 has been marked as a duplicate of this bug. ***

*** Bug 470878 has been marked as a duplicate of this bug. ***

*** Bug 474224 has been marked as a duplicate of this bug. ***

*** Bug 522963 has been marked as a duplicate of this bug. ***

*** Bug 492637 has been marked as a duplicate of this bug. ***

I still have this bug on this configuration:

- Thunderbird 3.0.3
- Gnome 2.29.92
- Ubuntu 10.04 Beta 1

To reproduce the bug, I just have to save a draft, and reopen it for modification.

After all these years, I finally found that I was still experiencing this bug (see my comment 88) because of the addon Enigmail, that automatically sets "mailnews.send_plaintext_flowed" to "false".