18576 - [DOGFOOD] plaintext replies wrap at wrong line

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect, P3)

Description

[Akkana Peck]

With the new plaintext reply code which uses the new plaintext sink stream code,
plaintext replies get wrapped too early.  To see this: find a message that wraps
at a fairly normal place, like 72 columns, reply to it in plaintext, resize the
compose window much bigger, and notice that the wrapping is hard rather than to
window width.

Now, to see something worse, do a Debug->Output Text on that plaintext window.
Even if the wrap column is set to 72, the actual reply wraps much earlier than
that.

I suspect that the first problem is actually a more insidious problem: if the
message we're replying to was wrapped at 72, then once we add the "> " it needs
to wrap at 74, but the new text in the reply should still wrap at 72.  I.e.
using a style node on the body isn't the right thing to do; we should leave
replied text unwrapped.  I'm not sure if you can override the style node on the
body specifying _moz-pre-wrap to 72 by wrapping the quotes in a <pre> or not.
I'll experiment with that once the wrapping problem in the ouput sink (the
second problem here) is fixed.

I have a small patch to nsHTMLToTXTSinkStream.cpp which will get the wrap column
from the style node on the body, and set mWrapColumn appropriately.  I'll attach
the patch to this bug.

Comment 1

[Akkana Peck]

Attachment: Patch to get wrap col from moz-pre-wrap style

Comment 2

[Akkana Peck]

This is much better now.  Some lines still get wrapped wrong, but most are
better.

Comment 3

[leger]

Since this is better...Putting on the PDT- radar.

Comment 4

[Akkana Peck]

Unfortunately, no.  I thought I had a fix, but it didn't really fix the problem,
so this is still quite bad.  Please re-evaluate for PDT.

Comment 5

[Akkana Peck]

This is cool: I've discovered that it does indeed work to wrap the quoted
plaintext inside a <pre>, and that overrides the wrap style on the body.  I have
code in my tree changing InsertAsPlaintextQuotation to do this.  That's the good
news.  It seems to solve the output problem, too, without needing a chance to
the output sink stream.
Here's the catch: hitting return inside a <pre> in the editor does not break the
pre, so new text added in the reply will also go in inside the pre and hence
unwrapped.  It should be fairly easy to write an edit rule for mail compose
which splits <pre> tags, though, just like we do for blockquotes now.  I'll look
into that.

Comment 6

[Akkana Peck]

I have a fix for the mail rule issue, too, so now hitting return splits the pre.
 I'm about to check in these fixes (adding this comment in mozilla to make sure
I didn't do anything weird to text area editing). :-)

Comment 7

[leger]

Putting on PDT+ radar.

Comment 8

[Akkana Peck]

This is fixed, and seems to be working nicely in my 11/17 build.

Comment 9

[lchiang]

laurel, can you help to verify this? Thanks.

Comment 10

[laurel]

I'm trying to verify this in the nov23m12 commercial builds on linux and mac.  I
either don't grasp the details of your description of the problem, or I'm seeing
that it is still broken.

Could you please give me a bit of clarification on steps to reproduce?  Thanks.

Comment 11

[Akkana Peck]

To reproduce:
1. Read a plaintext message that has lines wrapped at some normal width, like
72.
2. Reply to the message using plaintext compose.

You should see the quoted lines not wrapped (they're inside a pre tag, so they
may not fit on the screen if they're long) but any new text you type should be
wrapped to the appropriate width.

3. Send the message.

The quoted text should not be wrapped long-short-long-short, but should be
wrapped at the same place the original author wrapped it.  New text should be
wrapped to 72 columns (or whatever the wrap setting is; I'm not sure if that's
currently changeable).

Comment 12

[laurel]

OK. Thanks very much for the steps. I was not reading your original description
right and read some contradictions into what I should see.

OK using nov23m12 commercial builds on linux 6.0, win98 and mac OS 8.5.1.