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)
MailNews Core
Composition
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.
Comment 1•20 years ago
|
||
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>
Comment 2•20 years ago
|
||
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.
Reporter | ||
Comment 3•20 years ago
|
||
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.
Comment 4•20 years ago
|
||
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.
Reporter | ||
Comment 5•20 years ago
|
||
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...
Comment 6•20 years ago
|
||
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.
Reporter | ||
Comment 7•20 years ago
|
||
Reporter | ||
Comment 8•20 years ago
|
||
Reporter | ||
Comment 9•20 years ago
|
||
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...
Comment 10•20 years ago
|
||
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)
Comment 11•20 years ago
|
||
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).
Comment 12•20 years ago
|
||
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.
Reporter | ||
Comment 13•20 years ago
|
||
Confirmed - my mailnews.wraplength config option is set to 0.
Comment 14•20 years ago
|
||
xref bug 125928
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)
Updated•20 years ago
|
Product: MailNews → Core
Comment 15•20 years ago
|
||
*** Bug 184981 has been marked as a duplicate of this bug. ***
Comment 16•19 years ago
|
||
(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.
Comment 17•19 years ago
|
||
(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.
Comment 18•19 years ago
|
||
*** Bug 323217 has been marked as a duplicate of this bug. ***
Comment 19•18 years ago
|
||
*** Bug 336583 has been marked as a duplicate of this bug. ***
Comment 20•18 years ago
|
||
*** Bug 347074 has been marked as a duplicate of this bug. ***
Comment 21•18 years ago
|
||
*** Bug 352559 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 22•18 years ago
|
||
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).
Assignee | ||
Comment 23•18 years ago
|
||
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)
Assignee | ||
Comment 24•17 years ago
|
||
Attachment #265424 -
Flags: superreview?(bzbarsky)
Attachment #265424 -
Flags: review?(mscott)
Comment 25•17 years ago
|
||
Please ask someone else for sr; I'm not going to get to this in a reasonable time frame (months, at least).
Assignee | ||
Updated•17 years ago
|
Attachment #265424 -
Flags: review?(timeless)
Comment 26•17 years ago
|
||
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 27•17 years ago
|
||
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)
Comment 28•17 years ago
|
||
(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.
Assignee | ||
Comment 29•17 years ago
|
||
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).
Comment 30•17 years ago
|
||
(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.
Assignee | ||
Comment 31•17 years ago
|
||
(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)
Comment 32•17 years ago
|
||
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
Comment 33•17 years ago
|
||
(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.
Updated•17 years ago
|
OS: Windows XP → All
Hardware: PC → All
Comment 34•17 years ago
|
||
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+
Updated•17 years ago
|
Attachment #265424 -
Flags: superreview?(mscott) → superreview+
Updated•17 years ago
|
Assignee: nobody → andrit
Comment 35•17 years ago
|
||
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.
Comment 36•17 years ago
|
||
Ben, did you intend to r+ this? Or was that a still thinking about it?
Comment 37•17 years ago
|
||
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.
Assignee | ||
Comment 38•17 years ago
|
||
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)
Assignee | ||
Updated•17 years ago
|
Attachment #284993 -
Flags: superreview? → superreview?(mscott)
Updated•17 years ago
|
Attachment #284993 -
Flags: superreview?(mscott) → superreview+
Assignee | ||
Comment 39•17 years ago
|
||
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)
Assignee | ||
Comment 40•17 years ago
|
||
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.
Assignee | ||
Comment 41•17 years ago
|
||
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
Comment 43•16 years ago
|
||
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
Updated•16 years ago
|
Product: Core → MailNews Core
Comment 45•15 years ago
|
||
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.
Description
•