1568121 - Fix fixed line height of 20 in mail/components/im/content/chat-conversation.js

Status: RESOLVED FIXED

(Thunderbird :: Instant Messaging, defect)

Description

[Jorg K (CEST = GMT+2)]

As per bug 1564443 comment #11.

Florian is right, the hard-coded constant doesn't work.

In the Display Options set the font size for "Other Writing Systems" to a large value and observe that the height of the chat typing area doesn't increase.

Comment 1

[Richard Marti (:Paenglab)]

How can the height be calculated? See bug 1564443 comment #4.

Comment 2

[Alessandro Castellani [:aleca]]

Indeed, a fixed value doesn't work.

Some solutions I haven't tested but we could consider:

• Get the current font size of the conversation box and set the height accordingly.
• Get the current height of the textarea and set the conversation box height accordingly.
• Set the conversation height via CSS with rem in order to adapt to its scaled font size (would that work?).

Thoughts?

Comment 3

[Patrick Cloke [:clokep]]

Alex, can you make sure that this gets fixed? Thanks!

Comment 4

[Jorg K (CEST = GMT+2)]

Emilio, any idea how to replace the text height calculation we removed here:
https://hg.mozilla.org/comm-central/rev/ad5905771f43#l1.12

Do any of the suggestions in comment #2 sound OK? I'm a bit confused be the nomenclature here. What's the conversation box? The messages/posts/lines which have already been sent, so above the input box?

Which "textarea" are you referring to in the second suggestion? And then "conversation box" means something else in that suggestion?

No idea what rem is. root em, hmm, never heard of it.

(Wow, Emilio has 37 pending NI's).

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Calling getComputedStyle().lineHeight, and if that returns pixels, use that, otherwise if it's normal, font-size * 1.2 should be a pretty fair estimate.

Comment 6

[Jorg K (CEST = GMT+2)]

Attachment: 1568121-line-height.patch

I've discussed this with Emilio on IRC.

cs["font-size"], cs.getPropertyValue("font-size"), cs.fontSize are all the same, same for line-height and .lineHeight.

I tested this with some debugging and when "normal" is returned, we calculate 20.4.

Other than that, the values can be real numbers, so I've changed the parseInt to parseFloat as suggested by Emilio.

Comment 7

[Florian Quèze [:florian]]

Comment on attachment 9081759 [details] [diff] [review]
1568121-line-height.patch

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

Drive-by r+ as this looks good to me by code inspection. Thanks for fixing this!

Leaving Paenglab's r? flag as I haven't tested this locally and I don't know if you intended to do it as part of reviewing.

Comment 8

[Richard Marti (:Paenglab)]

Comment on attachment 9081759 [details] [diff] [review]
1568121-line-height.patch

Looks good and works for me. Thanks.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/57ed654fd2e0
restore line height calculation after bug 1564443 (consulting by :emilio). r=Paenglab,florian

Comment 11

[Jorg K (CEST = GMT+2)]

TB 69 beta 2:
https://hg.mozilla.org/releases/comm-beta/rev/0274467b8c7815966ebd56bda53c2ebeafb25509