Closed Bug 1307382 Opened 3 years ago Closed 3 years ago

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

Categories

(Firefox :: Migration, defect)

51 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 52
Tracking Status
firefox50 --- fixed
firefox51 --- fixed
firefox52 --- fixed

People

(Reporter: Gijs, Assigned: Gijs)

References

Details

Attachments

(1 file)

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) ?
Flags: needinfo?
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...
Flags: needinfo?
Comment on attachment 8797537 [details]
Bug 1307382 - fix sync case for automigration undo telemetry,

https://reviewboard.mozilla.org/r/83224/#review81712
Attachment #8797537 - Flags: review?(markh) → review+
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/1bb61a26fb37
fix sync case for automigration undo telemetry, r=markh
https://hg.mozilla.org/mozilla-central/rev/1bb61a26fb37
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
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
Attachment #8797537 - Flags: approval-mozilla-beta?
Attachment #8797537 - Flags: approval-mozilla-aurora?
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
Attachment #8797537 - Flags: approval-mozilla-beta?
Attachment #8797537 - Flags: approval-mozilla-beta+
Attachment #8797537 - Flags: approval-mozilla-aurora?
Attachment #8797537 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.