1231017 - Plain text forwarding of a format=flowed plain text message loses the flowing and inserts hard line breaks

Status: RESOLVED FIXED

(MailNews Core :: MIME, defect)

Description

[Jorg K (CEST = GMT+2)]

Attachment: Message showing the problem.

Forwarding or "editing as new" of a format=flowed plain text messages loses the flowing and inserts hard line breaks.

See sample enclosed.

Comment 1

[Jorg K (CEST = GMT+2)]

Attachment: Message after plain text forwarding.

This is more complicated.

When "editing as new" of a plain text message this plain text message is placed into a "plain text" editor, that is one with style "white-space:pre-wrap; width:72".

Of course the message is wrapped to 72 characters in the composition window. It is still sent as flowed, so no hard returns are inserted.

When HTML forwarding a plain text message (inline, of course), the original plain text message is placed into a <pre> block. In this case the flowing effect is lost. This is desired. If the long line were re-assembled before placing it into the <pre> block, a long line that overflows the screen would be created, since no wrapping/line breaking takes place in <pre> blocks.

So far, so good.

When plain text (inline) forwarding a plain text message (using shift forward or with an account that sends plain text by default), the original message is placed into a "plain text" editor, remember, "white-space:pre-wrap; width:72", so of course the message appears wrapped.

Upon sending the message again with format=flowed, the part above the quote "-------- Forwarded Message --------" is indeed sent as flowed, however, the forwarded inlined message below has trailing spaces removed and therefore loses the flowing effect. In effect, the HTML forwarded message with <pre> and the plain text forwarded message (trailing spaces removed) visually look the same.

The result is attached. Look at it closely. The lines with "flowed" above have trailing spaces and are therefore flowed, the lines with "huhu" below do not have trailing spaces.

This can be reproduced with TB 38.4, so it's not a regression from the CJK bugs.

Comment 2

[Jorg K (CEST = GMT+2)]

I'm changing the summary to reflect the problem.

Comment 3

[Jorg K (CEST = GMT+2)]

Attachment: Proposed solution (v1).

OK, after hours of detective work:

In nsMsgCompose::BuildBodyMessageAndSignature()
https://dxr.mozilla.org/comm-central/source/mailnews/compose/src/nsMsgCompose.cpp#4331

we string a message back together:
https://dxr.mozilla.org/comm-central/source/mailnews/compose/src/nsMsgCompose.cpp#4397
Sadly, the block is controlled by this condition:
if (!m_composeHTML && !addSignature && wrapping_enabled)

And for inline forwards, addSignature is set to true:
https://dxr.mozilla.org/comm-central/source/mailnews/compose/src/nsMsgCompose.cpp#4354
so the block that potentially removes the line breaks from the original message is not executed.

I reset addSignature if no signature is added.

Sorry, Magnus, more work for you. This is an old bug we found when testing the CJK stuff, but it has nothing to do with CJK. In fact, as noted in bug 1230971 comment #6, CJK messages are not affected, since their bodies are retrieved with long lines intact as can be seen in debug by adding
   m_compFields->GetBody(body);
+  printf("|%s|\n", NS_ConvertUTF16toUTF8(body).get());
here:
https://dxr.mozilla.org/comm-central/source/mailnews/compose/src/nsMsgCompose.cpp#4346

Comment 4

[Magnus Melin [:mkmelin]]

Comment on attachment 8696934 [details] [diff] [review]
Proposed solution (v1).

Review of attachment 8696934 [details] [diff] [review]:
-----------------------------------------------------------------

Sure, r=mkmelin
Would be nice with a test for this.

Comment 5

[Jorg K (CEST = GMT+2)]

Attachment: Proposed solution - alternative (v1).

Magnus, thank you for the review.

Sorry, I changed my mind a bit. Why would the restoring of the flowed forwarded message depend on whether there is a signature or not? Why not always restore the flowed inline part?
I looked at "blame". This line
  if (!m_composeHTML && !addSignature && wrapping_enabled)
has been there from day one.

I tested with a signature above and below the inline part and there is no problem.

Can you please take another look. Sorry again about the hassle.

Comment 6

[Magnus Melin [:mkmelin]]

Comment on attachment 8697875 [details] [diff] [review]
Proposed solution - alternative (v1).

Review of attachment 8697875 [details] [diff] [review]:
-----------------------------------------------------------------

Yes I don't see how that would be different. 
Would be nice to look at cvsblame, but that seems not to be working atm - http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/mailnews/compose/src/nsMsgCompose.cpp&rev=1.573&mark=4428#4428

Comment 7

[Jorg K (CEST = GMT+2)]

Comment on attachment 8696934 [details] [diff] [review]
Proposed solution (v1).

Thanks for the quick turnaround. Let's go with the other solution.

Comment 8

[:aceman]

https://hg.mozilla.org/comm-central/rev/072e15ff96e4