Closed Bug 1231017 Opened 9 years ago Closed 9 years ago

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

Categories

(MailNews Core :: MIME, defect)

defect
Not set
normal

Tracking

(firefox45 affected)

RESOLVED FIXED
Thunderbird 45.0
Tracking Status
firefox45 --- affected

People

(Reporter: jorgk-bmo, Assigned: jorgk-bmo)

Details

Attachments

(3 files, 1 obsolete file)

Attached file Message showing the problem. (deleted) —
Forwarding or "editing as new" of a format=flowed plain text messages loses the flowing and inserts hard line breaks. See sample enclosed.
Attachment #8696538 - Attachment mime type: message/rfc822 → text/plain
Attached file Message after plain text forwarding. (deleted) —
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.
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
Attached patch Proposed solution (v1). (obsolete) (deleted) — Splinter Review
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 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+
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 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+
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
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 9 years 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.

Attachment

General

Created:
Updated:
Size: