490106 - Tooltips for autoconfig server security quality don't wrap

Status: RESOLVED FIXED

(Thunderbird :: Account Manager, defect)

Description

[Phil Ringnalda (:philor)]

From bug 422814 comment 135:

[[[
Not sure what we need to do to make the tooltips with
insecureCleartext.description or insecureSelfsigned.description wrap, but
they're currently a single very long line (and while someone's in the
neighborhood, MDC and I don't think "bottom" is a valid value for the align
attribute).
]]]

STR for the long version:

1. File - New - Mail Account (Quick Setup)
2. Name = goats, email = [e-mail], password = goats (I really hope
the account doesn't exist)
3. Next, then if we don't discover 465/SSL for the SMTP server, force us to
4. Hover the yellow dot by the SMTP server, try to read a single long line that's actually cut off in less than 5 seconds

Comment 1

[David Ascher (:davida)]

* i gave it a whack, but can't figure out how to get the tooltip to shrink to fit, even if i can get the tooltip text to max-width.  

* I'm not sure the tooltip is enough of a UI to display that long paragraph.  Either the paragraph should be shorter, or we need a non-ephemeral text display.

Comment 2

[Mark Banner (:standard8)]

Targeting to b4 due to possible string impact.

Comment 3

[Blake Winton (:bwinton) (:☕️)]

Attachment: The tooltip, wrapped.

I tried reading the tooltip, and despite my being a fairly speedy reader, I still couldn't get to the 5th line before it went away.

Comment 4

[Blake Winton (:bwinton) (:☕️)]

Attachment: The patch to make it wrap those tooltips.

Here's a patch so you can test out the wrapping tooltips yourselves.  I think we'll probably need to make further changes, but making the text wrap seems like a good idea anyways.

Comment 5

[David Ascher (:davida)]

I wonder if using a notification bar that would then expand to a larger UI upon click would provide a better way of explaining what's going on.

Comment 6

[Phil Ringnalda (:philor)]

Since it's UI that mirrors Fx's Larry, who is an xul:panel, I'd think that would be a good way to go. That, and making the poor sucker who got saddled with the strings bug chop that essay down to size :)

Comment 7

[Blake Winton (:bwinton) (:☕️)]

Okay, I'm going to try for the Larry-esque xul:panel when you click the icon, and we'll see how that goes.

Comment 8

[Blake Winton (:bwinton) (:☕️)]

Attachment: Add a smaller tooltip and a popup with the longer description.

So, this isn't anything like the final version of this patch, but I wanted to know what you thought of this stab at a new UI.

Thanks,
Blake.

Comment 9

[Bryan Clark (DevTools PM) [:clarkbw]]

I'm having a hard time getting this to work on Linux.  I'll try a windows build to check it out there.

Comment 10

[Bryan Clark (DevTools PM) [:clarkbw]]

Ok, this is working as expected on Windows.  Adding wayne in case he knows of a platform bug related to tooltips / popups on Linux.

Comment 11

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 382580 [details] [diff] [review]
Add a smaller tooltip and a popup with the longer description.

I like the smaller + longer description on click.  I think we need at least a one liner identifier phrase for the smaller tooltips.

Tooltip:
.---------------------------.
| 00000  *Insecure server*  |
| LARRY                     |
| 00000                     |
'---------------------------'

Popup
.-----------------------------------------.
| 00000  *Insecure server*                |
| LARRY                                   |
| 00000  Slightly longer description text |
|        [...]                            |
|        <a>more information</a>          |
'-----------------------------------------'

putting a minus for now but I like the direction this is going and the wrapping is working pretty well.

Comment 12

[Wayne Mery (:wsmwk)]

(In reply to comment #10)
> Ok, this is working as expected on Windows.  Adding wayne in case he knows of a
> platform bug related to tooltips / popups on Linux.

a lot of these bugs are stale, so hard to say:
bug 228673
https://bugzilla.mozilla.org/buglist.cgi?query_format=advanced&short_desc_type=allwordssubstr&short_desc=wrap&product=Core&product=Firefox&product=MailNews+Core&product=Thunderbird&product=Toolkit&component=XUL&component=XUL+Widgets&resolution=---&bug_severity=major&bug_severity=normal&bug_severity=minor&chfieldto=Now

Comment 13

[Blake Winton (:bwinton) (:☕️)]

Attachment: Sample with single line.

Here you go, the pictures are, in order:
Secure tooltip
Secure popup
Insecure tooltip
Insecure popup

Let me know if you'ld like any changes to the wording/placement/colours/etc…

Thanks,
Blake.

Comment 14

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 385839 [details]
Sample with single line.

Is it possible to get bold styling and line breaks?

Such that the initial popup has spacing and bold + italic font like this:

*Warning!  This is an insecure server*

/Click circle for more details/

---

The click for more details makes sense to me.  Although I don't really like referring to the "circle" and without telling people what to click on I'm afraid they'd try to catch the tooltip with their mouse.

Comment 15

[David :Bienvenu]

We shouldn't be saying Shredder, but perhaps that's fixed in the string fix bug?

Comment 16

[Phil Ringnalda (:philor)]

Why shouldn't we? Other than "someone should tell bwinton how to build with official branding so he doesn't have to see the awful Shredder graphics," that's &brandShortName; and just what we should be using.

Comment 17

[David :Bienvenu]

oh, sorry, as long as it's using brandShortName, nm.

Comment 18

[Blake Winton (:bwinton) (:☕️)]

Attachment: Prettier tooltip.

(In reply to comment #14)
> Is it possible to get bold styling and line breaks?
> 
> *Warning!  This is an insecure server*
> /Click circle for more details/

How's this?

(In reply to comment #17)
> as long as it's using brandShortName, nm.

It is.  :)

Later,
Blake.

Comment 19

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 385896 [details]
Prettier tooltip.

That looks good!

I only have one more request now :)

Match the outer padding that the tooltip has in the popup such that the image and text are all equally padded.  Also some additional start/end padding on the text so it doesn't sit right against the image and edge.  Double whatever the padding the tooltip is using, 3px? for the text.

Comment 20

[Blake Winton (:bwinton) (:☕️)]

Attachment: Prettier tooltips that line up nicely, too.

(In reply to comment #19)
> I only have one more request now :)
> Match the outer padding that the tooltip has in the popup
> such that the image and text are all equally padded.

Okay, this looks a lot more spaced out to me.  Let's go for a code review.

Thanks,
Blake.

Comment 21

[Phil Ringnalda (:philor)]

Comment on attachment 386072 [details] [diff] [review]
Prettier tooltips that line up nicely, too.

>+        icon.setAttribute('popup', 'insecureserver-'+details+"-panel");

The single quotes are my fault, I should have dug in my heels about them, but that mixture in a single line is pure evil: how about if we start to make our stand against them by switching that whole switch, and at the same time adding the spaces around the +es that I also should have insisted on.

Comment 22

[Blake Winton (:bwinton) (:☕️)]

Attachment: The previous patch, with philor's suggestions.

(In reply to comment #21)
> (From update of attachment 386072 [details] [diff] [review])
> >+        icon.setAttribute('popup', 'insecureserver-'+details+"-panel");
> The single quotes are my fault, I should have dug in my heels about them, but
> that mixture in a single line is pure evil: how about if we start to make our
> stand against them by switching that whole switch, and at the same time adding
> the spaces around the +es that I also should have insisted on.

I've converted the whole function, cause I was there, and added the spaces around the +s, too.

Thanks,
Blake.

Comment 23

[Phil Ringnalda (:philor)]

Comment on attachment 386603 [details] [diff] [review]
The previous patch, with philor's suggestions.

Looks good, except for the align="bottom", which still isn't defined but is now even less something we'd want to happen, so I removed them.

Comment 24

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/comm-central/rev/f53696dd29ae