1226652 - Add ability to remove multiple (or all) logins from Login Manager

Status: RESOLVED FIXED

(Firefox for iOS :: General, defect)

Description

[Richard Newman [:rnewman]]

… prompting that these will be removed from synced devices (Bug 1225224).

Comment 1

[Stephan Leroux [:sleroux]]

Linked bug to Logins Management feature meta.

I guess we'll need some UI modifications to the current logins manager to facilitate this?

Comment 2

[Richard Newman [:rnewman]]

We just removed the ability to clear logins from the device without uninstalling the app. 

In the absence of this feature, or warnings and functionality when linking accounts, this can lead to leaking passwords between FxAs, amongst other things. 

Steph, please make sure this doesn't fall off your radar.

Comment 3

[Robin Andersen [:tecgirl]]

(In reply to Stephan Leroux [:sleroux] from comment #1)
> Linked bug to Logins Management feature meta.
> 
> I guess we'll need some UI modifications to the current logins manager to
> facilitate this?

Yes, I've started on those with options of clearing one/any/all, but now that I see Richard's comment I'm wondering if it was more intentional to leave it out altogether or simply provide a 'less easy' way to wipe all.

Comment 4

[Richard Newman [:rnewman]]

(In reply to Robin Andersen [:tecgirl] from comment #3)

> Yes, I've started on those with options of clearing one/any/all, but now
> that I see Richard's comment I'm wondering if it was more intentional to
> leave it out altogether or simply provide a 'less easy' way to wipe all.

Sorry, I was unclear. It's not at all intentional to have no way to delete all logins.

I was referring to Bug 1226654 landing before this one -- it means that getting this done is now a little more urgent, because there's currently no way to, e.g., switch FxAs without unexpectedly merging your passwords. It was accidentally shitty doing that through Clear Private Data, but at least a user could figure out how to do it. Now -- between 1.3 and whenever this bug reaches release -- they can't.

My perception of the appropriate level of less-easy:

Edit mode, select all, Remove Selected, with a confirmation prompt like the one we'll still show for CPD ("this will delete the selected logins from your synced devices" or "this will permanently delete the selected logins", depending on whether you're signed in).

I trust your judgment!

Comment 5

[Stephan Leroux [:sleroux]]

So the removal of 'Clear Passwords' is uplifted for 1.3 release and the login manager won't be complete until 2.0. Are we okay with shipping 1.3 with no way to clear passwords or do we need a temporary 'less-easy' way of clearing passwords?

Comment 6

[Richard Newman [:rnewman]]

It's not ideal, but I don't think we should hold the release. We just need to be aware that we have to recommend uninstalling then reinstalling to some users, and we shouldn't miss the next release.

Comment 7

[Richard Newman [:rnewman]]

See Bug 1233281 for inspiration.

Comment 8

[Stephan Leroux [:sleroux]]

Do you have the assets for the checkmark/empty circle when selecting the logins?

Comment 9

[Robin Andersen [:tecgirl]]

Attachment: loginSelection.zip

:sleroux sure do, here you go!

Comment 10

[Stephan Leroux [:sleroux]]

Attachment: Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1384

Comment 11

[Richard Newman [:rnewman]]

Comment on attachment 8701598 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1384

r- on two of the storage parts. I skimmed the rest; bnicholson can do a more thorough job!

Comment 12

[Robin Andersen [:tecgirl]]

:sleroux, it looks like the assets you're using are a bit off. The check looks to be from the UIKit, which is a bit off and the fox is larger and lighter, so I assume you're reusing the one we use on the Settings screen.

I'll attach a comparison of the mock to the current build and the default favicon fox.

Comment 13

[Robin Andersen [:tecgirl]]

Attachment: logins-compare.png

Comment 14

[Robin Andersen [:tecgirl]]

Attachment: favicon-fox.zip

Comment 15

[Brian Nicholson (:bnicholson)]

Comment on attachment 8701598 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1384

rnewman's and tecgirl's comments aside, I think everything else looks good.

Comment 16

[Stephan Leroux [:sleroux]]

Comment on attachment 8701598 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1384

Reflagging bug for UI review with new checkmark and fox icon.

Comment 17

[Robin Andersen [:tecgirl]]

Comment on attachment 8701598 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1384

Assets look good, one nit though…

When choosing to "Select All" the button label should change state to "Deselect All" and perform the action accordingly.

Comment 18

[Stephan Leroux [:sleroux]]

I couldn't reproduce the issue you were seeing with 'Select All' not selecting all and not updating the title of the button. How were you able to cause this?

Comment 19

[Stephan Leroux [:sleroux]]

I'm going to go ahead and land this and if there are any bugs we find we can file follow up issues.

Comment 20

[Stephan Leroux [:sleroux]]

master a39e055f41293abfe1ed1db35fc916a01dd8f3d2