202195 - Rewrap incorrect for "two spaces after period" sentences

Status: NEW

(MailNews Core :: Composition, defect)

Description

[Manish Jethani]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3) Gecko/20030312
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3) Gecko/20030312

If a sentence ends in "two spaces after the period" (which is common for
email/news) and there's a auto wrap at the right margin, the second space
carries over to the new line.  This makes the new line, and hence the
_next_sentence_, start out with a space!


Reproducible: Always

Steps to Reproduce:
1.  Set your mail composer to wrap at 64 characters.  (Edit -> Mail & Newsgroups
-> Composition -> Wrap plain text messages at [64] characters)
2.  Enter the following text in the mail composer window (without quotes!):
"This is a test for seeing whether a sentence ending in a period.  And two
spaces works."
3.  From the Edit menu, choose Rewrap.

Actual Results:  
Here you can see that the message has been formatted such that a carriage return
is inserted *between* the two spaces (and the first space is correctly discarded
since it has no business staying at the end of a line).  This looks ugly.

One might argue that not using the rewrap feature is a good workaround :) but
the wrap happens anyway even when you just send the message.  If the mail viewer
at the receiving end is format-flowed enabled (see rfc 2646) then it's not a
problem.  But otherwise it's going to display the message as-is, with the ugly
space preceding the start of the second sentence.


Expected Results:  
Discard the second space, insert carriage return there, and then discard the
first space.  More simply, just discard the second space and follow the same
logic being used currently.


A lot of people use two spaces after the period terminating their sentences,
esp. in email and usenet communication.  See this paragraph for an example.  The
case where a sentence termination happens exactly at the wrap margin has to be
considered in the formatting logic for format=flowed text.

Comment 1

[Boris 'pi' Piwinger]

I have not yet tested this, but rewrap is Akkana, changing.

pi

Comment 2

[Boris 'pi' Piwinger]

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4b) Gecko/20030507

I see it in Linux.

This seems similar, but not identical to bug 191881.

pi

Comment 3

[Boris 'pi' Piwinger]

Forgot to set it new.

pi

Comment 4

[Manish Jethani]

For Non-rfc2646 mail clients, this bug has the effect of some lines starting out
with a space.  HOWEVER, there's also a problem for rfc2646 mail clients:  In
some cases, the message gets displayed with only a single space between the two
sentences.

  my sentence is here.<space><space>new sentence

gets converted to (if there's a line wrap b/w the 2 spaces)

  my sentence is here.<space><newline><space>new sentence

which gets displayed (in mozilla mail itself) as

  my sentence is here.<space>new sentence

So you see that this message neither gets displayed correctly in a non-rfc2646
mail client, nor does it display correctly (one space is lost) in a rfc-2646
mail client like mozilla mail.

Comment 5

[Akkana Peck]

I type two spaces too -- force of habit.
I'm not quite sure of the right thing to do here.  Leave two spaces at the end
of the first line?  Leave one space?  Will either of those solutions confuse f=f
later when we save?

Implementing the fix is probably fairly easy, but testing all the combinations
of settings to find out what works and doesn't do the wrong thing on output is
time consuming.  If someone has a build tree and the time to test, I can help
with either suggestions on where the fix should go, or perhaps a patch.

Comment 6

[Manish Jethani]

Even though I've disabled f=f in my mozmail, I still have to deal with this
problem.  If my sentence ends just at the 63rd column (my wrap-margin being at
64), the next space character... you know what I mean.  In my opinion, if f=f is
disabled by the user, then the editor should do the formatting just like other
email editors (and none of the editors I've used in the past have a problem with
the two-space thing (just try out Vim ;) )).

I'm downloading the source tarball for win32.  If I'm able to build it, I'll
probably try out a fix.  Could you mention the relevant filenames/paths?  I've
never hacked on mozilla before.

Comment 7

[[e-mail]]

You probably shouldn't break in the middle of a run of spaces. For instance, if
you double-space in a textbox the words will wrap on the last space.

Comment 8

[Akkana Peck]

Sorry, Manish: I missed seeing your question when you asked it.

The main Rewrap code is in nsInternetCiter::Rewrap:
http://lxr.mozilla.org/seamonkey/source/editor/libeditor/text/nsInternetCiter.cpp#214

In particular, the part starting at line 340, where it uses the line breaker to
get the next reasonable place to break (space or otherwise) is probably the
general area where this fix would have to go.

Comment 9

[Mike Cowperthwaite]

Isn't this a duplicate of bug 125928  (and also bug 155015)?

Comment 10

[Manish Jethani]

Actually I don't think it's a duplicate of bug 125928 at all, simply because
125928 doesn't talk about f=f.  As for bug 155015, I think _this_ bug is more
generic.

Comment 11

[Manish Jethani]

Was just looking at the code (I can't build it, for lack of tools, etc.)
http://lxr.mozilla.org/seamonkey/source/editor/libeditor/text/nsInternetCiter.cpp#317

On line 317, it seems to indicate that unquoted lines aren't wrapped in this
function.  I've reported this bug only on the unquoted part of the text, so I
believe the fix may not be in this area.  Am I right?  (akkana, to you)

Comment 12

[Akkana Peck]

Oh!  If this is only happening in unquoted text, then I probably pointed to the
wrong piece of code, misunderstanding the problem.  Sorry!  For the unquoted
text, the wrapping is probably coming from the plaintext serializer, not the
cite/rewrap code.

But one thing I don't understand: if that's the case, then this wouldn't only
happen on rewrap: it would happen whenever a message was sent with that line
length.  Can someone test whether this only happens for rewrap, or if it happens
if you just send the message without ever applying rewrap?

If it's the serializer (which it is if it happens even without rewrap), then the
place to start looking is nsPlainTextSerializer::AddToLine,
http://lxr.mozilla.org/seamonkey/source/content/base/src/nsPlainTextSerializer.cpp#1284

Comment 13

[Stewart Gordon]

RFC 2646 doesn't specifically address the issue of double spaces, but it appears
that the intuitive

Here are two spaces.<space><space><newline>And here is another sentence.

should be perfectly valid.

Comment 14

[Mike Cowperthwaite]

*** Bug 270322 has been marked as a duplicate of this bug. ***

Comment 15

[Stewart Gordon]

On second reading of the RFC, I've noticed this:

"If the line ends in one or more spaces, the line is flowed.  Otherwise it is
fixed.  Trailing spaces are part of the line's content, but the CRLF of a soft
line break is not."

Notice the use of the plural.  So the intuitive method, putting the two spaces
at the end of the line, is one way of doing it correctly.

Comment 16

[Mike Cowperthwaite]

(In reply to comment #15)
> On second reading of the RFC...

This bug, as originally reported, is about the Rewrap function, not about 
reflowing the text per RFC 2646.  I wish reporter hadn't mentioned f=f in the 
original report; it's confused most of this bug (starting with his comment 4).  
The bug about handling end-of-line spaces in f=f (as you know Stewart) is 
bug 223279.

Since Rewrap *breaks* the flow -- it actually inserts a hard newline at the end 
of every visible line within the editor's buffer (bug 220575) -- it's hard to 
worry about its end-of-line space handling at this point.  In fact, I'm of the 
opinion that Rewrap should affect *only* quoted text.

I'm not convinced that this isn't a dupe of bug 191881, as Boris indicated in 
comment 2.

Comment 17

[Paolo Bonzini]

I cannot reproduce this in TB3.