1307382 - FX_STARTUP_MIGRATION_UNDO_REASON histogram has no hits for the 'signed into sync' case

Status: RESOLVED FIXED

(Firefox :: Migration, defect)

Description

[:Gijs (he/him)]

The explanation for this particular issue turns out to be quite simple: before sync signs in, it will store your sync credentials in the login manager. That adds a login, and that triggers the "you saved a password" case of the undo code, which means that the 'you saved a password' case is covering the cases where people sign into sync and/or save a password manually.

Mark, is there a notification sync will fire *before* saving its credentials, perhaps when starting to sign in (and also when signing up, I suppose) ?

Comment 1

[:Gijs (he/him)]

Mark said the basic way this gets worked around in other places is to just ignore the sync password being saved, which seems fair enough...

Comment 2

[:Gijs (he/him)]

Attachment: Bug 1307382 - fix sync case for automigration undo telemetry,

Review commit: https://reviewboard.mozilla.org/r/83224/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/83224/

Comment 3

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8797537 [details]
Bug 1307382 - fix sync case for automigration undo telemetry,

https://reviewboard.mozilla.org/r/83224/#review81712

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1bb61a26fb37
fix sync case for automigration undo telemetry, r=markh

Comment 5

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

https://hg.mozilla.org/mozilla-central/rev/1bb61a26fb37

Comment 6

[:Gijs (he/him)]

Comment on attachment 8797537 [details]
Bug 1307382 - fix sync case for automigration undo telemetry,

Approval Request Comment
[Feature/regressing bug #]: getting telemetry for reasons the 'undo' part of automigration is turned off
[User impact if declined]: we won't know how many people use sync vs. how many people save other passwords
[Describe test coverage new/current, TreeHerder]: nope
[Risks and why]: very low risk, tiny change to an observer() - even if there were issues with the code, the main risk would be that the telemetry gets worse, not better. I'll take that risk.
[String/UUID change made/needed]: nope

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8797537 [details]
Bug 1307382 - fix sync case for automigration undo telemetry,

Improved telemetry results, taking it.
Should be in 50 beta 5

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/02cc4b849623

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/437cc9b7e860