43455 - Quoted empty lines in flowed mails aten

Status: VERIFIED FIXED

(MailNews Core :: MIME, defect, P3)

Description

[Ben Bucksch (:BenB)]

Reproduce:
1. Read the attached mail

Actual result:
No empty lines in the quote shown

Expected result:
Some empty lines in the quote shown

Comment 1

[Ben Bucksch (:BenB)]

Attachment: Testcase - Empty lines before "These" and "1."

Comment 2

[Ben Bucksch (:BenB)]

Attachment: Fix

Comment 3

[Ben Bucksch (:BenB)]

Attached patch.

<quote>
// Look if the last character (after stripping ending end
// of lines) is a SPACE. If itg is, we are looking at a
// flowed line.
</quote>
This is not true for empty lines in quotes, which may look like ">> ". So, I
moved this part down the quote handling code, which determined this quoting
stuff, and then exclude the quoting stuff. (I just noticed, that I forgot to
correct the comment. The code should be OK, however.)

Patch also contains some code cosmetic.

Daniel, can you do a code review (since you cannot compile currently), please?

Comment 4

[Daniel Bratell]

> empty lines in quotes, which may look like ">> ".

No they may not. 
Quote from RFC 2646 4.2:
"If the line ends in one or more spaces, the line is flowed"

The only exception is in 4.3, Usenet signatures "-- ". 

Comment 5

[Ben Bucksch (:BenB)]

> No they may not.

Then, nsHTMLToTXTConv is broken, because this is what it produces for empty
lines in quotes.

> The only exception is in 4.3, Usenet signatures "-- ".

I disagree. While RFC2646 doesn't explicitly say something about this situation,
I would interpret
<quote src="ftp://venera.isi.edu/in-notes/rfc2646.txt">
4.4.   Space-Stuffing
[...]
   On reception, if the
   first character of a line is a space, it is logically deleted.  This
   occurs after the test for a quoted line, and before the test for a
   flowed line.
   [...]
   Other lines MAY be space-stuffed as desired.
[...]
4.5.  Quoting
[...]
   Note that because of space-stuffing, the lines
       >> Exit, Stage Left
   and
       >>Exit, Stage Left
   are semantically identical; both have a quote-depth of two, and a
   content of "Exit, Stage Left".
</quote>
in the way that ">> " is a space-stuffed, quoted and empty line and thus not
flowed.

Comment 6

[Daniel Bratell]

Hmm. I think you're right, and now I understands your patch. There were too many 
whitespace changes for me to see what you had done at first. You moved the space 
stuff removal so that it is done before the check if the line is flowed or not. 
That must be right.

I see now that RFC2646 explicitly says that you may space stuff any line and if 
someone wants to space stuff the empty line, we should be prepared for that. 

I think I will crawl back into my little hole. :-)

Comment 7

[Ben Bucksch (:BenB)]

> There were too many whitespace changes for me to see what you had done
> at first.

I know, sorry.

> I think I will crawl back into my little hole. :-)

Please don't. I appreciate your reviews :).

Comment 8

[Ben Bucksch (:BenB)]

Attachment: fix, version 2

Comment 9

[Ben Bucksch (:BenB)]

Fix now also prevents a memleak in a failure situation, thanks to daniel.

Comment 10

[Ben Bucksch (:BenB)]

Fix checked in.

Comment 11

[Ryan Cassin]

Verified in 9/29 build.