Closed Bug 1568121 Opened 3 months ago Closed 3 months ago

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

Categories

(Thunderbird :: Instant Messaging, defect)

defect
Not set

Tracking

(thunderbird69 fixed, thunderbird70 fixed)

RESOLVED FIXED
Thunderbird 70.0
Tracking Status
thunderbird69 --- fixed
thunderbird70 --- fixed

People

(Reporter: jorgk, Assigned: jorgk)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

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.

Flags: needinfo?(richard.marti)
Flags: needinfo?(alessandro)

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

Flags: needinfo?(richard.marti)

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?

Flags: needinfo?(alessandro)

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

Flags: needinfo?(alessandro)

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).

Flags: needinfo?(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.

Flags: needinfo?(emilio)

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.

Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Flags: needinfo?(alessandro)
Attachment #9081759 - Flags: review?(richard.marti)
Attachment #9081759 - Flags: feedback?(emilio)
Attachment #9081759 - Flags: approval-comm-beta?
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.
Attachment #9081759 - Flags: review+
Comment on attachment 9081759 [details] [diff] [review]
1568121-line-height.patch

Looks good and works for me. Thanks.
Attachment #9081759 - Flags: review?(richard.marti) → review+

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

Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 70.0
Attachment #9081759 - Flags: feedback?(emilio)
Duplicate of this bug: 1570699
Attachment #9081759 - Flags: approval-comm-beta? → approval-comm-beta+
You need to log in before you can comment on or make changes to this bug.