The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in Thunderbird 45.0

Status

MailNews Core
MIME
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: Jorg K (GMT+1), Assigned: Jorg K (GMT+1))

Tracking

Trunk
Thunderbird 45.0

Firefox Tracking Flags

(firefox45 affected)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

a year ago
Created attachment 8696538 [details]
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.
(Assignee)

Updated

a year ago
Attachment #8696538 - Attachment mime type: message/rfc822 → text/plain
(Assignee)

Comment 1

a year ago
Created attachment 8696894 [details]
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.
(Assignee)

Comment 2

a year ago
I'm changing the summary to reflect the problem.
Summary: Forwarding or "editing as new" of a format=flowed plain text messages loses the flowing and inserts hard line breaks → Plain text forwarding of a format=flowed plain text message loses the flowing and inserts hard line breaks
(Assignee)

Comment 3

a year ago
Created attachment 8696934 [details] [diff] [review]
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
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Attachment #8696934 - Flags: review?(mkmelin+mozilla)

Comment 4

a year ago
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.
Attachment #8696934 - Flags: review?(mkmelin+mozilla) → review+
(Assignee)

Comment 5

a year ago
Created attachment 8697875 [details] [diff] [review]
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.
Attachment #8697875 - Flags: review?(mkmelin+mozilla)

Comment 6

a year ago
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
Attachment #8697875 - Flags: review?(mkmelin+mozilla) → review+
(Assignee)

Comment 7

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

Thanks for the quick turnaround. Let's go with the other solution.
Attachment #8696934 - Attachment is obsolete: true
(Assignee)

Updated

a year ago
Keywords: checkin-needed

Comment 8

a year ago
https://hg.mozilla.org/comm-central/rev/072e15ff96e4
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Keywords: checkin-needed
OS: Unspecified → All
Hardware: Unspecified → All
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 45.0
You need to log in before you can comment on or make changes to this bug.