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

RESOLVED FIXED in Thunderbird 49.0

Status

defect
RESOLVED FIXED
3 years ago
8 months ago

People

(Reporter: atalanttore, Assigned: Paenglab)

Tracking

45 Branch
Thunderbird 49.0
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird46 wontfix, thunderbird47 fixed, thunderbird48 fixed, thunderbird49 fixed, thunderbird_esr4547+ fixed)

Details

Attachments

(2 attachments)

Reporter

Description

3 years ago
Posted file 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.
Reporter

Updated

3 years ago
OS: Unspecified → All
Hardware: Unspecified → All
Reporter

Updated

3 years ago
Version: 38 Branch → 45 Branch
Confirmed.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 2

3 years ago
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.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → INVALID
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
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Summary: Unable to add line breaks in email reply with quoted text → Unable to add line breaks in email reply with quoted text with line-height:0

Comment 4

3 years ago
(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.
Assignee

Comment 5

3 years ago
(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.
I'd expect it to work if you use !important

body {
  line-height: initial !important; 
}
Assignee

Comment 7

3 years ago
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.
Assignee: nobody → richard.marti
Status: REOPENED → ASSIGNED
Attachment #8741468 - Flags: review?(mkmelin+mozilla)
Reporter

Comment 8

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

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

This workaround works fine.

Comment 9

3 years ago
(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?
Assignee

Comment 10

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

Updated

3 years ago
Depends on: 1246500

Comment 11

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

Comment 12

3 years ago
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 ;-)
Attachment #8741468 - Flags: review?(mkmelin+mozilla) → review?(mozilla)
Assignee

Comment 13

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

Comment 14

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

Comment 15

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

Thanks and landed:
https://hg.mozilla.org/comm-central/rev/4173015e1455
Attachment #8741468 - Flags: review?(mozilla) → review+

Updated

3 years ago
Status: ASSIGNED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 49.0
Assignee

Comment 16

3 years ago
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
Attachment #8741468 - Flags: approval-comm-esr45?
Attachment #8741468 - Flags: approval-comm-beta?
Attachment #8741468 - Flags: approval-comm-aurora?

Updated

3 years ago
Attachment #8741468 - Flags: approval-comm-aurora? → approval-comm-aurora+
Comment on attachment 8741468 [details] [diff] [review]
Bug1263207.patch

2aaf5cbd9aaa
Attachment #8741468 - Flags: approval-comm-esr45? → approval-comm-esr45+
[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.
You need to log in before you can comment on or make changes to this bug.