Closed Bug 261467 Opened 20 years ago Closed 16 years ago

Spaces not stripped from end-of-line on Send (plain-text compose, wrap column=0)

Categories

(MailNews Core :: Composition, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: jkilmer, Assigned: andrit)

References

Details

Attachments

(3 files, 2 obsolete files)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20040910 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20040910 When composing & sending a message, if a line has one or more trailing spaces before a carriage return, the return is stripped before the message is saved or sent. The message saved in the "Sent" mailbox is identical to what the receiver sees. However, if that same message is then re-opened using the "Edit Message as New" function, the return is again present -- but it is again stripped if that message is sent. This has been occurring consistently since at least Mozilla 1.4, but seemed random enough that I was never able to nail it down. This seems very similar to Bug 99922, but that bug reported (back in 2001) that the _spaces_ were stripped. Currently, it seems like the spaces are retained, but the [return] is stripped. Reproducible: Always Steps to Reproduce: 1. Create a message similar to the following. Each line's text indicates what non-printable characters are placed at the end of the line. no trailing spaces, 1 return no trailing spaces, 2 returns 1 trailing space, 1 return 1 trailing space, 2 returns 4 trailing spaces, 1 return 4 trailing spaces, 2 returns end of test 2) Send the message Actual Results: The message saved in the "Sent" mailbox, and the message received by the recipient looked like: no trailing spaces, 1 return no trailing spaces, 2 returns 1 trailing space, 1 return 1 trailing space, 2 returns 4 trailing spaces, 1 return 4 trailing spaces, 2 returns end of test Note that the trailing spaces are maintained, but in all of the "1 return" cases with trailing spaces, the return is missing. In the 2 return cases, the second return is still retained, leaving 1 CR in the final result. Expected Results: The received (and saved) message should have been identical to the entered message.
Duplicate of Bug 197130 ? "Mail window doesn't show word wrap of plain text message as generated by mail compose window" In Bug 251671 "Failed to wrap line in message window." I wrote: Help_>About:config Check your setting for "mailnews.display.disable_format_flowed_support" Try setting this to true. <http://oceanpark.com/notes/howto_mozilla-mail-plain-text-word-wrap.html>
HTML composition, with the message converted to plain text? This looks like bug 125928, but that only occurs when more than one space is at the end of the line. And in fact, using your test data in an HTML compose window, I see these results: ===== no trailing spaces, 1 return no trailing spaces, 2 returns 1 trailing space, 1 return 1 trailing space, 2 returns 4 trailing spaces, 1 return 4 trailing spaces, 2 returns end of test ===== Comment 1 is correct that this is related to format=flowed -- the failure to strip all spaces from the end of the line when converting to plain causes the lines to be interpreted as flowable. With plain text composition, I see six separate lines. Same with HTML composition sent *as* HTML. Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a4) Gecko/20040924 Jim Kilmer, if on retesting you see the same results as I have described, please mark this bug as a dupe of 125928. Otherwise, please provide more information on how to reproduce this problem.
I don't believe this is a dup of bug 125928, as it definitely, consistently occurs with only one space at the end of the line -- not just multiple spaces. Also, this occurs with plaintext composition, not HTML composition. So, while almost certainly related, it doesn't seem to be a clear dup. I have confirmed that the issue does NOT occur when using HTML composition. The suggestion in Comment 1 regarding setting "mailnews.display.disable_format_flowed_support" to true _did_ correct this behavior. However, reading the reference link provided, it seems that this was intended to fix issues with auto-linebreaks when wrapping lines. It even states "Only hard newlines that I actually entered manually were properly displayed." In this case, it's the hard newlines that _aren't_ being properly displayed. The same could be said for Bug 168420, and the mini-FAQ attached there. It dances around describing this behavior, but focuses on the auto-linewrapping and the whole 72-column thing. From my perspective, this is still a bug as it provides unexpected and arguably erroneous behavior from an "out of the box" Mozilla install set to compose messages in plaintext. I think there's probably an issue here that has been obscured in the whole discussion over what's correct or not for _wrapped_ lines. For a plaintext message whose line content doesn't extend to the 72-column mark, I don't know if it can be considered correct to strip or ignore a hard newline entered by the user during composition.
Are you running with any extensions in place that might interfere with the plain text composition mode (such as Enigmail)? I know you say you've got an "out of the box" installation, but I do not, and never have, seen the behavior you're reporting.
No extensions. I'm quite confused as to why it would not be replicatable, as I did a fresh install on my new laptop over the weekend, and it yielded identical results to my desktop @ work. I would be happy to create an attachment containing my config settings, or any other information, if it would be of assistance. If you can't duplicate this, I have no idea how to helpfully proceed...
OK, try this: - Compose the message with the test data; save as draft; save the message from Drafts as a .eml file. - Send the message; save the message from the Sent folder as a .eml file. - Attach both files to this bug.
Attached file test message, saved as draft (deleted) —
Attached file test message, saved from Sent mailbox (deleted) —
As the contents of the .eml files attached to this bug appear correct and identical, I'm attaching this screenshot of that message as viewed inside Mozilla Mail as well, if only to prove that I'm not going insane...
I'm confirming this bug based on the attachments, but I can't reproduce the problem myself with Moz 1.8a4-0924 or TB 0.8 (20040913), Win2K.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Carriage return stripped from line if line ends w/ trailing whitespace → Spaces not stripped from end-of-line on Send (plain-text compose)
Just to double check, I retested after setting mailnews.send_plaintext_flowed to false -- I still get the same results of spaces stripped from end of line, and the same display of ten lines (three blank, seven with text).
On a hunch from reading bug 180451 (which appears to be about quite the opposite behavior), I reproduced this bug after setting mailnews.wraplength to 0 Jim Kilmer, please confirm that you've got your wrap column set to 0.
Confirmed - my mailnews.wraplength config option is set to 0.
Summary: Spaces not stripped from end-of-line on Send (plain-text compose) → Spaces not stripped from end-of-line on Send (plain-text compose, wrap column=0)
Product: MailNews → Core
*** Bug 184981 has been marked as a duplicate of this bug. ***
(In reply to comment #15) > *** Bug 184981 has been marked as a duplicate of this bug. *** I'm the original reporter of 184981. Reading the comments here, it sounds completley different to me, but I'm not a programmer. I've seen the bug in many versions of seamonkey, and now in thunderbird 1.0.x. I have a new way of clarifying my bug. When I view any of the messages with the bug in the "Message Pane" or in its own window, the empty line(s) is missing. When I right click the message and "Edit As New" (or when I originally was composing it), the empty line(s) is there. When or how seamonkey/thunderbird generates these "bugged" messages is still a mystery to me, but once they're created, they display this behavior consistantly forever. I already uploaded a bugged message (.eml) with this behavior in # 184981. If you ever want more, let me know.
(In reply to comment #16) Apparently I was supposed to post my previous comment under 184981. We've followed up with it there, but it appears it was the fault of having a setting of mailnews.wraplength to 0.
*** Bug 323217 has been marked as a duplicate of this bug. ***
*** Bug 336583 has been marked as a duplicate of this bug. ***
*** Bug 347074 has been marked as a duplicate of this bug. ***
*** Bug 352559 has been marked as a duplicate of this bug. ***
I think it is not correct behavior of composer to set format=flowed for saved/sent mails that were created with manual wrapping (i.e. when automatic wrapping is switched off - mailnews.wraplength=0).
The possible workaround for this i think would be to set mailnews.send_plaintext_flowed=false if you don't use automatic wrapping (i.e. if mailnews.wraplength=0)
Attached patch proposed patch (obsolete) (deleted) — Splinter Review
Attachment #265424 - Flags: superreview?(bzbarsky)
Attachment #265424 - Flags: review?(mscott)
Please ask someone else for sr; I'm not going to get to this in a reasonable time frame (months, at least).
Attachment #265424 - Flags: review?(timeless)
Comment on attachment 265424 [details] [diff] [review] proposed patch i understand strings and can understnad mailnews at times. however i'd rather leave format=flowed stuff to people who understand it
Attachment #265424 - Flags: review?(timeless) → review?(ben.bucksch)
Comment on attachment 265424 [details] [diff] [review] proposed patch I know someone who is very good at understanding the impact to format-flowed :)
Attachment #265424 - Flags: superreview?(mscott)
Attachment #265424 - Flags: superreview?(bzbarsky)
Attachment #265424 - Flags: review?(mscott)
Attachment #265424 - Flags: review?(mozilla.org)
(In reply to comment #24) > Created an attachment (id=265424) [details] > proposed patch > Andriy Tkachuk, can you briefly explain what this patch does, and how it fixes this bug? It's not obvious from the code (comments may help). It doesn't seem to do what you described in comment 23; anyway, please explain further. Thanks.
Sure. The patch just strips spaces from end of lines if wrap_column=0 and f=f is used. Without this the text in f=f aware mail viewers will be interpreted incorrectly, i.e. lines w/ spaces at the end will be joined w/ next lines (that is incorrect if wrap_column was set to 0, as far as I understand).
(In reply to comment #29) > Sure. The patch just strips spaces from end of lines if wrap_column=0 and f=f > is used. Without this the text in f=f aware mail viewers will be interpreted > incorrectly, i.e. lines w/ spaces at the end will be joined w/ next lines (that > is incorrect if wrap_column was set to 0, as far as I understand). > Andriy Tkachuk, thanks for the additional explanation. If I understand you correctly, this patch is not designed to fix the behavior described in comment 1 (at least not in all cases), but was designed to address a more specific situation when wrap_column=0 and format=flowed. I do not think the behavior of your patch is correct. As I understand it, if Thunderbird is set to (1) force manual wrapping and (2) use format=flowed, your patch would trim trailing spaces from each line. You are correct that in format=flowed content, trailing spaces before user-inserted hard line breaks should be trimmed (see RFC 3676 section 4.2). And you are correct that this means all trailing spaces in a manually wrapped message will be trimmed. But the reason this is true is because all hard line breaks in a manually wrapped message are "user-inserted"--it is not because the wrap method was set to "manual" instead of "automatic." To show what I mean, suppose I create the following message (# means a hard line break inserted using the Enter key) Hello # world # If I am using format=flowed, the trailing spaces in both lines of the message should be trimmed regardless of whether I have the wrap method set to manual or automatic (the default wrap method doesn't make a difference, because both the line breaks in the message were "user-inserted" hard line breaks). However, your patch would trim the spaces only if I had the wrap method set to manual; it would not trim the spaces if I had it set to automatic. Please let me know if I am misunderstanding your patch or the spec. I am not familiar enough with the Composer code to suggest another approach, but maybe others can help. Thanks.
(In reply to comment #30) > However, > your patch would trim the spaces only if I had the wrap method set to manual; > it would not trim the spaces if I had it set to automatic. Exactly. In automatic wrapping trimming is performing well, the bug is appearing only if wrap_column=0. When wrap_column=0 preformatted code path is executing that does not perform trimming (see comment at http://lxr.mozilla.org/mozilla1.8/source/content/base/src/nsPlainTextSerializer.cpp#1671)
sorry for the spam. making bugzilla reflect reality as I'm not working on these bugs. filter on FOOBARCHEESE to remove these in bulk.
Assignee: sspitzer → nobody
(In reply to comment #31) Sorry, I have been busy and have not had time to look at this lately. The logic sounds correct as you describe it, but I have not been able to look into the composer code more deeply. Perhaps someone who is more familiar with the composer code or has more time can take a look at this sooner; otherwise I will try to look into it more when I have more time.
OS: Windows XP → All
Hardware: PC → All
Comment on attachment 265424 [details] [diff] [review] proposed patch I think this behavior is correct--thanks and sorry it took so long.
Attachment #265424 - Flags: review?(mozilla.org) → review+
Attachment #265424 - Flags: superreview?(mscott) → superreview+
Assignee: nobody → andrit
I wonder who still really groks this code (there's so much newline- and space- searching and skipping), but at least the change matches the change from bug 125928 in the |else| block.
Ben, did you intend to r+ this? Or was that a still thinking about it?
This fix looks like it works for the trailing-space issue. However, I notice that lines with leading spaces, or leading '>' signs, are sent thru unaltered -- that is, they do not get the additional 'stuffing' space -- with wrap=0, and there's nothing in the patch that appears to address that. xref bug 215068.
Attached patch patch2 (obsolete) (deleted) — Splinter Review
Previous patch did not honor "-- " (signature delimiter - RFC 2646, 4.3)
Attachment #265424 - Attachment is obsolete: true
Attachment #284993 - Flags: superreview?
Attachment #284993 - Flags: review?(ben.bucksch)
Attachment #265424 - Flags: review?(ben.bucksch)
Attachment #284993 - Flags: superreview? → superreview?(mscott)
Attachment #284993 - Flags: superreview?(mscott) → superreview+
Comment on attachment 284993 [details] [diff] [review] patch2 proper fix provided in patch to bug 215068
Attachment #284993 - Attachment is obsolete: true
Attachment #284993 - Flags: review?(ben.bucksch)
Sorry, I was mistaken - there is no fix to this bug there and patch2 is not the right one - it does not take into account signature delimiter in PGP-signed mails and also it removes leading space(s?) in first mail line in HTML <pre> compose. I hope I will provide proper fix soon.
Ok, from now find the right patch to this bug in bug 215068. The reason is that fixes are connected. The actual fix is in these lines: + if ((outputLineBreak || !spacesOnly) && // bugs 261467,125928 + !stringpart.EqualsLiteral("-- ") && + !stringpart.EqualsLiteral("- -- ")) + stringpart.Trim(" ", PR_FALSE, PR_TRUE, PR_TRUE); the case is when there is !spacesOnly in input line. The outputLineBreak case fixes bug 125928.
Status: NEW → ASSIGNED
Depends on: 215068
I can confirm that the end-of-line spaces are now properly stripped from a plain-text compose message sent as f=f when the wrap column is 0. Marking FIXED: patch at bug 215068, per Andriy's comment 40. However, there is still a bug seen with wrap-column = 0: Lines that are typed in with a leading '>' are not space-stuffed and so are treated as quotes rather than text. I've opened bug 441020 for this.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Product: Core → MailNews Core
VERIFIED to be fixed on trunk with both plaintext composer and HTML composer.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: