39372 - Blockquoting plain text

Status: VERIFIED FIXED

(MailNews Core :: Composition, enhancement, P3)

Description

[Ben Bucksch (:BenB)]

With bug 31906, we can recognize nested quotes and display the in HTML style.
WeE should quote them in HTML style as well. This helps tremendously with
(re)wrapping, is easier to recognize and looks better.

Code bascially works already with patches for bug 31906, try it out by setting
"mail.quoteasblock" to true. Remaining problems:
- Too much whitespace: Remove all linebreaks at the beginning and and of a <pre>
block before inserting into the editor.
- Some other minor problem I don't remember right now.
- Test, test, test (already did for ~1 day), e.g. with other mailers. You
(listeners/lurkers) can help here.

Comment 1

[Ben Bucksch (:BenB)]

Akk, J-F, what do you think about this?

Comment 2

[Jean-Francois Ducarroz]

add rhp to the cc list

Comment 3

[Ben Bucksch (:BenB)]

I think, the first issue I mentioned above is not really a stopper for enabling
the feature, because the whitespace around quotes is still messed in all modes
anyway (e.g. see bug 38492). Of course, that doesn't mean, I won't fix the extra
whitespace. I just don't think, we should stop this feature because of it and
risking its inclusion into final.

The code has been checked in with bug 31906. You can enable it with
|user_pref("mail.quoteasblock", true);|. I urge everybody to try it out and so
to help testing interoperability (and operability in general :) ).

OTOH, I think, it is a big improvement for HTML mail.

Should we enable this by default now to get wider testing or should I ask for
testers on .mailnews first and wait 1 or 2 weeks until enabling it as default?

(BTW: For those concerned about the safety of the recognition: Lines must start
with a (mixed) sequence of ">" or "> ", while the last ">" must have a space
after it. E.g.
> > bla
>> bla
>> > bla
are recognized, but not
 > bla
>bla
> >bla
I consider this being pretty safe.)

Comment 4

[Ben Bucksch (:BenB)]

I'm assuming the OK of the mailnews team here, of course. I'll speak with
putterman.

Comment 5

[Ben Bucksch (:BenB)]

One more problem: 4.x' editor doesn't like the <div>s. I'll just remove them for
now (until we have a better solution). Minor tweak.

As for the "too much newlines" problem: I don't even know if this is actually a
bug. The newlines around <blockquote>s the editor puts out is odd anyway. This
bug needs to be fixed before I know if something/what has to be done. Until
then, we may just have 2 empty lines between 2 quotes - currently a minor
problem.

Summary: IMO, we can enable this now. Waiting for decision.

Comment 6

[Ben Bucksch (:BenB)]

OK, no answer till now, so I'm assuming OK unless somebody objects. Will create
patch. Akk, would you then review, please?

Comment 7

[Ben Bucksch (:BenB)]

Attachment: Remove <div>s, version 1

Comment 8

[Ben Bucksch (:BenB)]

Attachment: Change default pref, version 1

Comment 9

[Ben Bucksch (:BenB)]

The "Remove <div>s"-patch is for libmime, but is just removing the div in the
quoting case, adding quotes around the class in the other cases and changing
some comments. akk, rhp: Could one of you please review it (and deny, if not)?

The "Change default pref"-patch is the "real" change, it turn the feature on by
default. Akk, could you please review that?

Comment 10

[Ben Bucksch (:BenB)]

Ducarroz, if you want to review, that is fine, too.

Comment 11

[Daniel Bratell]

Will the removal of <div> stop the "This message contains HTML. How do you want 
to send it?"-message for pure text messages?

Comment 12

[Ben Bucksch (:BenB)]

Daniel, yes. See also bug 44552.

Comment 13

[Jean-Francois Ducarroz]

Rich is the right persone to review this fix. Rich can you do it?

Comment 14

[Ben Bucksch (:BenB)]

Rich reviewed the change in mimetpla.cpp. Leaving is "just" the change of the
default pref. As this pref defines with what we feed the composer (by default),
I would argue, an editor person is the best reviewer, not?

Comment 15

[Ben Bucksch (:BenB)]

I interpret
<quote src="http://www.newsreaders.com/gnksa/gnksa.txt, Requirements 14">
To get well-readable articles, the user SHOULD be provided with the
possibility to rewrap excessively long lines of quoted text, respecting
quotation -- i.e. have the option to correct `inherited' bad formatting.
</quote>
to support this bug.

Comment 16

[Ben Bucksch (:BenB)]

a=alecf. Checked in. Yippie!

Comment 17

[pmock]

Changing qa assign.

Comment 18

[pmock]

Verified on win32 and linux using the following builds:
 win32 commercial seamonkey build 2001-010821-mtrunk installed on P500 Win98
 linux commercial seamonkey build 2001-011911-mtrunk installed on P200 RedHat 
6.2

Note: per Ben "It should change the quote in the *HTML* composer, not the 
plaintext composer, after replying to a real plaintext msg."

but failed on macos.  The user pref "mail.quoteasblock" as no effect on mac.  I 
will open separate bug to track.  

Comment 19

[pmock]

Logged parity bug on Mac
http://bugzilla.mozilla.org/show_bug.cgi?id=66349