Comment 22 for bug 239360

Revision history for this message
In , Kai Engert (kaie) wrote :

This is my answer to Nelson's concerns from comment 21 and my other review comments to this patch.

(a)
"The information provided by this site hasn't been verified by a recognized authority, and may be misleading. You should only add this exception if you have already confirmed the information yourself."

Nelson is unhappy with the second sentence. After hearing his concerns I tend to agree and propose to simply remove the second sentence altogether.

The statement is like
  "There is a problem. But if think it's not a problem then go ahead."

The second sentence is trying to give the user an advice what to do. But giving the right advice is difficult. At least, it requires a lot more words. But the more words we use in the dialog, the less likely is it that users will read it (my opinion).

So my proposal is to:
- state the problem
- be short and concise and scary
- have the user look elsewhere for more details and assistance (see b)

In other words, I propose to remove all 3 sentences from the patch that start with "You should only".

(b)
Nelson proposes to give more information in order to assist the user to make the correct decision. I agree. But I recommend to keep the dialog wording concise and provide the assistance in a separate window. What's the usual way to give more assistance? I propose to add a help button to the dialog, which pops up a separate window.

Nelson, would you be interested to write text for such a help page?

After thinking more about the above and looking at the current screenshots, I played around with the code to come up with ideas... Johnathan, please feel free to tell me this is ugly, but maybe the following is helpful.

(c)
The "Add Exception" label in the dialog sounds innocent. What about "Disable Security"?

(d)
I really like the phrase "Most people would not do this ... (and other's) ... would not ask you to do this".

I wonder if this phrase should get the focus. Right now it's being displayed at the top of the window, in plain text, next to another bold paragraph, which makes it loose attention.

Should we print "You are about to..." as plain text, show "most people would not..." as bold, and move the latter close to the "disable security" button?

(e)
The "Certificate Location" label.
As we don't have a separate certificate download location, but rather a site we connect to, I wonder if we can improve this wording. But maybe that's only because I know how it technically works :-)

We want the user to enter the location of the site he wants to connect to. So, I think "Location" is good, because that's what being used in Firefox' File menu, too.
Could we use "Site" or "Web Site" as a label for the group box?

I hate to suggest "Web Site", because this will be used for mail as well. But as of today Certificate Manager has a "Web Sites" tab, even though it applies to any SSL site.
So, until we fix that generally, I propose let's be consistent and re-use "Web Site" in this dialog.

(I was tempted to say "Secure Site", but after all, that site might not be secure, so let's not add confusion.)

(f)
The current statement "You are about to..." talks about "this site". When you open a dialog and have not yet opened any details, could this be confusing? We are not in the context of a web site, instead, we came from cert manager, so maybe we should use a slightly different wording like this: "You are about to override Firefox and force it to treat a site as trusted even though it may not be."

Finally here are comments related to the code and dialog behavior.

(g)
The Port number.
I propose we open the dialog with the port number field being "empty".
I played around with the dialog and found the 443 to be confusing.
One of our goals for this dialog is: Allow the user to paste a full URL into the Location box. An URL can contain a port number as in https://www.somewhere.org:1234/. It feels inconsistent if we show 443 after the user pasted such an URL.

My proposal is:
- have the port field empty by default
- should the user enter a hostname, but no port number, we default to 443 (without displaying that)
- if the user enters/pastes a full URL, we use the port number from that URL (or default to 443)
- if the user explicitly enters a port number, we use it (and override whatever got entered into the Location field)
- the above also works if the user enters host:port into the Location field

I experimented and think I have a code change that makes it work like this.

(h)
I get JavaScript strict warnings when I run the code, I think we need declarations at the top of the exceptionDialog.js file to make them go away:

var gDialog;
var gBundleBrand;
var gPKIBundle;
var gSSLStatus;
var gChecking;
var gBroken;

All variables should get declared before being used first.

(i)
I think the dialog should be reset to the initial state (no cert, action button disabled) as soon as the user makes changes to the Location or Port fields.