missing padding in SMTP "Server Hostname" input field on Mail Account Setup

RESOLVED FIXED in Thunderbird 54.0

Status

defect
--
trivial
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: bugzilla, Assigned: Paenglab)

Tracking

Trunk
Thunderbird 54.0

Thunderbird Tracking Flags

(thunderbird52 fixed, thunderbird53 fixed, thunderbird54 fixed)

Details

Attachments

(6 attachments, 4 obsolete attachments)

Reporter

Description

3 years ago
Posted image Screenshot of bug (obsolete) —
there is missing some padding in the smtp server hostname field of the Mail Account Setup dialog
The same seems to be the case for the port
Reporter

Updated

3 years ago
Attachment #8820228 - Attachment is obsolete: true
Reporter

Comment 1

3 years ago
Posted image Screenshot of bug
Yeah.  

But port looks OK to me
Severity: normal → trivial
Summary: missing padding in SMTP input field on Mail Account Setup → missing padding in SMTP "Server Hostname" input field on Mail Account Setup

Comment 3

3 years ago
It's because incoming hostname is a textbox but the smtp hostname is an editable menulist element. And they happen to have different internal padding. It's not sure that is even a problem.
But the effect is even much worse on Linux.

I'd think this needs to be fixed in toolkit/XUL css, not Thunderbird's. Paenglab, what do you think?
Flags: needinfo?(richard.marti)
OS: Unspecified → All
Hardware: Unspecified → All
Version: unspecified → Trunk
Assignee

Comment 4

3 years ago
It's hard to get toolkit reviews for code they don't use.

For Windows I have already a c-c patch with two small rules. Next is Linux and then I check macOS.
Flags: needinfo?(richard.marti)
Assignee

Comment 5

3 years ago
Posted patch emailWizard.patch (obsolete) — Splinter Review
This patch fixes the padding issue.

On Linux and Windows I also made the menulists the same height as the textboxes. Please check this is also the case with your Linux theme.

On macOS I made the menulist margins the same as the textbox to let them have the same width.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8821605 - Flags: review?(acelists)

Comment 6

2 years ago
Thanks, looks much better now. I think the server hostname and port boxes are still 1px higher than the other surrounding menulists.

Also, if you select the SMTP hostname from the list of already existing ones, the menulist changes to non-editable and the other fields in the row are hidden, causing the SMTP row to take less vertical space. This causes the other rows to move around. Can you add some fixed passing e.g. to the "Outgoing" label so that this does not happen?
Assignee

Comment 7

2 years ago
Didn't knew that this menulist changes. Works now on all platforms, macOS needed no change.
Attachment #8821605 - Attachment is obsolete: true
Attachment #8821605 - Flags: review?(acelists)
Attachment #8835067 - Flags: review?(acelists)

Comment 8

2 years ago
Thanks, now the rows do not jump on change of the SMTP menulist. But now the hostname seems to be 1px higher (on the top) than other widgets.
Assignee

Comment 9

2 years ago
Posted patch emailWizard.patch (obsolete) — Splinter Review
Hmm, again a issue with different Linux themes. For me the elements have the same height.

I tried something and hope this helps. Here it still works. If not, I need to change the XUL file which could affect SM.
Attachment #8835067 - Attachment is obsolete: true
Attachment #8835067 - Flags: review?(acelists)
Attachment #8835700 - Flags: review?(acelists)

Comment 10

2 years ago
Comment on attachment 8835700 [details] [diff] [review]
emailWizard.patch

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

With this version, hostname AND port is 1xp higher and the SMTP picker is jumping a bit again. So the previous version was better.
Attachment #8835700 - Flags: review?(acelists) → review-

Comment 11

2 years ago
Comment on attachment 8835067 [details] [diff] [review]
emailWizard.patch v2

This is currently the preferred candidate.
Attachment #8835067 - Attachment filename: emailWizard.patch → emailWizard.patch v2
Attachment #8835067 - Attachment is obsolete: false
Attachment #8835067 - Flags: feedback+

Updated

2 years ago
Attachment #8835067 - Attachment description: emailWizard.patch → emailWizard.patch v2
Attachment #8835067 - Attachment filename: emailWizard.patch v2 → emailWizard.patch
Assignee

Updated

2 years ago
Attachment #8835700 - Attachment is obsolete: true
Assignee

Comment 12

2 years ago
Posted patch emailWizard.patch v4 (obsolete) — Splinter Review
This patch should stretch all items of the row to the same height. I also made the margin-top/bottom the same of all elements the same. When there are still differences it must be a Linux theme issue.
Attachment #8836104 - Flags: review?(acelists)

Comment 13

2 years ago
Comment on attachment 8836104 [details] [diff] [review]
emailWizard.patch v4

Thanks, this is almost perfect. I just see there is more padding between the row with Username vs Outgoing and Outgoing vs Incoming . I'll attach a new picture. It would be great if you could make the paddings the same, preferably as the larger one (between Username and Outgoing). If it can't be done, we take this patch.
Attachment #8836104 - Attachment description: emailWizard.patch → emailWizard.patch v4
Attachment #8836104 - Flags: review?(acelists) → review+

Comment 14

2 years ago
The heights of the widgets are all the same now, I'm happy ;)
Just see the padding between the rows in this screenshot.
Assignee

Comment 15

2 years ago
You're right. Fixed.
Attachment #8836104 - Attachment is obsolete: true
Attachment #8836225 - Flags: review?(acelists)

Comment 16

2 years ago
Comment on attachment 8836225 [details] [diff] [review]
emailWizard.patch v5

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

Perfect now, thanks.
Attachment #8836225 - Flags: review?(acelists) → review+
Assignee

Comment 17

2 years ago
Thank you.

https://hg.mozilla.org/comm-central/rev/3a925dc53d92ab7c4fbf04c19cc9c7d918adaa72
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 54.0
Assignee

Comment 18

2 years ago
Comment on attachment 8836225 [details] [diff] [review]
emailWizard.patch v5

[Approval Request Comment]
User impact if declined: Not well aligned "Manual config" page
Testing completed (on c-c, etc.): on c-c
Risk to taking this patch (and alternatives if risky): low
Attachment #8836225 - Flags: approval-comm-beta?
Attachment #8836225 - Flags: approval-comm-aurora?

Updated

2 years ago
Attachment #8836225 - Flags: approval-comm-beta?
Attachment #8836225 - Flags: approval-comm-beta+
Attachment #8836225 - Flags: approval-comm-aurora?
Attachment #8836225 - Flags: approval-comm-aurora+
You need to log in before you can comment on or make changes to this bug.