Closed Bug 155622 Opened 22 years ago Closed 17 years ago

Wrapping information lost on "Edit as new" (including templates), "Edit draft" -- RFC 2646 format=flowed

Categories

(MailNews Core :: Composition, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9beta3

People

(Reporter: bryce2, Assigned: andrit)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 8 obsolete files)

In composer paragraphs automatically wrap to the window width (make the window bigger and smaller to see). Now send such a message. Go back to your sent folder and 'edit the message as new'. All the formatting is gone. The composer no longer wraps the message to the window width. This should not happen with Mozilla's RFC 2646 compliant format=flowed, messages.
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Wrapping information lost in sent folder (RFC 2646 format=flowed) → Wrapping information lost on "Edit as new", "Edit draft" (RFC 2646 format=flowed)
Depends on: 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!
Blocks: 168420
*** 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. ***
Summary: Wrapping information lost on "Edit as new", "Edit draft" (RFC 2646 format=flowed) → Wrapping information lost on "Edit as new" (including templates), "Edit draft" -- RFC 2646 format=flowed
Blocks: 259121
*** Bug 270718 has been marked as a duplicate of this bug. ***
Product: MailNews → Core
No longer blocks: 259121
*** 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 mscott@mozilla.org 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".
Assignee: ducarroz → nobody
QA Contact: esther
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
Flags: blocking-thunderbird2?
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. ***
Flags: blocking-thunderbird2? → blocking-thunderbird2-
Keywords: helpwanted
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?selm=QsmdnVttbbBQ9EzZnZ2dnUVZ_s6dnZ2d@mozilla.org
*** 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.)
Attached patch Possible bugfix (obsolete) (deleted) — Splinter Review
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.
Comment on attachment 259665 [details] [diff] [review] Possible bugfix Sorry, the fix does not work correctly with flowed quoted text.
Attached patch Improved fix (obsolete) (deleted) — Splinter Review
This patch works correctly with flowed quotes.
Attachment #259665 - Attachment is obsolete: true
Comment on attachment 259776 [details] [diff] [review] Improved fix 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]
Attached patch improved fix2 (obsolete) (deleted) — Splinter Review
Thank you Tuukka for detailed review. Here is adjusted patch.
Attachment #259776 - Attachment is obsolete: true
Attachment #259815 - Flags: review?(bienvenu)
Comment on attachment 259815 [details] [diff] [review] improved fix2 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!
Attachment #259815 - Flags: review?(bienvenu) → review-
Attached patch fix3 (obsolete) (deleted) — Splinter Review
Thank you David for detailed review and suggestions. Here is the adjusted by your comments patch.
Attachment #259815 - Attachment is obsolete: true
Attachment #259823 - Flags: review?
Attached patch fix3.1 (obsolete) (deleted) — Splinter Review
Sorry. Brace indentation fixed.
Attachment #259823 - Attachment is obsolete: true
Attachment #259824 - Flags: review?
Attachment #259823 - Flags: review?
Attached patch fix3.2 (obsolete) (deleted) — Splinter Review
Comments adjusted.
Attachment #259824 - Attachment is obsolete: true
Attachment #259874 - Flags: review?
Attachment #259824 - Flags: review?
Attachment #259874 - Flags: review? → review?(bienvenu)
Attachment #259874 - Flags: superreview?(mscott)
Comment on attachment 259874 [details] [diff] [review] fix3.2 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...
Attachment #259874 - Flags: review?(bienvenu) → review+
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.
Ok, then here is the adjusted patch with Cut() and checking whether wrapping is enabled.
Attachment #259874 - Attachment is obsolete: true
Attachment #260183 - Flags: superreview?(mscott)
Attachment #260183 - Flags: review?(bienvenu)
Attachment #259874 - Flags: superreview?(mscott)
Attachment #260183 - Attachment description: fix4 with checking wether wrapping is enabled → fix4 with 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.
Comment on attachment 260183 [details] [diff] [review] fix4 with checking whether wrapping is enabled 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!
Attachment #260183 - Flags: superreview?(mscott) → superreview+
> 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.
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.
Comment on attachment 260183 [details] [diff] [review] fix4 with checking whether wrapping is enabled sorry, this one got past me. Looks good, thx for the patch.
Attachment #260183 - Flags: review?(bienvenu) → review+
Assignee: nobody → andrit
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
Status: NEW → RESOLVED
Closed: 17 years ago
Keywords: checkin-needed
QA Contact: composition
Resolution: --- → FIXED
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!
Status: RESOLVED → VERIFIED
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)
Here is the patch to already checked in fix4 that handle correctly "-- " and "- -- " delimiters.
Resolution: FIXED → INCOMPLETE
Attachment #289216 - Flags: review?(bienvenu)
Attachment #289216 - Flags: superreview?(mscott)
Attachment #289216 - Attachment description: patch to handle "-- " (signature delimiter) correctly → patch5 to handle "-- " (signature delimiter) correctly
Attachment #289216 - Flags: superreview?(mscott)
Attachment #289216 - Flags: review?(bienvenu)
Status: VERIFIED → REOPENED
Resolution: INCOMPLETE → ---
Status: REOPENED → ASSIGNED
patch5 adjusted a bit
Attachment #289216 - Attachment is obsolete: true
Attachment #289225 - Flags: superreview?(mscott)
Attachment #289225 - Flags: review?(bienvenu)
Attachment #289225 - Flags: superreview?(mscott) → superreview+
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.
Comment on attachment 289225 [details] [diff] [review] patch5.1 to handle "-- " (signature delimiter) correctly 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!
Attachment #289225 - Flags: review?(bienvenu) → review+
Thanks David. Here it is.
Attachment #289225 - Attachment is obsolete: true
Attachment #294946 - Flags: superreview?(mscott)
Attachment #294946 - Flags: review?(bienvenu)
Comment on attachment 294946 [details] [diff] [review] patch5.2: style adjusted a bit in patch5.1 You don't need any more reviews for this.
Attachment #294946 - Flags: superreview?(mscott)
Attachment #294946 - Flags: review?(bienvenu)
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
Status: ASSIGNED → RESOLVED
Closed: 17 years ago17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M11
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/
Product: Core → MailNews Core
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.
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".
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: