1263207 - Unable to add line breaks in email reply with quoted text with line-height:0

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Ettore Atalan]

Attachment: OneDrive.eml

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20160315153207

Steps to reproduce:

1. Open the attached EML file. (Don't worry, no malware.)
2. Click on "Reply".
3. Try to add some line breaks above or below the quoted text.


Actual results:

Thunderbird doesn't add line breaks above or below the quoted text when I press 'Enter'.


Expected results:

Line breaks accordingly to the amount of pressed 'Enter' keys.

Comment 1

[Magnus Melin [:mkmelin] (away, back Aug 4)]

Confirmed.

Comment 2

[Jorg K (CEST = GMT+2)]

Caused by Microsoft's horrible HTML contained in this message, especially the line-height:0.
With line-height set to zero, all lines entered collapse. If you replace all occurrences of line-height:0 with - say - line-height:12px it all works.

Say hello to Bill, Steve and Satya if you see them.

Comment 3

[Magnus Melin [:mkmelin] (away, back Aug 4)]

Bad css, but the bug sounds valid. 
I read setting a HTML5 doctype we'll get a minimum line-height for inline elements. In that case bug 335499 would fix part of it. Or maybe we could just set line height for the composition to something reasonable. and !important

Comment 4

[Jorg K (CEST = GMT+2)]

(In reply to Magnus Melin from comment #3)
> Or maybe we could
> just set line height for the composition to something reasonable. and
> !important
Maybe Richard is interested.

Comment 5

[Richard Marti (:Paenglab)]

(In reply to Magnus Melin from comment #3)
> Or maybe we could just set line height for the composition to something
> reasonable. and !important

I tried

body {
  line-height: initial;
}

in messageQuotes.css in lack of a better place and it worked. But this works only in compose window. After sending the message this override is no more functional and the line-height is back to 0 because the inline CSS is still set to 0.

I think what would be needed is either a logic which exchanges the body{line-height:0; to body{ (removing the line-height:0;) or adds a inline style body{line-height:initial !important;} to the message.

Comment 6

[Magnus Melin [:mkmelin] (away, back Aug 4)]

I'd expect it to work if you use !important

body {
  line-height: initial !important; 
}

Comment 7

[Richard Marti (:Paenglab)]

Attachment: Bug1263207.patch

Yes, the !important is needed for reading the message. But my concern was for other readers which still only respect the line-height:0.

This patch will not have an effect on Daily until Bug 1246500 is fixed or my patch from it is applied.

Comment 8

[Ettore Atalan]

Comment on attachment 8741468 [details] [diff] [review]
Bug1263207.patch

Review of attachment 8741468 [details] [diff] [review]:
-----------------------------------------------------------------

This workaround works fine.

Comment 9

[Jorg K (CEST = GMT+2)]

(In reply to Richard Marti (:Paenglab) from comment #7)
> This patch will not have an effect on Daily until Bug 1246500 is fixed or my
> patch from it is applied.
I'd be happy to review this since Magnus has a few reviews to do, but I need to understand what you're saying here. This needs the patch from bug 1246500? Why?

Comment 10

[Richard Marti (:Paenglab)]

messageQuotes.css is not loaded in messagebody because of security restrictions which bug 1246500 should fix.

Comment 11

[Jorg K (CEST = GMT+2)]

OK, looks like bug 1246500 will be fixed soon. Then we can come back to this.

Comment 12

[Jorg K (CEST = GMT+2)]

Comment on attachment 8741468 [details] [diff] [review]
Bug1263207.patch

Stealing the review since Magnus has enough of them and we're implementing his suggestion anyway ;-)

Comment 13

[Richard Marti (:Paenglab)]

Jörg, now that bug 1269254 has landed, could you look at his?

Comment 14

[Jorg K (CEST = GMT+2)]

Sure. Within the next 24 hours. I get a daily "overdue review" reminder, so I haven't forgotten.

Comment 15

[Jorg K (CEST = GMT+2)]

Comment on attachment 8741468 [details] [diff] [review]
Bug1263207.patch

Thanks and landed:
https://hg.mozilla.org/comm-central/rev/4173015e1455

Comment 16

[Richard Marti (:Paenglab)]

Comment on attachment 8741468 [details] [diff] [review]
Bug1263207.patch

[Approval Request Comment]
User impact if declined: line height issues on replies of MS Outlook mails
Testing completed (on c-c, etc.): on c-c
Risk to taking this patch (and alternatives if risky): low

Comment 17

[Jorg K (CEST = GMT+2)]

Aurora (TB 48):
https://hg.mozilla.org/releases/comm-aurora/rev/0bf35c0be66d

Comment 18

[Kent James (:rkent)]

Comment on attachment 8741468 [details] [diff] [review]
Bug1263207.patch

http://hg.mozilla.org/releases/comm-beta/rev/d48976525021

Comment 19

[Kent James (:rkent)]

Comment on attachment 8741468 [details] [diff] [review]
Bug1263207.patch

2aaf5cbd9aaa

Comment 20

[Stanimir Stamenkov]

[Sorry for the spam – just want to leave a remark for possible future development.]

Strictly speaking:

    body {
      line-height: inherit !important;
    }

is a better rule as it honors any line-height customization applied on the parent element.  I'm using an user style to increase the initial (normal) line-height by applying it to the :root of documents.  The current workaround:

    body {
      line-height: initial !important;
    }

strictly overrides my preference.  I imagine the default line-height might be controlled via UA style <about:PreferenceStyleSheet> which would be defeated by the employed workaround, also.

I know I could workaround the current workaround, but it would be better and it could be made less obtrusive to start with.