314213 - Visual spacing between quotes lost after auto-downgrade to plain text e-mail. M-C serializer problem.

Status: NEW

(MailNews Core :: Composition, defect)

Description

[Ben Bucksch (:BenB)]

The mail sending routines do not insert appropriate linebreaks between quote and interleaved comment. Looks fine in Composer, but not in viewer and msg source.

Reproduction:
1. Take a format=flowed message (e.g. by writing a format-flowed "plaintext" msg to yourself)
2. click reply, using the html composer
3. select menu options|format|plaintext and HTML
4. hit return at the end of some lines, type a few words
5. send to yourself
6. retrieve and view the msg, as msg source and in viewer using all of msg|body as| plaintext, simple html and original html [1]
7. Take a bugmail (i.e. a real, non-flowed plaintext mail) and repeat steps 2-6

[1] While I'm using a viewer feature here, it shows what would happen when we sent the mail separately as plaintext and HTML; it just saves steps in the reproduction.

Actual result:
I see empty lines between quote and interleaved comment in the Composer, but they are gone after sending.

I'll attach/quote the source and screenshots.

This is clearly a bug in the sending - not viewing - part of Mozilla, proven by the msg source.


It's strange that Simple HTML and Original HTML rendering differs. Simple HTML shouldn't be filtering anything out in this case, there's no CSS in the source either. messageBody.css may be missing, but I don't see anything relevant in there either. Just ignore Simple HTML for now.


Importance:
This is a major problem while using the client. At times, I had to followup to some recipients to make sure they don't miss important one-line interleaved comments.

This is new in Thunderbird 1.5 beta, i.e. a regression, I think from mrbkap. There was a different bug in 1.0.6, see e.g. bug 144998, but the state now is worse than before.

Comment 1

[Ben Bucksch (:BenB)]

Attachment: Screenshots of the 8 cases

Comment 2

[Ben Bucksch (:BenB)]

Attachment: Source, Reply to f=f

Click on attachment to have it rendered somehow in the browser, say view source in the browser to see the source. Plaintext part is (and that's already your bug):

Ben Bucksch wrote:
> fdghdgh
cfvghdfgh
> dfghdsfgh
sdrgsdfg
> dghsdgfh
> dfghh
sdfgsdfg

Comment 3

[Ben Bucksch (:BenB)]

Attachment: Source, reply to bugmail

Same here. Plaintext part:

[e-mail] wrote:
> Do not reply to this
>   
xfghdfgh
> https://bugzilla

Comment 4

[Ben Bucksch (:BenB)]

(In reply to comment #2)
> Click on attachment to have it rendered somehow in the browser

Interestingly enough, it looks fine here for me (Moz 1.7.12), when using the HTML part.

This, however, shows how messed up and unreadable things are:

Ben Bucksch wrote:
> fdghdgh
cfvghdfgh
> dfghdsfgh
sdrgsdfg
> dghsdgfh
> dfghh
sdfgsdfg

This is how many users will see the message, esp. when sent only as plaintext.

--

A few relevant bugs: bug 144998, bug 165077, both reference more relevant bugs.

Comment 5

[Ben Bucksch (:BenB)]

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

Comment 6

[Ben Bucksch (:BenB)]

Looking at the screenshots, I guess a simple fix would be to output an empty line before and after quotes, but we'd need to see if that produces the right result in all cases.

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Except that will just bring back bug 144998 (if I understand your suggestion correctly). I'm not convinced that this isn't an editor problem... I think our previous behavior (before the patch that I checked in) was actually closer to what we "should" have been outputting, and that the fix in that bug was a hack to special-case mail quoting. Maybe we should output an empty line only after the last level of quotes has been closed.

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Something like this

I can't test this at the moment, but could you try this and see if it helps, Ben?

Comment 9

[Philip Chee]

I don't have a build tree, but if someone can check this in to the Seamonkey trunk, I can test the patch out

Comment 10

[Magnus Melin [:mkmelin]]

(In reply to comment #8)
> Created an attachment (id=201176) [edit]
> Something like this
> 
> I can't test this at the moment, but could you try this and see if it helps,

With this patch I get the needed extra space on top, but not after the inserted line(s) in middle of the text.

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

It turns out that this bug is very hard because of the way that editor creates the document. Fixing this bug would cause e-mails with:
Person A wrote:

> something

reply

> somethinge else...

So to fix this, editor will need to give the serializer more of a clue where to insert extra newlines.

Comment 12

[Ernesto Baschny]

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

Comment 13

[Ernesto Baschny]

The CSS in bug 307112 will force me to manually add a new-line after the quote. But this will generate HTML like:

<blockquote>
quoted text
</blockquote>
<br>
Test<br>
<br>
<blockquote>
quoted text
</blockquote>

With a correct looking plain-text:

> quoted text

Test

> quoted text

But now imagine a regular and correctly configured client, which already adds margins around blockquotes. Now my HTML-mail looks ugly with double margins around the quotes.

I could live more with bug 144998 (something which I never noticed) than with this new one that was introduced by the fix. Could this be reverted?

Comment 14

[Mike Cowperthwaite]

As Ernesto noted at the dupe, if the composed text were created using <p> instead of just stuffing text into the <body>, the serializer would automatically generated spacing around the paragraphs.

Comment 15

[Ernesto Baschny]

Any solution to this problem yet?

The easiest solution would be to make the "Paragraph Style" the default style (or at least make the default configurable) for composing HTML-styled mails (instead of "Normal text"). I currently have to change every reply-line manually to "Paragraph style" to get proper plain-text output, and there isn't even a shortcut to do that (or is there?).

Comment 17

[Magnus Melin [:mkmelin]]

Yeah, I wonder if bug 330891 would help...

Comment 20

[Dan Mosedale (:dmosedale, :dmose)]

I see this in trunk Thunderbird from time to time when simply replying to HTML messages without doing any manual mode switches in composer.  

It looks really crummy and unprofessional, so marking as blocking Tb 3.

Comment 23

[Magnus Melin [:mkmelin]]

Blake: is this something you're working on?

Comment 24

[Blake Kaplan (:mrbkap) (inactive)]

Not actively, no.

Comment 26

[David Ascher (:davida)]

As much as I'd like this bug fixed, I don't think it would block tb3.  Moving to wanted.

Comment 29

[Kamil Burzynski]

6-year old bug and still biting ;(

Comment 31

[Thomas D. (:thomas8)]

Ben, you removed Blocks: bug 889315 which I have just set.

Imo, this bug 314213 clearly matches the definition of bug 889315, which is about delivery format ux problems, i.e. difference between what user sees during composition (nicely formatted HTML), and what gets sent by TB (plaintext with no or substitute formatting), as depicted in your excellent screenshot compilation of attachment attachment 201137 [details].

Can you pls explain why you removed this bug from the dependency list of delivery format ux tracker bug 889315?

Comment 32

[Jorg K (CEST = GMT+2)]

Attachment: Replying to a plain text flowed message with a HTML message.

I replied to a flowed plain text message with an HTML message and viewed the received result in three different ways: Result: No problem.

Replying to a bug mail has the issue that the content is inside a <pre> block. In any case, your result from attachment 201137 [details] cannot be reproduced.

Can I close this bug or can you explain what the problem is?

Comment 33

[Jorg K (CEST = GMT+2)]

Looks like the reporter, who is busily discussing on tb-planning, lost interest in the bug. Since I can't reproduce the problem reported in 2005, I'm closing the bug.

Comment 34

[Boris 'pi' Piwinger]

The problem is still there. It is as shown in the originally attached screenshots from 2005. Steps to reproduce:

1) Answer to any e-mail with the HTML editor
2) Somewhere in the quoted text press enter, so that you can enter new text between the quotes
3) Enter text
4) Observe that space is displayed between a) the quote above the text and the new text and b) between the new text and the quoted text below
5) Send message as text
6) Observe that there is no space between quoted text and new text.

And BTW: closing a bug because of the reporter is strange. Many bugs have been duped on this one.

Comment 35

[Jorg K (CEST = GMT+2)]

Attachment: Replying to a plain text flowed message with a plain text message.

(In reply to Boris 'pi' Piwinger from comment #34)
> And BTW: closing a bug because of the reporter is strange. Many bugs have
> been duped on this one.

Thanks for your reply. I am sorry, I didn't read the description properly. This bug is about the effects of the HTML to plain text auto-downgrade, nothing else.

Closing the bug at least got some response ;-)

I followed the STR and sent a reply to a flowed plain text mail which I created in the HTML editor, and this reply was automatically auto-downgraded, which loses a bit of the spacing as can be seen in my picture.

This is totally expected and a result of the lossy auto-downgrade. There is no better way to auto-downgrade this. The space you see in the HTML editor is "spacing" between the two different CSS styles, there are no newlines which could the serialised/down-converted to newlines. I can see that there is a patch in attachment 201176 [details] [diff] [review] which tries to change the serialiser, but I think this is wrong.

Replying to a non-flowed plain text e-mail (like the ones from Bugzilla) the effect is more drastic. The quoted original e-mail is inserted into a <pre> block and adding newlines there suggests huge spacing, but in the sent e-mail there is no space at all between the lines. I can see that this is unexpected.

Again, this would need to be fixed in the serialiser code in M-C, which is unowned and unmaintained.

This bug should be in Core::Serializers.

Personally, I'd mark this "wontfix". If you don't want auto-downgrade, simply switch it off, there is an option now, see bug 136502.

Comment 36

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (GMT+1) from comment #35)
> Replying to a non-flowed plain text e-mail (like the ones from Bugzilla) the
> effect is more drastic. The quoted original e-mail is inserted into a <pre>
> block and adding newlines there suggests huge spacing, but in the sent
> e-mail there is no space at all between the lines. I can see that this is
> unexpected.

There is also an M-C editor problem when adding a newline in this HTML:

<html>
  <head>

    <meta http-equiv="content-type" content="text/html; charset=windows-1252">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <br>
    <br>
    <div class="moz-cite-prefix">On 10/12/2015 11:24, Jörg Knobloch
      wrote:<br>
    </div>
    <blockquote class=" cite"
      id="mid_2bcab04e_d24c_7184_fac7_f4a23de4bb1d_jorgk_com"
      cite="mid:[e-mail]"
      type="cite">
      <pre wrap="">Line 1
Line 2 Line 2 Line 2 Line 2 Line 2 Line 2 Line 2 Line 2 Line 2 Line 2
Line 3 Line 3 Line 3 Line 3 Line 3 Line 3 Line 3 Line 3 Line 3 Line 3
Line 4
Line 5
</pre>
    </blockquote>
    <br>
    <br>
  </body>
</html>

Now inserting a <br> after line 1:

<html>
  <head>

    <meta http-equiv="content-type" content="text/html; charset=windows-1252">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <br>
    <br>
    <div class="moz-cite-prefix">On 10/12/2015 11:24, Jörg Knobloch
      wrote:<br>
    </div>
    <blockquote class=" cite"
      id="mid_2bcab04e_d24c_7184_fac7_f4a23de4bb1d_jorgk_com"
      cite="mid:[e-mail]"
      type="cite">
      <pre wrap="">Line 1</pre>
    </blockquote>
    <br>
    <blockquote class=" cite"
      id="mid_2bcab04e_d24c_7184_fac7_f4a23de4bb1d_jorgk_com"
      cite="mid:[e-mail]"
      type="cite">
      <pre wrap=""> <======= Error here, there *must* not be another newline after the <pre> tag.
Line 2 Line 2 Line 2 Line 2 Line 2 Line 2 Line 2 Line 2 Line 2 Line 2
Line 3 Line 3 Line 3 Line 3 Line 3 Line 3 Line 3 Line 3 Line 3 Line 3
Line 4
Line 5
</pre>
    </blockquote>
    <br>
    <br>
  </body>
</html>

Summary:

As far as I can see, the M-C serialiser works as expected. The effects observed are due to lossy auto-downgrade.
There is a bug in the M-C editor (unmaintained and unowned module) when inserting a <br> into a <blockquote><pre> block:
Starting with

  <blockquote>
  <pre>line 1
  line 2
  </pre>
  </blockquote>

we get

  <blockquote>
  <pre>line 1</pre>
  </blockquote>
  Inserted line<br>
  <blockquote>
  <pre>
  line 2
  </pre>
  </blockquote>

but correct would be

  <blockquote>
  <pre>line 1</pre>
  </blockquote>
  Inserted line<br>
  <blockquote>
  <pre>line 2    <=== Look here
  </pre>
  </blockquote>

Comment 37

[Boris 'pi' Piwinger]

The key here is the user expectation WYSIWYG. If there is a space shown in the editor (as you say between different CSS styles), then it needs to there in the sent message.

Also the idea of downgrading is not the right approach. If I reply to text/plain, there is no need an HTML editor comes up, but if it does, it needs to work as WYSIWYG. 

If the editor would not show the spacing (obviously, this can be realized by CSS), there would be no confusion.

Comment 38

[Jorg K (CEST = GMT+2)]

(In reply to Boris 'pi' Piwinger from comment #37)
> Also the idea of downgrading is not the right approach. If I reply to
> text/plain, there is no need an HTML editor comes up, but if it does, it
> needs to work as WYSIWYG. 

The system is WYSIWYG, only auto-downgrade catches you. Now you can switch it off, bug 136502.

If you want to reply to a plain text message in plain text, press "shift reply". What to see is what the recipient will get.

If your account is configured to send HTML, then the default is an HTML reply which is 100% WYSIWYG unless it's down-graded. Again, switch that off if you don't want this to happen.

Obviously plain text can't show CSS spacing. So whenever there's a downgrade, you lose WYSIWYG.

The only bug to fix here is the M-C editor bug when injecting a new line into a <blockquote><pre>. But that only happens for non-flowed plain text mail. Note that plain text is sent flowed by default.

Comment 39

[Boris 'pi' Piwinger]

This is not about downgrading, it is about involuntary upgrading in the first place. It would be wrong to switch off the "downgrading", it is bad style to answer plain text with HTML.

It is very simple: If styles show space, add empty lines.

Comment 40

[Jorg K (CEST = GMT+2)]

(In reply to Boris 'pi' Piwinger from comment #39)
> it is about involuntary upgrading in the
> first place. It would be wrong to switch off the "downgrading", it is bad
> style to answer plain text with HTML.
Use shift reply and you won't have a problem.

> It is very simple: If styles show space, add empty lines.
That's unrealistic. The space shown is less than the height of one line. And CSS features are not taken into account when serialising.

All you could do is use a different style sheet when HTML-answering a plain text e-mail. Then the spacing wouldn't show in the first place.

Comment 41

[Thomas D. (:thomas8)]

(In reply to Boris 'pi' Piwinger from comment #37)
> The key here is the user expectation WYSIWYG. If there is a space shown in
> the editor (as you say between different CSS styles), then it needs to there
> in the sent message.
> 
> Also the idea of downgrading is not the right approach. If I reply to
> text/plain, there is no need an HTML editor comes up, but if it does, it
> needs to work as WYSIWYG. 
> 
> If the editor would not show the spacing (obviously, this can be realized by
> CSS), there would be no confusion.

Good points Boris, although it's never easy to get this right for everyone.
I don't have time for this right now, but I think this bug has been correctly reopened for exactly those reasons mentioned by Boris (esp. ux-wysiwyg). At least you can now turn auto-downgrade off.