1121291 - Remove "Show Passwords" button from password manager, add ability to show password for single logins

Status: VERIFIED FIXED

(Toolkit :: Password Manager, defect, P1)

Description

[Justin Dolske [:Dolske]]

It's been suggested that we remove the "Show All" button from the password manager.

It has an unclear use case (specifically around showing ALL passwords, see note below), and is a perceived security risk from users who are surprised that someone can walk up, click it, and see everything. The latter is, IMO, not a highly compelling reason for removal, but that it repeatedly comes up and combined with an unclear usecase makes me support removing it.

Note that we _do_ need some mechanism to be able to see the password for at least individual logins. The intent is not to prevent being able to see actual passwords. Details TBD, I think the suggestion was to initially show masked passwords (i.e. "••••••••••"), make it editable upon focus, and show the actual password then.

The "show all" was originally added in the Wallet version of this UI on 2004-03-09, from bug 78754. Seems it was easier to just implement showing everything-or-nothing.

Comment 1

[Robert Kaiser]

For me, the primary use case that use the "show all" thing for is that sometimes I need passwords on a device/application not synched to my Firefox (e.g. Firefox OS or a desktop client for a service that I have a web login for) or autofilling a password doesn't work, but I do not remember the password, so I go into the password manager and want to reveal the password for this site. Right now, "show all" is the only way to do that, but for my usecase it would be enough if *in the password manager* (and that's the important piece) I could reveal a single password.

Comment 2

[Justin Dolske [:Dolske]]

Like I said, there's no intent to remove the ability to view individual passwords.

Comment 3

[Chris Karlof [:ckarlof]]

Blocked by UX design.

Comment 4

[Matthew N. [:MattN]]

Ryan has designed UX for this now. WIP patch (90% done but depending on a <tree> improvement) coming up.

Comment 5

[Matthew N. [:MattN]]

Attachment: MozReview Request: Bug 1121291 - Remove "Show Passwords" button from pwmgr and allow inline password editing. r=Dolske,rchtara

Bug 1121291 - Remove "Show Passwords" button from pwmgr and allow inline password editing.

Comment 6

[Matthew N. [:MattN]]

Screenshot in attachment 8645311 [details]

Comment 7

[Matthew N. [:MattN]]

Comment on attachment 8645314 [details]
MozReview Request: Bug 1121291 - Remove "Show Passwords" button from pwmgr and allow inline password editing. r=Dolske,rchtara

Bug 1121291 - Remove "Show Passwords" button from pwmgr and allow inline password editing. r=Dolske,rchtara

Comment 8

[Riadh Chtara[:rchtara]]

Comment on attachment 8645314 [details]
MozReview Request: Bug 1121291 - Remove "Show Passwords" button from pwmgr and allow inline password editing. r=Dolske,rchtara

https://reviewboard.mozilla.org/r/15515/#review17057

perfect, great job matt :-)

Comment 9

[Justin Dolske [:Dolske]]

Comment on attachment 8645314 [details]
MozReview Request: Bug 1121291 - Remove "Show Passwords" button from pwmgr and allow inline password editing. r=Dolske,rchtara

https://reviewboard.mozilla.org/r/15515/#review17619

r+ with a couple caveats:

1) Telemetry seems to show high usage of this, but I expect that's likely to be from people just wanting to view 1 password. Still, I think we need to do more work to make that a pleasant, discoverable experience -- I think many users will not realize they need to doubleclick/edit the password cell to see it, and it's a bit of an awkward interaction. Matt says y'all have discussed copying the Safari approach (show password on selected rows), or adding a hover-affordance to reveal it.

2) As a result, please don't land in the next few days. This doesn't feel quite ready to ride to Aurora in the pending uplift. (Uplift later on is fine if we think it's ready.)

::: toolkit/components/passwordmgr/content/passwordManager.js:134
(Diff revision 2)
> +    window.close();

Maybe return here? If we're closing the window, no point in running the rest of this code?

::: toolkit/components/passwordmgr/content/passwordManager.js
(Diff revision 2)
> -  Services.telemetry.getHistogramById("PWMGR_MANAGE_VISIBILITY_TOGGLED").add(showingPasswords);

Looking at this telemetry data, out of curiousity...

There are 155K samples on FF40 release, 87% for "show", 13% for "hide". As a rough comparison, PWMGR_NUM_SAVED_PASSWORDS has 19.4M samples (16% have no passwords, or 62% having 0-5 passwords). 155K is fairly small relative to 19M.

But... PWMGR_MANAGE_OPENED, which measures how many times this UI is shown, has 222K samples. 155K is very large relative to that. In fact, it implies ~60% of the time people open this dialog they clock Show Passwords.

Still, this isn't super surprising. I'd expect a major use case to be "see the password I previously saved", and the "Show Passwords" is the only way to do that. We don't really know from this data how many uses actually want to see _all_ their password.

One option would be to improve the UI/affordances to view a single password, and seem how much Show Password usage drops (although I expect some people will continue to use it from habit). Or we can just kill it and watch feedback.

::: toolkit/components/passwordmgr/content/passwordManager.js:307
(Diff revision 2)
> -  if (showingPasswords && aSignon.password &&
> +  if (Services.logins.isLoggedIn && aSignon.password &&

Kind of related to the other bug about maybe prompting for the windows login, but I wonder if we could simplify all the master-password checking stuff by just refusing to open the dialog / show any logins in the first place, if no MP is entered.

Not really relevant here, but something to consider.

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/e4e3a8b66db4

Comment 11

[Matthew N. [:MattN]]

https://reviewboard.mozilla.org/r/15515/#review17619

> Kind of related to the other bug about maybe prompting for the windows login, but I wonder if we could simplify all the master-password checking stuff by just refusing to open the dialog / show any logins in the first place, if no MP is entered.
> 
> Not really relevant here, but something to consider.

That's what my window.close addition was helping with. Before we would show a blank list and no we will never show it (from in-content prefs) or immediately close it in the windowed version (not ideal but arguably no worse than an empty tree).

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e4e3a8b66db4

Comment 13

[Stefan Plewako [:stef]]

This bug seems to add yet another column to password manager but do not adjust dialog size - for English this still works but for a locale with longer labels it doesn't anymore.

Comment 14

[Kamil Jozwiak [:kjozwiak]]

Thanks Stefan. I've created Bug # 1210938.

Comment 15

[Paul Silaghi, QA [:pauly]]

Verified fixed FF 44.0a1 (2015-10-14) Win 7

Comment 16

[Tony Mechelynck [:tonymec]]

UA:"Mozilla/5.0 (X11; Linux x86_64; rv:44.0) Gecko/20100101 Firefox/44.0 SeaMonkey/2.41a1" ID:20151017003002 c-c:fb83f130e2d554c0d0d007c6a5159a7e993e3e07 m-c:01e37977f8da2e1f8b9ce9b777e556ffb1437960 en-US

Verified on Linux x86_64 SeaMonkey by opening chrome://passwordmgr/content/passwordManager.xul in a tab. (The Data Manager, which "Tools → Password Manager → Manage Stored Passwords" opens, is still broken.)

Comment 17

[Majken Connor [:Kensie]]

Please let me know a better place to leave this feedback, but I really dislike the edit UX in place of view. If I can edit it I can change it, I don't want that, I just want to see it. It's also not necessarily intuitive that edit will let you see the password, not just over-write it (when you change your password anywhere else it never lets you see the old password, only submit a new one).

Is there a plan to add view only?

Comment 18

[Robert Kaiser]

I was confused by not having a "View" as well. If me and Kensie, who are well-versed in software, get confused by this, I imaging that people that are not that enthusiastic about technology will have an even harder time to grasp this.

Comment 19

[Justin Dolske [:Dolske]]

(In reply to Justin Dolske [:Dolske] from comment #9)

> r+ with a couple caveats:
> 
> 1) Telemetry seems to show high usage of this, but I expect that's likely to
> be from people just wanting to view 1 password. Still, I think we need to do
> more work to make that a pleasant, discoverable experience -- I think many
> users will not realize they need to doubleclick/edit the password cell to
> see it, and it's a bit of an awkward interaction. Matt says y'all have
> discussed copying the Safari approach (show password on selected rows), or
> adding a hover-affordance to reveal it.
> 
> 2) As a result, please don't land in the next few days. This doesn't feel
> quite ready to ride to Aurora in the pending uplift. (Uplift later on is
> fine if we think it's ready.)

Was there ever a final outcome on what the UI we want here is?

IIRC (and I may not!), it seemed that there was agreement on needing to improve this before shipping. Should we consider backing this out until we're ready to implement that?

Comment 20

[Matthew N. [:MattN]]

(In reply to Justin Dolske [:Dolske] from comment #19)
> Was there ever a final outcome on what the UI we want here is?

Yes, bug 1208145.

> IIRC (and I may not!), it seemed that there was agreement on needing to
> improve this before shipping. Should we consider backing this out until
> we're ready to implement that?

Yes, I agree and it's one of the bugs I mentioned earlier today that I want to finish soon. A straight backout won't work since the password column will be shown permanently for users who used a build with this fix due to XUL persistence being setup for the hidden attribute on that column.

Kairo and Majken, as comment 9 indicates, the plan was never to let this get to release as-is and I was tracking this but unfortunately got pulled off password manager abruptly so have less time for this :(

Comment 21

[Robert Kaiser]

(In reply to Matthew N. [:MattN] from comment #20)
> A straight backout won't work since the password column will
> be shown permanently for users who used a build with this fix due to XUL
> persistence being setup for the hidden attribute on that column.

For now, this looks to be in Developer Edition 44 (and Nightly) only, so I think if we back out right away, the amount of users affected is not too large - but given that this is sensitive, it would be good if we can take care of those users even there. Do you have a good way to do this? Also, can we get a bug filed and tracked for the 44 train to make sure this is done properly and this incomplete experience doesn't ship there?

Comment 22

[Matthew N. [:MattN]]

(In reply to Robert Kaiser (:[e-mail]) from comment #21)
> (In reply to Matthew N. [:MattN] from comment #20)
> > A straight backout won't work since the password column will
> > be shown permanently for users who used a build with this fix due to XUL
> > persistence being setup for the hidden attribute on that column.
> 
> For now, this looks to be in Developer Edition 44 (and Nightly) only, so I
> think if we back out right away, the amount of users affected is not too
> large - but given that this is sensitive, it would be good if we can take
> care of those users even there. Do you have a good way to do this? Also, can
> we get a bug filed and tracked for the 44 train to make sure this is done
> properly and this incomplete experience doesn't ship there?

I already said it's bug 1208145 that we should implement.

Comment 23

[Robert Kaiser]

(In reply to Matthew N. [:MattN] from comment #22)
> I already said it's bug 1208145 that we should implement.

That sounds like it needs an additional string, which can't be done in something that already is on Dev Edition or higher channel.

Comment 24

[captain]

Praise Jesus!  Thanks to these comments I too finally figured out that I just have to double click the password field to see the password.  Still, I'd like to be able to see ALL of the passwords at the same time TOO.  Having BOTH options would be ideal.  Maybe restore the Show Passwords button, but when hovering over it (or upon clicking it) a popup box could inform users that double-clicking on any individual password field will reveal ONLY that one password.

Comment 25

[Matth78]

Same here ! Got a hard time to figure how to see my previous saved passwords... Never would have guessed double clicking will reveal an individual password. But being in edit mode I think it's a bad thing, you could easily overwrote password without intending to.

Not being able to see multiple passwords is really a bad thing as most of the times that is what I want to do => recently profiles and passwords have been stolen on a forum I subscribed. I wanted to check if I didn't used same login/password on other sites but doing this is no longer possible.

Comment 26

[Tony Mechelynck [:tonymec]]

(In reply to captain from comment #24)
> Praise Jesus!  Thanks to these comments I too finally figured out that I
> just have to double click the password field to see the password.  Still,
> I'd like to be able to see ALL of the passwords at the same time TOO. 
> Having BOTH options would be ideal.  Maybe restore the Show Passwords
> button, but when hovering over it (or upon clicking it) a popup box could
> inform users that double-clicking on any individual password field will
> reveal ONLY that one password.

(In reply to Eriatile from comment #25)
> Same here ! Got a hard time to figure how to see my previous saved
> passwords... Never would have guessed double clicking will reveal an
> individual password. But being in edit mode I think it's a bad thing, you
> could easily overwrote password without intending to.
> 
> Not being able to see multiple passwords is really a bad thing as most of
> the times that is what I want to do => recently profiles and passwords have
> been stolen on a forum I subscribed. I wanted to check if I didn't used same
> login/password on other sites but doing this is no longer possible.

You can also right-click anywhere on that line, then click "Edit Password" in the context menu. This makes it more obvious that the password is in read-write mode.

In SeaMonkey which I use, the interface this bug is about is available as chrome://passwordmgr/content/passwordManager.xul which I have bookmarked, and the "Passwords" tab of the Data Manager still has the [Show Passwords] button and its are-you-sure dialog. That Data Manager used to be available as an add-on for Firefox bu apparently it has been retired.

Comment 27

[earl.sinclair.esq]

Yeah, you know, I've always disliked that button. I click it and I click it and I click it... and it never just shows me my passwords. Nope, it asks me, "Are you sure...?". Why, no, I was just randomly clicking buttons in the window to see what happens. OK, not being a complete idiot, when I leave my workstation unattended for more than a second or two, I always make sure to lock it first. ("It was suggested..." Did you suggest it to yourself? that the rest of us are all complete idiots? [oh, were you the one who left your workstation unattended and unlocked and someone--good buddy, no doubt--pranked you? oopsy]) At least, thankfully, there's one add-on that can be relied upon to rectify Mozilla's stupid UX mistakes and provide us users with that which we crave--some real help... until you get rid of XUL/XPCOM so as to exert complete control-freak control over what we see and do in "your" browser (for the few who by then won't have found a browser made once-again to satisfy its users instead of its "makers"). Man, you guys are a laugh-riot (and, no, weren't not laughing *with* you).

Comment 28

[Chris]

(In reply to captain from comment #24)
> Still, 
> I'd like to be able to see ALL of the passwords at the same time TOO. 
> Having BOTH options would be ideal.

(In reply to Eriatile from comment #25)
> Not being able to see multiple passwords is really a bad thing as most of
> the times that is what I want to do => recently profiles and passwords have
> been stolen on a forum I subscribed. I wanted to check if I didn't used same
> login/password on other sites but doing this is no longer possible.

Agree with above regarding ability to see all passwords. Really comes in handy at times as Eriatile has pointed out.