Justin Dolske [:Dolske] Reporter
	Description • 10 years ago

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

Mainly because it can be a big footgun. It's also not something that is likely to be used very often, and has acceptable equivalent functionality available via the Remove button, which we are keeping... Most users have just a few logins, so removing them one-by-one is no big deal. But one can also shift-click to select multiple (all) logins, and Remove will delete all selected items.


I did a little code archaeology to see where these came from because I am a glutton for punishment... The toolkit code popped into existence in 2007 (5110f45a0f8d) as a clone of the ancient wallet UI. The last easily accessible version of that was moved in 2003 in bug 26020. After manually fiddling with Bonsai links, I was able to get the pre-move history in http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/extensions/wallet/signonviewer/Attic/SignonViewer.xul 

The "remove all" was added in version 1.18 on 2000-03-15 by morse@netscape.com, no bug for context.

	Chris Karlof [:ckarlof]
	Comment 1 • 10 years ago

Blocked by UX design.

	Chris Karlof [:ckarlof]
	Comment 2 • 10 years ago

Actually, not blocked! We could remove it today.

	:Margaret Leibovic Assignee
	Comment 3 • 10 years ago

/r/2875 - Bug 1121292 - Remove "Remove All" button from password manager

Pull down this commit:

hg pull review -r 0807f1ab4240eb0897d5f03375131894758ae850

	:Margaret Leibovic Assignee
	Comment 4 • 10 years ago

I think this should be all it takes. Let me know if I missed something!

	Matthew N. [:MattN]
	Comment 5 • 10 years ago

Comment on attachment 8553406 [details]
MozReview Request: bz://1121292/margaret

https://reviewboard.mozilla.org/r/2873/#review2317

::: toolkit/components/passwordmgr/content/passwordManager.xul
(Diff revision 1)
> -      <button id="removeAllSignons" icon="clear"

There are 3 references to the ID "removeAllSignons" left in passwordManager.js that I think need to be removed.

You will have to deal with references to `removeAllButton` in passwordManagerCommon.js for DeleteSelectedItemFromTree and DeleteAllFromTree so I guess you'll have to make those arguments optional as I kinda think we should leave "Remove All" on the exceptions dialog as I think it's more common than deleting all passwords.

::: toolkit/components/passwordmgr/content/passwordManager.xul
(Diff revision 1)
> -              label="&removeall.label;" accesskey="&removeall.accesskey;"

I see these 2 strings are still used by the exceptions dialog so that's fine.

	Justin Dolske [:Dolske] Reporter
	Comment 6 • 10 years ago

Comment on attachment 8553406 [details]
MozReview Request: bz://1121292/margaret

ReviewBoard seem to be broken at the moment for me...

- <button id="removeAllSignons" icon="clear"

Unfortunately there are a couple other point in the code that look for this button by ID. So that code needs fixed too. (One getElementById in this file, and the last arg to DeleteSelectedItemFromTree)

	:Margaret Leibovic Assignee
	Comment 7 • 9 years ago

Comment on attachment 8553406 [details]
MozReview Request: bz://1121292/margaret

/r/2875 - Bug 1121292 - Remove "Remove All" button from password manager. r=MattN

Pull down this commit:

hg pull -r 6f8557606e9fff350e049a93a4c0bd9c6558fd6a https://reviewboard-hg.mozilla.org/gecko/

	Matthew N. [:MattN]
	Comment 8 • 9 years ago

https://reviewboard.mozilla.org/r/2875/#review6659

r=me on the technical aspect but preliminary data (from Aurora) indicates that this is used around 2.77% (330/11910) of the time that the management page is opened (assuming there isn't multiple remove all calls in one opening which seems rare). I would prefer to put this bug aside until we have a good reason to remove it. Removing this makes it harder for users to remove all logins as some users don't know about shift-click on lists.

We're going to throw away this UI when we implement the new one so I would rather we do this at the same time so there is some benefit for users who miss this feature.

	Matthew N. [:MattN]
	Comment 9 • 9 years ago

Comment on attachment 8553406 [details]
MozReview Request: bz://1121292/margaret

https://reviewboard.mozilla.org/r/2873/#review6661

Ship It!

	Edwin Wong [:edwong]
	Comment 10 • 9 years ago

setting to P1

	Matthew N. [:MattN]
	Comment 11 • 9 years ago

> As a user I don't want to wipe out all my logins by accident.      

There's a confirmation dialog so it would have to be 2 consecutive accidents.

I think we should WONTFIX for the reasons in comment 8.

	Edwin Wong [:edwong]
	Comment 12 • 9 years ago

Product agrees - there's a confirm dialog which a user can cancel out.  We should be sensitive to removing functionality and not worth the risk considering there will be new UI soon.