1744319 - Improve bad certificate and insecure socket warnings in the Account Setup

Status: NEW

(Thunderbird :: Account Manager, enhancement)

Description

[Alessandro Castellani [:aleca]]

There seem to be some areas in the Account Setup protocol selection where we don't properly handle warnings and visual feedback for when we detect insecure sockets or bad certificates.

Bad Certificate
If we detect a bad certificate from the server, we add an .insecure class which is supposed to highlight in red some element.
https://searchfox.org/comm-central/rev/e441dd8b6f04570c5b9529ed5a6cce43efe8770e/mail/components/accountcreation/content/accountSetup.js#1305-1309

It seems that the .cert-status element doesn't really exists and it's never generated.

This seems to be a regression introduced here: https://hg.mozilla.org/comm-central/rev/76d9e4c16c04c7c4996d88304f715eafba4d3ec7#l1.1412
Before the regression, we used to print this in red: "Warning! Could not verify server"

Is this still necessary?
is this string accurate?
Is this warning related to the whole server or only to a specific protocol?
If this is necessary and we need to reintroduce it, we should improve the string and add a better explanation.

Insecure Socket Type
In case the server doesn't offer SSL or STARTTLS socket, we highlight the available option in red, to represent the fact that it's insecure: https://searchfox.org/comm-central/rev/e441dd8b6f04570c5b9529ed5a6cce43efe8770e/mail/components/accountcreation/content/accountSetup.js#1301-1304

We should improve this aspect since having a red pill doesn't communicate much.
We should add a title tooltip with a string explaining the situation.

What would be an accurate text feedback for the user?

Comment 1

[Magnus Melin [:mkmelin]]

I guess it's all fallout essentially from bug 1547096.

There's just too much junk in GuessConfig.jsm, so really hard to reason with it. Anyway, seems we don't get an error status for bad cert at https://searchfox.org/comm-central/rev/fbd21ecf0fd7cc799d1021dc1bb238edb380bcbe/mail/components/accountcreation/GuessConfig.jsm#1221 so a lot of this code is very likely dead. We really should rewrite it to using TCPSocket instead.

Is this still necessary?
Probably not, during verifyConfig you do get notified anyway and will have to accept the cert override to continue.

is this string accurate?

Sure. Maybe "Warning! Bad certificate. Could not verify server" would be better...

Is this warning related to the whole server or only to a specific protocol?

To the specific configuration (hostname+port) - whatever we got back from that. (In practice, cert will cover the whole hostname at least)

If this is necessary and we need to reintroduce it, we should improve the string and add a better explanation.

The popup one does get during verifyConfig has the more detailed explanation.

Comment 2

[Alessandro Castellani [:aleca]]

Thanks for looking into this.
I guess this could be postponed for a low priority clean up at the beginning of next year.