Closed Bug 1583415 Opened 5 years ago Closed 5 years ago

HTML Mail Question dialog pops up in wrong size, and misses the SEND/CANCEL button (are they hidden?)

Categories

(Thunderbird :: Message Compose Window, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 71.0

People

(Reporter: ishikawa, Assigned: Paenglab)

References

Details

Attachments

(5 files, 2 obsolete files)

I am using linux, SMP Debian 5.2.9-2 (2019-08-21) x86_64 GNU/Linux.
As I mention in the subject, there are times (so far two or three times. This is the second day I observe the bug. The first one occurred last week, and my memory is a bit hazy.) when the HTML Mail Question (that asks regarding the HTML formatting for recipients who are not specified as being able to receive HTML mails) pops up, the size of the dialog window is incorrect and hides the [CANCEL]/[SEND] button.

I am attaching the screen capture when it happened today.
( I now notice there seem to be the upper corner of the buttons that got hidden in the too small window.)

When I close the incorrectly sized window by clicking on the X of the window pane, and retried sending the e-mail, the pop up windows showed up in correct size and I could see [CANCEL] [SEND] buttons.

Very annoying, especially for the first time.

Fairly sure this has been happening for years. There may be an earlier report but I have not found it yet.
As you mention it only shows if you are sending to a recipient who does not have formatting specified.
Is the dialog resizable?

(In reply to Ian Neal from comment #2)

Fairly sure this has been happening for years. There may be an earlier report but I have not found it yet.
As you mention it only shows if you are sending to a recipient who does not have formatting specified.
Is the dialog resizable?

Good question.

It seems that this incorrect size does happen quite often.
(I mean I have many correspondents some of whom do not receive HTML and thus this confirmation dialog popped up quite often. But I don't see this incorrect size until last week.)

I just tried to send an e-mail to someone and the incorrectly sized popup appeared.

I could resize it and when I enlarged it, I could see the CANCEL and SEND button.

HOWEVER, I don't think I notice this before this version 68.1.0 (previously it was 68.0.x, I think.)
Maybe rewriting for 68.1.0 caused the problem to appear more often.

Anyway, if someone sees it for the first time, like I did last week, this is very annoying, and I was at a loss about what to do next to send the e-mail.

Richard, please take a look.

Flags: needinfo?(richard.marti)
Attached image dialog-20em-height.png

On Windows it looks okay. On Linux I see the issue.
It is because the font is wider than on Windows. This makes the lines wrap and this shifts the buttons outside the dialog because the dialog height is calculated without wrapping. This could also be a issue on locales with longer wording than English on Windows.

My screenshot shows the dialog with set the height to 20em. Then it looks okay. What do you think?

Flags: needinfo?(richard.marti)

And this how it looks on Windows with the height: 20em;

Alessandro, what do you think about this height: 20em; ?

Flags: needinfo?(alessandro)

It looks good.

Flags: needinfo?(alessandro)

Okay, added the height: 20em;

Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #9094996 - Flags: review?(alessandro)
Attachment #9094996 - Flags: approval-comm-esr68?
Attachment #9094996 - Flags: approval-comm-beta?
Comment on attachment 9094996 [details] [diff] [review]
1583415-askSendFormat-height.patch

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

Unfortunately I don't think this solution can solve all the various cases.
In my case for example I have a HiDPI monitor with scaled up font size, and with the height specified, the buttons are partially outside the dialog.

I think we could probably solve this with the `window.sizeToContent()` method, once the dialog is loaded and all the content is there, instead of manually specifying the height.

I did some tests and by adding a timeout of 50ms, the dialog resizes properly.
```
setTimeout(() => {
  window.sizeToContent();
}, 50);
```

What do you think Richard?
Attachment #9094996 - Flags: review?(alessandro) → review-

(In reply to Alessandro Castellani (:aleca) from comment #10)

Comment on attachment 9094996 [details] [diff] [review]
1583415-askSendFormat-height.patch

Review of attachment 9094996 [details] [diff] [review]:

Unfortunately I don't think this solution can solve all the various cases.
In my case for example I have a HiDPI monitor with scaled up font size, and
with the height specified, the buttons are partially outside the dialog.

I think we could probably solve this with the window.sizeToContent()
method, once the dialog is loaded and all the content is there, instead of
manually specifying the height.

I did some tests and by adding a timeout of 50ms, the dialog resizes
properly.

setTimeout(() => {
  window.sizeToContent();
}, 50);

Where have you added this? I tried it and it didn't work.

Flags: needinfo?(alessandro)

(In reply to Alessandro Castellani (:aleca) from comment #10)

Comment on attachment 9094996 [details] [diff] [review]
1583415-askSendFormat-height.patch

Review of attachment 9094996 [details] [diff] [review]:

Unfortunately I don't think this solution can solve all the various cases.
In my case for example I have a HiDPI monitor with scaled up font size, and
with the height specified, the buttons are partially outside the dialog.

I think we could probably solve this with the window.sizeToContent()
method, once the dialog is loaded and all the content is there, instead of
manually specifying the height.

I did some tests and by adding a timeout of 50ms, the dialog resizes
properly.

setTimeout(() => {
  window.sizeToContent();
}, 50);

What do you think Richard?

IMHO, the automagical resizing is better: We will definitely have someone in an esoteric locale who would see this problem eventually.

Come to think of it, on my linux installation, the fonts picked up by TB seem to have CHANGED with 68.
I now see a more sharper/bolder font for folder names [all in ASCII characters], which happened to be easier to read on my HiDPI monitor. But the subject lines got a bit flimsy and not quite aesthetically acceptable: I am going to find out what has gone wrong.
Under Linux, or X more specifically, it is an acrobatic work to figure out what font is exactly used for a particular character :=(
This is also the problem with drawing dialog. You can't easily figure out the height of characters that you will need unless you have an API like window.sizeToContent(), which is a good to thing to have around.

I am afraid there may be many similar dialogs which might be affected in a similar manner by this gratuitous font change in 68 under linux (and each installation may be quite different, ugh). However, in a sense, we are forced to use proper and correct size for such dialogs due to this subtle font changes, and the users would report such inappropriate size in droves for dialogs that are used often as I did.
So we should be OK over time.

I will report if I find any other dialog that gets skewed due to font size issues.

(In reply to Richard Marti (:Paenglab) from comment #11)

Where have you added this? I tried it and it didn't work.

I added it here: https://searchfox.org/comm-central/rev/f9b94449f75d2e465c1a25a7b01f8ba69c3f04ab/mailnews/compose/content/askSendFormat.js#56

Inside the Startup() function of the askSendFormat.js file.

Flags: needinfo?(alessandro)
Attached patch 1583415-askSendFormat.patch (obsolete) — Splinter Review

Does this work for you? It does for me.

Attachment #9095266 - Flags: review?(richard.marti)
Comment on attachment 9095266 [details] [diff] [review]
1583415-askSendFormat.patch

It's exactly like my patch I tried. The strange thing is, that it doesn't always work. When it doesn't work, I need to change the height manually then it works on the following tries. But after some restarts it doesn't grow again.

It looks also a bit weird how the dialog appears and after a short bit grows.

Mh, interesting.
Maybe we stumbled upon a deeper problem related to data loading rather than a simple dialog height.

Which data is loaded and not coming straight from the DTD file? The radio button options?
Maybe that method has some sort of random delay that causes the issue?

Attachment #9094996 - Flags: approval-comm-esr68?
Attachment #9094996 - Flags: approval-comm-beta?

What do we want to do now?

Attached patch 1583415-askSendFormat.patch (obsolete) — Splinter Review

This is a bit of an overkill, and probably a bit silly, but it seems to solve the problem for good.
I created an observer and attached to the description that gets filled with bundle.getString() (this is the method that causes the size issue).
Once the text is applied, I remove the disabled attribute from the description, which triggers the observer, which triggers the sizeToContent() method with a 20ms delay.
I don't see any height jump and it seems to work all the times.

Thoughts?

Attachment #9095266 - Attachment is obsolete: true
Attachment #9095266 - Flags: review?(richard.marti)
Attachment #9095374 - Flags: review?(richard.marti)
Comment on attachment 9095374 [details] [diff] [review]
1583415-askSendFormat.patch

Hmm strange, it still doesn't have all the time the correct height here (tested on Ubuntu VM). :-(

So, what could we do? Maybe combine our patches with the height (or min-height) set and then use sizeToContent() ?

Or maybe the two other <description> need a similar treatment as they do also wrap here. And this wrapping is the problem because it seems the height is calculated without the wrapping.

The problem is that the <description id="mailSendFormatExplanation"/> element starts empty and gets filled with text once the dialog is loaded.
So, extra text appears once the dialog has already set its height based on the static content present on creation.

I don't understand why the observer doesn't always trigger at the right time, after all the content has been loaded.
Is there anything else dynamically changing in this dialog that we're missing?

Attachment #9095374 - Flags: review?(richard.marti)

Maybe Magnus has some insight on this.

Flags: needinfo?(mkmelin+mozilla)
Comment on attachment 9095374 [details] [diff] [review]
1583415-askSendFormat.patch

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

Probably doesn't trigger the observer because you're not changing an attribute. Changing a property wouldn't trigger it (unless, it's set up so that the property also sets the attribute, which I assume doesn't happen here).
But, I don't think is the right thing to do anyway.

FWIW, I think this is nothing new. I've seen it many years ago.

Flags: needinfo?(mkmelin+mozilla)

Thanks Magnus for the feedback, and I think Richard you're right.
Let's do a mix up between having a defined height and the sizeToContent() method with a small delay.

In this way, if the height value is bigger than the content height, the method doesn't do anything.
We have the resize method as a fail-safe for those edge cases where the font size or translated strings are longer than what we're seeing. Even if that means having a little jump in the UI, it's better than not showing the buttons.

Attachment #9095374 - Attachment is obsolete: true
Attachment #9099730 - Flags: review?(richard.marti)
Comment on attachment 9099730 [details] [diff] [review]
1583415-askSendFormat.patch

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

Looks good. r+ with considering the delay.

::: mailnews/compose/content/askSendFormat.js
@@ +54,5 @@
>    }
> +
> +  setTimeout(() => {
> +    window.sizeToContent();
> +  }, 80);

80 ms seems a bit long. Is this needed or would a shorter delay work too?
Attachment #9099730 - Flags: review?(richard.marti) → review+

On Linux, a delay of 25ms was what needed to always have a consistent, but on macos, I noticed a random delay between 50 and 75ms.
I agree that 80ms is quite long, but I think it's a safe number to cover any possible delay due to slow a machine or other factors.

Do you think this is ready for a checkin?

Flags: needinfo?(richard.marti)

Yes.

Flags: needinfo?(richard.marti)
Keywords: checkin-needed

Pushed by richard.marti@gmail.com:
https://hg.mozilla.org/comm-central/rev/6370a0f66ec1
Autoresize askSendFormat dialog to not let the buttons go out of view. r=Paenglab

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 71.0

It occurred to me: Can the symptom/cause of this be the same for the problem I see in
https://bugzilla.mozilla.org/show_bug.cgi?id=625678
Missing input search fields from the search window

I still see this from time to time. But my TB is pre-71.0. I hope I will get the fixed version soon...

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: