515599 - Improve wording of insecureCleartext.description

Status: RESOLVED FIXED

(Thunderbird :: Account Manager, defect)

Description

[Peter Weilbacher]

From bug 506290 comment 30 and bug 506290 comment 31

>> The latest one leads me to question the point of
>> insecureCleartext.description. It implies that if the configuration
>> contains a SSL or STARTTLS setting (port?), nobody could read it. But AFAIU
>> this really only secures the transit between the sender and that one SMTP
>> server. All subsequent SMTP transactions are still likely to be insecure
>> and there is no way for the user (or TB) to check that. So overall I think
>> this warning will be confusing.
>
>That's true, but I think that the first SMTP transaction is probably the most
>important one to secure, since that'll be the one that requires
>authentication.
>
>But the wording doesn't really convey that, so if you had a suggestion for a
>better way to say that, I'ld love to change it.

Instead of
   "Email is sent in clear-text, so your email could be read by other people
   on the internet. &brandShortName; will let you get to your mail, but you
   should really get your email provider to configure the server with a secure
   connection."
perhaps it should focus more on the password which is really the problematic part that is transmitted in clear (even if it's mangled using MD5 stuff or the like):
   "Your email and authentication are sent unencrypted, so your password
   (and your message) could easily be read by other people. &brandShortName;
   will let you get to your mail, but you should contact your email provider
   about configuring the server with a secure connection."

I'm not a native speaker and really not very good at polishing wording. This is just a first try.

Comment 1

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

Bryan, what do you think of that text?

Comment 2

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

Bryan, what do you think of that text?

Comment 3

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

That sounds much better, thanks Peter!  Blake, if you want to swap this in assume a ui-r+ on that text.

Comment 4

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

Attachment: A patch to swap the text, and the entity name.

Marking ui-r+ as per Bryan's comment above.

Does this even need an r, being essentially a string change?

Thanks,
Blake.

Comment 5

[Phil Ringnalda (:philor)]

Comment on attachment 401136 [details] [diff] [review]
A patch to swap the text, and the entity name.

You did realize that would make me find a reason to r-, no matter how hard I had to look, right?

Quite often, https://developer.mozilla.org/en/Writing_localizable_code is full of crap when it claims that when you change a key, you have a wonderful opportunity to change to a better name, but in this case, if "clear-text" wasn't a good choice for the text, then Cleartext isn't a good choice for the key, and in any case changing the case of a single letter for a key change is just asking for trouble and confusion.

"insecureUnencrypted.description", por favor.

Comment 6

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

(In reply to comment #5)
> (From update of attachment 401136 [details] [diff] [review])
> You did realize that would make me find a reason to r-, no matter how hard I
> had to look, right?

Of course, but I was hoping that it would take you longer than 8 minutes.  :)

> Quite often, https://developer.mozilla.org/en/Writing_localizable_code is full
> of crap when it claims that when you change a key, you have a wonderful
> opportunity to change to a better name, but in this case, if "clear-text"
> wasn't a good choice for the text, then Cleartext isn't a good choice for the
> key, and in any case changing the case of a single letter for a key change is
> just asking for trouble and confusion.

I'm sure this will damn me even further, but the localizers didn't seem to mind the change from ClearText to Cleartext back in http://hg.mozilla.org/comm-central/rev/977abd1818d2  Heck, you even gave it the r+.  ;)

> "insecureUnencrypted.description", por favor.

I'm happy to change it, but while I'm there would you also like me to change:
mail/locales/en-US/chrome/messenger/accountCreation.properties
3:# LOCALIZATION NOTE(cleartext_warning): %1$S will be the hostname of the server the user was trying to connect to.
4:cleartext_warning=%1$S does not use encryption.
8:cleartext_details=Insecure mail servers do not use encrypted connections to protect your passwords and private information. By connecting to this server you could expose your password and private information.

and:
mailnews/base/prefs/content/accountcreation/emailWizard.xul
90:  <panel id="insecureserver-cleartext-panel" class="popup-panel">
95:        <description class="details">&insecureClearText.description;</description>
117:  <tooltip id="insecureserver-cleartext">

And:
mailnews/base/prefs/content/accountcreation/emailWizard.js
113:    this._incomingWarning = 'cleartext';
114:    this._outgoingWarning = 'cleartext';
670:          case 'cleartext':
672:              "cleartext_warning", [incoming.hostname]);
673:            incoming_details = gStringsBundle.getString("cleartext_details");
694:          case 'cleartext':
696:              "cleartext_warning", [outgoing.hostname]);
697:            outgoing_details = gStringsBundle.getString("cleartext_details");
721:        // no certificate or cleartext issues
1133:            this._setIncomingStatus('weak', 'cleartext');
1172:              this._setOutgoingStatus('weak', 'cleartext');

?  (i.e. does it make sense to expand the scope of this a little?)

Thanks,
Blake.

Comment 7

[Phil Ringnalda (:philor)]

Nope, changing keys without changing values is a sure path to confusion. And unfortunately, much as I wish it wasn't so, "no localizers complained" is miles away from "no localizers were confused" - nearly, but not quite as far away as "philor reviewed it" is from "it was the right thing to do."

Comment 8

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

Attachment: The previous patch, with philor's nit fixed.

Ta-da!

Question #2, does it need an sr, since it changes mailnews "code"?

Thanks,
Blake.

Comment 9

[Phil Ringnalda (:philor)]

Comment on attachment 401148 [details] [diff] [review]
The previous patch, with philor's nit fixed.

It doesn't need sr as far as I'm concerned: other than the backend bits that allowed for probing and trial logins, I had to review the whole thing without benefit of sr backup, and I've reviewed a dozen or so patches since without backup, and the only reason it's in mailnews/ at all is because we expect at some point SM will want to use it too, and it'll be less trouble than moving it then. Until someone complains, I say we just keep treating it like the Thunderbird UI that it is, and I'll keep making bienvenu sr when I get nervous that I'm not actually following what you're doing. And since anyone who is in a position to successfully complain will also be in a position for me to shunt every single review of it to them, I think we're pretty safe ;)

Comment 10

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/df2979552db3