Closed Bug 554404 Opened 12 years ago Closed 12 years ago

SMTP editor dialog misaligned after Bug 525238 landed

Categories

(Thunderbird :: Account Manager, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Thunderbird_Mail_DE, Unassigned)

References

Details

Attachments

(4 files, 1 obsolete file)

Attached patch Patch to remove hbox element (obsolete) — Splinter Review
In SMTP editor dialog the last line (Username: _______________) is misaligned because of an <hbox class="indent"> element.

IMHO the <hbox> element could be removed. I'll attach a patch.
Attachment #434290 - Attachment is patch: true
Attachment #434290 - Attachment mime type: application/octet-stream → text/plain
Attachment #434290 - Flags: ui-review?(clarkbw)
Depends on: 525238
Attachment #434290 - Attachment is obsolete: true
Attachment #434290 - Flags: ui-review?(clarkbw)
Attachment #434291 - Flags: ui-review?(clarkbw)
Please show a screenshot documenting what you mean with "misaligned", and how your patch changes it.

IIRC, I intentionally intended the username, because it is dependent to the authentication, and it also was intended in the old dialog for the same reason.
Attached image Before auth patch
This is how it looked before my change
Attachment #434294 - Attachment description: Before → Before auth patch
This is how it looks after bug 525238 and without your patch.

Granted, it's not pretty, but it's intentional, not a bug, and you can see it preserves the idea of the old dialog.

I don't have an opinion of whether it's better with or without the indention. I agree it would look better without.
(In reply to comment #2)
> IIRC, I intentionally intended the username, because it is dependent to the
> authentication, and it also was intended in the old dialog for the same reason.

Okay, you're right.

(In reply to comment #4)
> I don't have an opinion of whether it's better with or without the indention.
> I agree it would look better without.

Maybe only the "centered" looking string "User Name:" confused me.

From my point of view we can close this bug.
I'm undecided. I leave it to Bryan.
Can you add message submission port number in new panel upon panel design/layout change?
> Port: [ 25 ] Default: 25
>              Message Submission Port: 587

Question about "StartTLS, if avail" in sample screen shot.
Will "StartTLS, if avail" be enabled again to resolve StartTLS variant of bug 534158?
  No STLS to EHLO, if connection from internal network.
  STLS to EHLO and SMTP forces STLS, if connection from external network.
(bug 534158)
  No AUTH to EHLO, if connection from internal network.
  AUTH to EHLO and SMTP forces SMTP-AUTH, if connection from external network.

Question relates to solution of bug 534158.
Is there any room for option like "Use name and password, if requested" in addition to current "Use name and password"(always try SMTP-AUTH))?
WADA, all of this is offtopic here. Please keep one bug to one issue. And questions should go per mail, not bugzilla. I reply per email.
I've today encountered the same thing and I agree with Alex, its really misaligned.
Here is a screenshot from a build without and with the patch from #1, so you can see the differences.
Comment on attachment 434291 [details] [diff] [review]
Patch to remove hbox element (encoding fix)

didn't try this out but it looks good from the screen shot and the code change makes sense.
Attachment #434291 - Flags: ui-review?(clarkbw) → ui-review+
Comment on attachment 434291 [details] [diff] [review]
Patch to remove hbox element (encoding fix)

ok, then r=BenB
Attachment #434291 - Flags: review+
Attachment #434291 - Flags: superreview?(neil)
Attachment #434291 - Flags: review?(bwinton)
Comment on attachment 434291 [details] [diff] [review]
Patch to remove hbox element (encoding fix)

It looks fine to me, but I'm not actually qualified to review this bit of the code.  ;)

Perhaps bienvenu would give it a look?
Attachment #434291 - Flags: review?(bwinton) → review?(bienvenu)
Comment on attachment 434291 [details] [diff] [review]
Patch to remove hbox element (encoding fix)

[I didn't notice on the original bug, but you probably didn't need a separate hbox just for the indent; adding class="indent" to the label should have worked.]
Attachment #434291 - Flags: superreview?(neil) → superreview+
Attachment #434291 - Flags: review?(bienvenu) → review+
Commited as <http://hg.mozilla.org/comm-central/rev/8d802c091f7c>

FIXED
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Keywords: checkin-needed
Blocks: 525238
No longer depends on: 525238
You need to log in before you can comment on or make changes to this bug.