408869 - Make the extension installation dialog easier to understand

Status: RESOLVED FIXED

(Core Graveyard :: Installer: XPInstall Engine, enhancement)

Description

[Madhava Enros [:madhava]]

Mockup and notes:

http://wiki.mozilla.org/Firefox:Add-ons_Manager_UI#Scary_Confirmation

The redesign is an attempt to do the following:

- remove redundant and confusing messages from the dialog
- elevate the warning, rather than the list of add-ons, to primacy in the dialog.  The point of the dialog, after all, is the warning -- the list of add-ons is just to give some context for the warning
- make the "Signed" or "Unsigned" status, currently expressed in those terms and red lettering, something that is more user-understandable.

Comment 1

[Mike Connor [:mconnor]]

Attachment: wip

Comment 2

[Jesse Ruderman]

In the mockup, "Install add-ons only from sources you trust" looks like a title, not a warning.  In the current dialog, it's just bold (not huge) and near the bottom, where I think it's more likely to catch users' attention.

Any particular reason you removed "Malicious software can damage your computer or violate your privacy"?

Comment 3

[Daniel Veditz [:dveditz]]

In the current dialog "Unsigned" is in red to scare people. you have absolutely no clue who wrote this thing, and neither does anyone else.

Didn't work, practically no one bothers to sign theirs anyway. At least we managed to substitute hashes to regain some of the "tamper-evident wrapper" aspects of signing, but that only works on AMO when the files are served by the AMO server. Signing is a property of the package itself regardless of where it's copied to. Then again, having something signed by some random person you've never heard of isn't worth a whole lot, but at least you know the package is as they intended it to be.

The signer's name alone really isn't that great, the ultimate goal was to provide a way to look at the certificate itself (by making the name a link?) so you can see who they are and who's vouching for it.

Comment 4

[Daniel Veditz [:dveditz]]

For SSL Larry uses the terms "Location Verified" and "Identity Unknown" (I think I would have preferred 'unverified'). Would it be better to use "unverified" to echo the identity messaging?

Can we say "Author" rather than "Source"? people are going to think of the website where they got it as the "source" and that's not what the signature on an install package asserts.

So "(Author Unknown)" or "(Author unverified)" would get my vote. In this context I prefer unknown, I'd only suggest 'unverified' to match Larry if we changed the Larry wording.

Comment 5

[Dave Townsend [:mossop]]

I agree that we need to keep the warning about malicious software, I've seen at least a few people shocked when I suggested that extensions could be harmful if they wished and they asked why we didn't warn them about it. That line is what I usually have to point them to.

Maybe we change the title to something more title-like then have the real warning in bold underneath that?

Also tend to agree about using the same terms as Larry for consistency, and I don't see why on a known source the text couldn't be a link or something that pops up those more details.

I wonder if we also want to be highlighting the fact that the xpi is going to be checked against a hash in the unsigned case. It is some added assurance that the xpi is the one you chose from the website.

Comment 6

[Madhava Enros [:madhava]]

To respond to a couple of these comments...

Comment #2:
If they read anything on the dialog, users are going to tend to be drawn to the boldest (whether by size, contrast, or weight) text that's near the colourful graphic.  In the case of the current dialog, that's the list of add-ons -- we want it, instead, to be a short statement of the decision they have to make: "Do you trust this?"  Making the important text less central (lower down the dialog) and smaller means that fewer people will pay attention.

As for removing "Malicious software can damage your computer or violate your privacy" -- I've got nothing against it, really.  The reason to remove it was just that, of everything on the dialog, it seems like the least actionable piece of information, and, in a dialog where everything is competing for the user's attention, everything inessential that we remove is a benefit.

That said, I'm certainly willing to believe that that statement may be more helpful in explaining to the user why they must make this decision than the "Add-ons have the same access..." Do you think that's the case?

Comment #3:
Agreed -- that's exactly why it's not red anymore.  Given that so few add-ons are signed, the message is always red, and users habituate to it.  On top of that, when it's always red, it doesn't really help anybody make a decision while still making them vaguely anxious.

Comment #4:
I agree with using "Author" rather than "Source" for the reason given.  A lot of addons will be coming from AMO, which will make saying "Source Unknown" kind of odd.  Oh, and "unknown" seems fine too - it's simpler than "unconfirmed," certainly.

Comment 7

[Jesse Ruderman]

Yes, I think "Malicious software can damage your computer or violate your privacy" is more helpful than "Add-ons have the same access to your computer as Firefox itself".

Comment 8

[Madhava Enros [:madhava]]

I'd be happy to have the subtitle be the "Malicious software can damage your computer or violate your privacy."

The main message should be "Only install add-ons from authors whom you trust." to stress author rather than source.

Comment 9

[Dave Townsend [:mossop]]

Attachment: current wip

This is the current work in progress with the updated text suggested by Madhava. Unless there is any further discussion needed I can get this ready for review.

Just need to add the icon overlays on the puzzle icon, I presume it is warning symbol for unsigned and nothing for normal?

Comment 10

[Madhava Enros [:madhava]]

(In reply to comment #9)

Looking at this again, the little warning overlays are actually kind of redundant, given the large ones again.  Even when the author is known, it doesn't mean we're recommending that that trust them, so having the overlays or not, on that basis, doesn't make much sense.  I think we can dispense with them.

Generally, I think it's ready to go (assuming that the text over the list of add-ons pluralizes properly when appropriate).

I do have some minor spacing adjustments to recommened -- I'll attach a diagram with explanation.

> Created an attachment (id=296324) [details]
> current wip
> 
> This is the current work in progress with the updated text suggested by
> Madhava. Unless there is any further discussion needed I can get this ready for
> review.
> 
> Just need to add the icon overlays on the puzzle icon, I presume it is warning
> symbol for unsigned and nothing for normal?
> 

Comment 11

[Madhava Enros [:madhava]]

Attachment: minor layout adjustments

The attachment outlines some layout/spacing adjustments.  The letters in the graphic correspond to the following notes.  

a - more space here
b - less space here
c - indent more
d - less space
e - less height

Comment 12

[Dave Townsend [:mossop]]

Attachment: patch rev 1

This is the final patch, checked on all 3 platforms and Madhava has confirmed the look.

Comment 13

[Asaf Romano (gone)]

Comment on attachment 297181 [details] [diff] [review]
patch rev 1


>@@ -77,7 +77,7 @@ XPInstallConfirm.init = function ()
>     if (icon != "")
>       installItem.icon = icon;
>     var cert = this._param.GetString(++i);
>-    installItem.cert = cert || bundle.getString("Unsigned");
>+    installItem.cert = "(" + (cert || bundle.getString("unsigned")) + ")";

The parentheses ought to live in the properties file.

> 
>+.alert-icon {
>+  width: 48px;
>+  height: 48px;
>+  margin: 0px 20px 6px 6px !important;

Please use -moz-margin-start/end and margin-top/bottom. Why is !important needed?


>+  height: 14em;
>+}
>+
>+#itemWarningIntro {
>+  margin-left: 8px;

-moz-margin-start


>+  margin: 2px 5px 1px 6px;
..
>+#itemWarningIntro {
>+  margin-left: 8px;
> }
>

ditto, in the two other themes as well.

Comment 14

[Dave Townsend [:mossop]]

Attachment: patch rev 2

This address the margin and locale changes.

As for why !important is needed, this is splitting the alert-icon global styling out from its current definition which also has !important in its margin rule so I would prefer to leave this in place rather than potentially affect use of this elsewhere.

Note that the alert icon is changing this globally because it will fix bug 386757.

Comment 15

[Asaf Romano (gone)]

We should probably make the dialog width localizable.

Comment 16

[Dave Townsend [:mossop]]

Attachment: patch rev 3

(In reply to comment #15)
> We should probably make the dialog width localizable.

Done

Comment 17

[Asaf Romano (gone)]

Now, the only problem is that we'll use this generic alert-icon in chrome alerts too, I'm pretty sure Alex didn't realize that when commenting on bug 386757.

Comment 18

[Dave Townsend [:mossop]]

Attachment: patch rev 4

Ok this drops the global style change for the alert icon and just adjusts it in the single dialog.

Comment 19

[Asaf Romano (gone)]

Comment on attachment 297603 [details] [diff] [review]
patch rev 4

s/0px/0/

r=mano.

Comment 20

[Dave Townsend [:mossop]]

Attachment: patch rev 5

Final patch for check in. Requesting approval, this is a change to the UI of the installation dialog only as requested by the UX team, the look has been checked on all 3 platforms.

Comment 21

[Dave Townsend [:mossop]]

Checking in toolkit/locales/en-US/chrome/mozapps/xpinstall/xpinstallConfirm.dtd;
/cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/xpinstall/xpinstallConfirm.dtd,v  <--  xpinstallConfirm.dtd
new revision: 1.3; previous revision: 1.2
done
Checking in toolkit/locales/en-US/chrome/mozapps/xpinstall/xpinstallConfirm.properties;
/cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/xpinstall/xpinstallConfirm.properties,v  <--  xpinstallConfirm.properties
new revision: 1.5; previous revision: 1.4
done
Checking in toolkit/mozapps/xpinstall/content/xpinstallConfirm.js;
/cvsroot/mozilla/toolkit/mozapps/xpinstall/content/xpinstallConfirm.js,v  <--  xpinstallConfirm.js
new revision: 1.10; previous revision: 1.9
done
Checking in toolkit/mozapps/xpinstall/content/xpinstallConfirm.xul;
/cvsroot/mozilla/toolkit/mozapps/xpinstall/content/xpinstallConfirm.xul,v  <--  xpinstallConfirm.xul
new revision: 1.4; previous revision: 1.3
done
Checking in toolkit/mozapps/xpinstall/content/xpinstallItem.xml;
/cvsroot/mozilla/toolkit/mozapps/xpinstall/content/xpinstallItem.xml,v  <--  xpinstallItem.xml
new revision: 1.2; previous revision: 1.1
done
Checking in toolkit/themes/pinstripe/mozapps/xpinstall/xpinstallConfirm.css;
/cvsroot/mozilla/toolkit/themes/pinstripe/mozapps/xpinstall/xpinstallConfirm.css,v  <--  xpinstallConfirm.css
new revision: 1.3; previous revision: 1.2
done
Checking in toolkit/themes/winstripe/mozapps/xpinstall/xpinstallConfirm.css;
/cvsroot/mozilla/toolkit/themes/winstripe/mozapps/xpinstall/xpinstallConfirm.css,v  <--  xpinstallConfirm.css
new revision: 1.7; previous revision: 1.6
done

Comment 22

[Robert Kaiser]

Comment on attachment 297612 [details] [diff] [review]
patch rev 5

>+<!ENTITY dialog.style            "width: 45em">

Hmm, we should specify widths of dialogs (and elements) in ch units, not em units, as em is relative to font height, and ch is the one relative to character widths, actually.