1221290 - "Saved logins" needs to be removed from "Clear private data" on Fennec for the sake of consistency with desktop FF

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Settings and Preferences, defect, P3)

Description

[Anselmo Canfora]

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:42.0) Gecko/20100101 Firefox/42.0
Build ID: 20151029151421
Firefox for Android

Steps to reproduce:

Go to GitHub or whatever site you have credentials saved on password manager and synced via Sync


Actual results:

Login credentials are not filled


Expected results:

Same login credentials that get filled in FF desktop should be filled in FF mobile as well (same Sync account with password sync enabled).

Comment 1

[Anselmo Canfora]

It needs to remove the FF account and add it back again to force pwd sync, really annoying. This is the last of a long string of Sync issues, when Sync stops to sync without notice and you need to reinstate it manually to get it working, *especially* on Android. This badly sucks compared to Chrome smooth and way faster sync feature.

Comment 2

[Ryan Kelly [:rfkelly]]

/cc Nick since this sounds potentially Android-client-specific.

Comment 3

[Anselmo Canfora]

Hello,
some more info:
while reinstating Sync on FF Mobile does make credentials again available, this lasts only for a little bit, then Sync will lose all your credentials again on mobile. Furthermore all entries in "share page with other FF instances" menu will get duplicated after Sync reinstatement.
This makes Sync practically useless for Android FF.

Comment 4

[Anselmo Canfora]

Version info:

FF 42 on Android 6.0

Comment 5

[Nick Alexander :nalexander [he/him]]

(In reply to Anselmo Canfora from comment #4)
> Version info:
> 
> FF 42 on Android 6.0

Anselmo: sounds like you're seeing problems with the passwords database in Fennec.  Question: are you using Master Password?

To help debug this, it would help for you to capture |adb logcat| logs.  There's an add-on https://addons.mozilla.org/en-us/android/addon/logview/ that might help, or you can follow the instructions at http://160.twinql.com/how-to-file-a-good-android-sync-bug/.  Start capturing, force a sync a few times (using Fennec > Settings > Sync > Sync now), and then stop capturing.  I'm interested if you are seeing things like Bug 1217588.

Thanks!

Comment 6

[Anselmo Canfora]

Hi Nick,
thank you for your attention. Correct, I have a master password set in my desktop FF. I just installed the addon you pointed me to and ran several syncs but no data at all shown up in about:logs
Then I tried to disable the master password and ran some more syncs, and something really weird happened: no new passwords did show up in Fennec (BTW, nice to use the old name) until I did authenticate on my *desktop* FF to some sites, then I noticed that upon new syncs the credentials of these sites I authenticated on with the desktop FF got synced to Fennec, *but only those*, all the remaining entries stayed unsynced. I have the feeling you will have fun with this.
PS: sorry for my bad English, hope you will be able fo understand this.

Comment 7

[Anselmo Canfora]

Found the cause of this: a change of settings menu in last Fennec version.
Fennec option "Delete data on exit" *never* contained the item "delete passwords" like the desktop FF dialog "Clear Recent History" does not contains the "delete passwords" item for obvious security reasons.
Now you added this to Fennec, with also a name that in Italian language is ambiguous ("accessi" is used to denote basic authentication accesses in FF parlance). I am also a bot back and front end developer and I use this feature a lot when testing my work, and I am accustomed to the "delete all" FF behaviour being sure that it won't affect my saved passwords and bookmarks. This seems to not be the case anymore on mobile FF.

Comment 8

[Anselmo Canfora]

Also: deletion of access credentials on Fennec is not synced back to desktop FF anymore (luckily in my present case).

Comment 9

[Nick Alexander :nalexander [he/him]]

(In reply to Anselmo Canfora from comment #7)
> Found the cause of this: a change of settings menu in last Fennec version.
> Fennec option "Delete data on exit" *never* contained the item "delete
> passwords" like the desktop FF dialog "Clear Recent History" does not
> contains the "delete passwords" item for obvious security reasons.
> Now you added this to Fennec, with also a name that in Italian language is
> ambiguous ("accessi" is used to denote basic authentication accesses in FF
> parlance). I am also a bot back and front end developer and I use this
> feature a lot when testing my work, and I am accustomed to the "delete all"
> FF behaviour being sure that it won't affect my saved passwords and
> bookmarks. This seems to not be the case anymore on mobile FF.

I respectfully disagree: "Saved logins" has been in that list since https://hg.mozilla.org/mozilla-central/rev/d34f2b0c4e8b, which landed in 2012.

I think removing "Saved logins" is a valuable part of clearing data on exit that we are unlikely to remove, but I could imagine wanting to be clear that Desktop and Android and iOS do the same things here.

Now, I still think you are seeing Password syncing problems, but I can't help without adb logs.

Comment 10

[Nick Alexander :nalexander [he/him]]

barbara: mpopova: I'd like to verify that both Android and iOS have an option to clear "Saved logins", and that both clear "Saved logins" on exit if the "Clear on exit" is selected.

I don't know who would own this product decision on Desktop; can you make sure that mobile and Desktop do the same thing re: clearing "Saved logins" on exit, or explain why this product inconsistency (if it exists!) is desirable?  Thanks!

Comment 11

[Richard Newman [:rnewman]]

This seems like a sibling of Bug 1202810.

On iOS we sync deleted logins. On Android if we don't, that's actually a bug. Bug 1162778 is very relevant here.

We have Bug 1209097 to show a warning prompt, and we have Bug 833045.

Comment 12

[Richard Newman [:rnewman]]

If there's something immediately actionable here, it would be to leave "Saved logins" unchecked by default in Clear Private Data. This is the second time it's surprised a user.

Comment 13

[Maria Popova]

on iOS we don't have "Clear on Exit" but that's just a little nuance.

Comment 14

[Justin Dolske [:Dolske]]

Oops. This bug is probably the "please file a bug" followup from  bug 1102184 that seems to have never happened. :/

That bug (bug 1102184) removed the ability to clear saved logins on shutdown from Desktop. From comment 13 there:

---
Also... Looks like mobile has a fork of some of this. See mobile/android/modules/Sanitizer.jsm and the privacy.item.passwords pref... Seems like we should remove it from mobile too, for the same reasons. Although the contexts are subtly different...

On Desktop, History -> Clear Recent History only clears "Active Logins" (e.g. HTTP auth you've entered during the session). It does not offer to clear logins the user has explicitly _saved_. Clearing saved logins is only a part of the "Clear history when Firefox closes" stuff we're fixing here. Mobile, on the other hand, doesn't seem to have a "Clear when Firefox closes" feature (I suppose in large part because you generally don't close apps!), but does offer to clear _saved_ logins as part of this "Clear Private Data" function.

It's a little more plausible to keep, but I'd still suggest removing it (or at least defaulting it to false for passwords). All the other privacy.item things it clears are things users don't explicitly ask to store (history, cookies, etc -- just like desktop's Clear Recent History). Better still, make this "Clear Active Logins" like Desktop has
---

I'd still suggest either removing it entirely or changing it to just be for "active logins".

Comment 15

[Justin Dolske [:Dolske]]

(In reply to Richard Newman [:rnewman] from comment #11)

> On iOS we sync deleted logins. On Android if we don't, that's actually a
> bug. Bug 1162778 is very relevant here.

It does seem like what ever we do here, there's some Syncish bug... Shouldn't the technically-expected result of deleting all logins on one device be that those logins are deleted from all synced devices? (If users actually expect/want that is a different story ;).

Comment 16

[Richard Newman [:rnewman]]

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

> I'd still suggest either removing it entirely or changing it to just be for
> "active logins".

While I think it's problematic because it rarely matches user expectations, it does seem like there should be *some* way to clear saved logins. If we remove this, I don't think we have one.

So perhaps I can propose two steps:

1. Keep "Saved Logins" in Clear Private Data, but default it to off (on iOS and Android). You must explicitly opt in to clearing logins.

2. Remove it from the list and provide some mechanism closer to where users manage passwords. I suggest a follow-up set of bugs for that.


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

> Shouldn't the technically-expected result of deleting all logins on one
> device be that those logins are deleted from all synced devices?

Yup, exactly.

If Android just drops the contents of the DB then it's wrong, because the data you're clearing has already escaped to the server and your other devices, and will magically come back at some point in the future.

And …

> (If users actually expect/want that is a different story ;).

Bug 1162778; you shouldn't be able to clear data (data that we sync, anyway) on your sync-connected device without making an explicit statement about what you mean -- to clear everywhere or to disconnect-then-clear.

If a user clears their history, they need to understand that it's already 'escaped'. They either want that or they don't, and we won't guess correctly.

(And you probably shouldn't be able to disconnect your device from your account without having the option to decide what happens to your data, but that's not so urgent.)

Comment 17

[Richard Newman [:rnewman]]

Attachment: Default to not clearing logins as part of Clear Private Data. v1

This patch is entirely untested. Guess I should charge one of these Android devices…

Comment 18

[Anselmo Canfora]

(In reply to Richard Newman [:rnewman] from comment #16)
> (In reply to Justin Dolske [:Dolske] from comment #14)
> 
> > I'd still suggest either removing it entirely or changing it to just be for
> > "active logins".
> 
> While I think it's problematic because it rarely matches user expectations,
> it does seem like there should be *some* way to clear saved logins. If we
> remove this, I don't think we have one.

yes you have: Settings -> Privacy -> Manage logins

> 
> So perhaps I can propose two steps:
> 
> 1. Keep "Saved Logins" in Clear Private Data, but default it to off (on iOS
> and Android). You must explicitly opt in to clearing logins.

"Clear Private Data" is intended to discard transient, cache-like, session-bind, automatically recreated items like cache, cookies and so on. That has nothing to do with long term items like bookmarks and login credentials.
The semantics of this "nuke my passwords" thing are basically flawed, this is just another nonsense copycat of a bad idea from Chrome, IMHO.

> 
> 2. Remove it from the list and provide some mechanism closer to where users
> manage passwords. I suggest a follow-up set of bugs for that.

why lose the good chance to do it now?   :)

> 
> (In reply to Justin Dolske [:Dolske] from comment #15)
> 
> > Shouldn't the technically-expected result of deleting all logins on one
> > device be that those logins are deleted from all synced devices?
> 
> Yup, exactly.
> 
> If Android just drops the contents of the DB then it's wrong, because the
> data you're clearing has already escaped to the server and your other
> devices, and will magically come back at some point in the future.
> 
> And …
> 
> > (If users actually expect/want that is a different story ;).
> 
> Bug 1162778; you shouldn't be able to clear data (data that we sync, anyway)
> on your sync-connected device without making an explicit statement about
> what you mean -- to clear everywhere or to disconnect-then-clear.
> 
> If a user clears their history, they need to understand that it's already
> 'escaped'. They either want that or they don't, and we won't guess correctly.
> 
> (And you probably shouldn't be able to disconnect your device from your
> account without having the option to decide what happens to your data, but
> that's not so urgent.)

It seems to me that you are dealing with two different kind of data mixing them up. I would like to point out that here you should draw a clear line to set apart important stuff (bookmarks, passwords, and settings) from not-so-important ones (history, cookies, opened tabs).
The first category should be synchronised, not present in "clear private data" menu and always consistent across devices, so if I add/delete/change an item on a device the change must be reflected on all connected devices.

BTW, I noted now that "clear logins" was not put by me by mistake, I did uncheck that option from my mobile FF, synced and then all my logins were back there. *But now I noticed that logins went away again, and the damn option automagically checked itself again... and "synced tabs" option "unchecks" itself as well. We have a real bug here*.

If you are wondering: I am not trying to beat the record for the longest bug summary   :)

Comment 19

[Anselmo Canfora]

Also, the "Clear on exit" checkbox cannot be set from Privacy menu, it opens always "Select which data to clear" menu, an UI bug here.

Comment 20

[Richard Newman [:rnewman]]

(In reply to Anselmo Canfora from comment #18)

> > While I think it's problematic because it rarely matches user expectations,
> > it does seem like there should be *some* way to clear saved logins. If we
> > remove this, I don't think we have one.
> 
> yes you have: Settings -> Privacy -> Manage logins

Not on iOS, yet.

On Android, I don't think about:logins lets you clear them all. I might be mistaken.


> "Clear Private Data" is intended to discard transient, cache-like,
> session-bind, automatically recreated items like cache, cookies and so on.

No, it's meant to clear private data. You might be thinking of desktop's "Clear Recent History". User behaviors on mobile devices are different, and users do some really weird things.

I am constantly surprised at the number of people who want to do stuff like save logins during a session and throw them away when they quit the browser.


> why lose the good chance to do it now?   :)

Because we have a one-line fix that stops the bleeding (indeed, I just this afternoon submitted changes for review for both Android and iOS). Providing a feature to clear logins elsewhere is more work, merits UX involvement, and likely requires a string change.
 

> It seems to me that you are dealing with two different kind of data mixing
> them up. I would like to point out that here you should draw a clear line to
> set apart important stuff (bookmarks, passwords, and settings) from
> not-so-important ones (history, cookies, opened tabs).

What's important to you isn't necessarily important to everyone else. Some users want to keep their history and clear their logins!

We have lots of different kinds of users. If we were designing a browser just for me, we wouldn't have this feature at all, but here we are.


> BTW, I noted now that "clear logins" was not put by me by mistake, I did
> uncheck that option from my mobile FF, synced and then all my logins were
> back there. *But now I noticed that logins went away again, and the damn
> option automagically checked itself again... and "synced tabs" option
> "unchecks" itself as well. We have a real bug here*.

Please file a different bug with steps to reproduce if you're expecting your data clearing preferences to persist and they're not.

I just checked, and they do on both Android (both clear-on-exit and clear-now) and iOS.

Comment 21

[Richard Newman [:rnewman]]

New bug for your other issue, please.

Comment 22

[Richard Newman [:rnewman]]

Comment on attachment 8687459 [details] [diff] [review]
Default to not clearing logins as part of Clear Private Data. v1

Tested on tablet. Checkbox is off by default.

Comment 23

[Anselmo Canfora]

(In reply to Richard Newman [:rnewman] from comment #21)
> New bug for your other issue, please.

Here you have:

https://bugzilla.mozilla.org/show_bug.cgi?id=1224769

Comment 24

[Anselmo Canfora]

(In reply to Richard Newman [:rnewman] from comment #20)

> > "Clear Private Data" is intended to discard transient, cache-like,
> > session-bind, automatically recreated items like cache, cookies and so on.
> 
> No, it's meant to clear private data. You might be thinking of desktop's
> "Clear Recent History". User behaviors on mobile devices are different, and
> users do some really weird things.

well, I would like to have the name changed to "Clear recent history" to make it just like the desktop version... just kidding, nevermind.

> 
> I am constantly surprised at the number of people who want to do stuff like
> save logins during a session and throw them away when they quit the browser.

I have a good imagination, but this was beyond its reach.

> 
> > why lose the good chance to do it now?   :)
> 
> Because we have a one-line fix that stops the bleeding (indeed, I just this
> afternoon submitted changes for review for both Android and iOS). Providing
> a feature to clear logins elsewhere is more work, merits UX involvement, and
> likely requires a string change.
>  
> 
> > It seems to me that you are dealing with two different kind of data mixing
> > them up. I would like to point out that here you should draw a clear line to
> > set apart important stuff (bookmarks, passwords, and settings) from
> > not-so-important ones (history, cookies, opened tabs).
> 
> What's important to you isn't necessarily important to everyone else. Some
> users want to keep their history and clear their logins!
> 
> We have lots of different kinds of users. If we were designing a browser
> just for me, we wouldn't have this feature at all, but here we are.

yep, you have hundreds millions users, sometimes I am a little bit egocentric... as user, as long this setting will behave correctly (staying unchecked when I uncheck it) I can live with that.

Comment 25

[Richard Newman [:rnewman]]

https://hg.mozilla.org/integration/fx-team/rev/ea060d62594e0cb3522b7fcc9fcfc7ca635007ad
Bug 1221290 - Default to not clearing logins as part of Clear Private Data. r=liuche

Comment 26

[Phil Ringnalda (:philor)]

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

Comment 27

[Barbara Bermes [:barbara] - NI please!]

(In reply to Nick Alexander :nalexander from comment #10)
> barbara: mpopova: I'd like to verify that both Android and iOS have an
> option to clear "Saved logins", and that both clear "Saved logins" on exit
> if the "Clear on exit" is selected.
> 
> I don't know who would own this product decision on Desktop; can you make
> sure that mobile and Desktop do the same thing re: clearing "Saved logins"
> on exit, or explain why this product inconsistency (if it exists!) is
> desirable?  Thanks!

Hi, just tested it out in Nightly, Saved Logins is by default checked off when the Clear private data pops up.

I checked it and it cleared all my logins in about:logins. Then tried to get them back by syncing manually again but nothing...list is still empty.

NI Margaret

Comment 28

[Richard Newman [:rnewman]]

(In reply to Barbara Bermes [:barbara] from comment #27)

> Hi, just tested it out in Nightly, Saved Logins is by default checked off
> when the Clear private data pops up.

Good! Thanks for verifying.
 
> I checked it and it cleared all my logins in about:logins.

So you *checked* the checkbox, and it cleared your logins. That's expected.

> Then tried to get
> them back by syncing manually again but nothing...list is still empty.

That's expected, for one of two technical reasons -- if nothing else, Sync doesn't know you cleared your passwords. But what do you want to happen here?

It sounds like you expected your passwords to come back. That doesn't make sense if you just deliberately deleted them -- you'd clear your passwords, then ten minutes later Sync would run and they'd reappear. User surprise! "Firefox won't let me delete my passwords!"

(If what you want is to make your device match your Sync account contents, that's a different feature that we used to have…)

If you did expect them to come back, my assertion is that we should be correcting that expectation before deleting them (Bug 1162778).

And we should verify that the deletion follows synced data, as Dolske summarized in Comment 15 -- i.e., if a login has 'escaped' into your Sync account, this wipe will 'chase' it. I suspect that Fennec is buggy in that regard.

Comment 29

[Richard Newman [:rnewman]]

Margaret: please flag this if you want it uplifted.

Comment 30

[:Margaret Leibovic]

(In reply to Richard Newman [:rnewman] from comment #29)
> Margaret: please flag this if you want it uplifted.

This seems like a low-risk patch to uplift, I think it's worth it to avoid accidental data loss.

Comment 31

[:Margaret Leibovic]

(In reply to :Margaret Leibovic from comment #30)
> (In reply to Richard Newman [:rnewman] from comment #29)
> > Margaret: please flag this if you want it uplifted.
> 
> This seems like a low-risk patch to uplift, I think it's worth it to avoid
> accidental data loss.

That being said... it feels kinda awkward that there's this one unchecked checkbox in the middle of the dialog. Can we move this checkbox to the bottom?

Also, what's our criteria for including items in this list? Should other things be unchecked by default?

Comment 32

[Richard Newman [:rnewman]]

(In reply to :Margaret Leibovic from comment #31)

> That being said... it feels kinda awkward that there's this one unchecked
> checkbox in the middle of the dialog. Can we move this checkbox to the
> bottom?

I see no reason why not if you feel that would help.

If we were to do that I'd likely move "Cookies and active logins" down, too, because they help to explain each other.


> Also, what's our criteria for including items in this list? Should other
> things be unchecked by default?

I think inertia and whether something is clearable :)

Offline website data, form history, and site settings seem to me like kinda-persistent data that a user might want to keep. But from a different perspective, all of those are similar to cookies and history.

Comment 33

[Richard Newman [:rnewman]]

Comment on attachment 8687459 [details] [diff] [review]
Default to not clearing logins as part of Clear Private Data. v1

Approval Request Comment
[Feature/regressing bug #]:
  N/A

[User impact if declined]:
  Risk of users clearing their logins when they only mean to clear transient browser data.

[Describe test coverage new/current, TreeHerder]:
  None.

[Risks and why]: 
  Low risk; flips a checkbox in Settings.

[String/UUID change made/needed]:
  None.

Comment 34

[:Margaret Leibovic]

Comment on attachment 8687459 [details] [diff] [review]
Default to not clearing logins as part of Clear Private Data. v1

I filed bug 1226209 to update the order of the checkboxes, but let's not block uplift on that. This seems like a simple fix to help prevent user data loss.

Approval Request Comment
[Feature/regressing bug #]: None.

[User impact if declined]: Users may accidentally clear their passwords.

[Describe test coverage new/current, TreeHerder]: No automated tests, landed on m-c.

[Risks and why]: Low-risk. Change to default "checked" state of a single checkbox.

[String/UUID change made/needed]: None.

Comment 35

[:Margaret Leibovic]

Clearly I need to refresh bugs before taking action, since rnewman beat me to this :)

Comment 36

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8687459 [details] [diff] [review]
Default to not clearing logins as part of Clear Private Data. v1

Minor change to a default setting in android preferences.
Sounds good for consistency with desktop. OK to uplift.

Comment 37

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/118a51d986e6

Comment 38

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

This got uplifted earlier today, too:
https://hg.mozilla.org/releases/mozilla-beta/rev/ed7e1814d9e6

Comment 39

[u549602]

Verified as fixed on all builds (45.0a1, 44.0a2 and 43.0b6), the "saved logins" check-box is not enabled when opening the "Clear now" menu.
This issue was tested on Sony Xperia Z2(Android 5.0.2), Moto X (Android 4.4.4)