Closed Bug 1121292 Opened 6 years ago Closed 6 years ago

Remove "Remove All" button from password manager

Categories

(Toolkit :: Password Manager, defect, P1)

defect

Tracking

()

RESOLVED WONTFIX

People

(Reporter: Dolske, Assigned: Margaret)

References

Details

User Story

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

Attachments

(1 file, 1 obsolete file)

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.
Blocked by UX design.
Whiteboard: [blocked]
Actually, not blocked! We could remove it today.
Whiteboard: [blocked]
/r/2875 - Bug 1121292 - Remove "Remove All" button from password manager

Pull down this commit:

hg pull review -r 0807f1ab4240eb0897d5f03375131894758ae850
Attachment #8553406 - Flags: review?(MattN+bmo)
I think this should be all it takes. Let me know if I missed something!
Assignee: nobody → margaret.leibovic
Attachment #8553406 - Flags: review?(MattN+bmo)
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.
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)
Attachment #8553406 - Flags: review-
Attachment #8553406 - Flags: review- → review?(MattN+bmo)
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/
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.
Attachment #8553406 - Flags: review?(MattN+bmo) → review+
Comment on attachment 8553406 [details]
MozReview Request: bz://1121292/margaret

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

Ship It!
setting to P1
Priority: -- → P1
User Story: (updated)
> 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.
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.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
Attachment #8553406 - Attachment is obsolete: true
Attachment #8619125 - Flags: review+
You need to log in before you can comment on or make changes to this bug.