Closed Bug 1118871 Opened 5 years ago Closed 5 years ago

Add telemetry to count the number of sites in the password manager blocklist

Categories

(Toolkit :: Password Manager, defect, P1)

defect
Points:
2

Tracking

()

RESOLVED FIXED
mozilla38
Iteration:
38.1 - 26 Jan
Tracking Status
firefox36 --- affected
firefox37 --- fixed
firefox38 --- fixed

People

(Reporter: MattN, Assigned: liuche)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

We can report the number of origins in the blacklist.
Flags: firefox-backlog+
We just need to report the size of the array returned from Services.logins.getAllDisabledHosts();
Points: --- → 2
Flags: qe-verify-
Summary: Add telemetry to count the number of sites in the password manager blacklist → Add telemetry to count the number of sites in the password manager blocklist
Priority: -- → P1
I'll take this one as a first bug, since I haven't done very much Telemetry/toolkit work.
Assignee: nobody → liuche
Attached file MozReview Request: bz://1118871/liuche (obsolete) —
/r/2871 - Bug 1118871 - Add telemetry to count the number of sites in the password manager blocklist. r=MattN

Pull down this commit:

hg pull review -r 1c589669886fcd032ec61c89739626150f22a2fa
Attachment #8553364 - Flags: review?(MattN+bmo)
Comment on attachment 8553364 [details]
MozReview Request: bz://1118871/liuche

https://reviewboard.mozilla.org/r/2869/#review2127

Pushed with fixes: https://hg.mozilla.org/integration/fx-team/rev/40079e3249d1

::: toolkit/components/telemetry/Histograms.json
(Diff revision 1)
> +    "description": "The number of host sites for which the user has explicitly rejected saving a login"

Nit: s/host /

Nit: I think the description could cause this probe to be confused with the user choosing not to save a login after submitting. How about:
s/a login/logins/.

::: toolkit/components/passwordmgr/nsLoginManager.js
(Diff revision 1)
> +      let numPasswordsBlocklist = Services.telemetry.getHistogramById("PWMGR_NUM_PASSWORDS_BLOCKLIST");

This histogram name doesn't match Histograms.json so this will throw.
Attachment #8553364 - Flags: review?(MattN+bmo) → review+
https://hg.mozilla.org/mozilla-central/rev/40079e3249d1
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Iteration: --- → 38.1 - 26 Jan
Comment on attachment 8553364 [details]
MozReview Request: bz://1118871/liuche

Approval Request Comment
[Feature/regressing bug #]: Improvements to password manager in 2015 - Bug 1121127
[User impact if declined]: Possibly less ideal decisions based on lack of data.
[Describe test coverage new/current, TreeHerder]: No tests for the telemetry since it's outside the normal code flow so shouldn't break other aspects of password manager if an exception occurs. No problems have been reported on Nightly and data looks good there.
[Risks and why]: Low risk since it's new code only run during the telemetry gathering phase.
[String/UUID change made/needed]: None
Attachment #8553364 - Flags: approval-mozilla-beta?
Attachment #8553364 - Flags: approval-mozilla-aurora?
Comment on attachment 8553364 [details]
MozReview Request: bz://1118871/liuche

I'm not sure about the value of uplifting this patch to Beta at this point in the cycle. (I'll leave it to Sylvestre to make the call on that.) Happy to take this on Aurora.

Aurora+
Attachment #8553364 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8553364 [details]
MozReview Request: bz://1118871/liuche

Too late for beta.
Attachment #8553364 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Attachment #8553364 - Attachment is obsolete: true
Attachment #8619060 - Flags: review+
You need to log in before you can comment on or make changes to this bug.