[Form Autofill] Add probe for form autofill availability

RESOLVED FIXED in Firefox 56

Status

()

Toolkit
Form Manager
P1
normal
RESOLVED FIXED
11 months ago
10 months ago

People

(Reporter: steveck, Assigned: steveck)

Tracking

(Blocks: 1 bug)

unspecified
mozilla57
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox55 unaffected, firefox56 fixed, firefox57 fixed)

Details

(Whiteboard: [form autofill:M3][autofill-metrics])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

11 months ago
We should be able to know whether the feature is available or not after bug 1385196 landed. PMs would like to know the number of available users.
(Assignee)

Updated

11 months ago
Whiteboard: [form autofill:M4][autofill-metrics]
status-firefox55: --- → unaffected
status-firefox56: --- → affected
status-firefox57: --- → affected
status-firefox-esr52: --- → unaffected
Whiteboard: [form autofill:M4][autofill-metrics] → [form autofill:M3][autofill-metrics]
Comment hidden (mozreview-request)

Comment 2

10 months ago
mozreview-review
Comment on attachment 8896833 [details]
Bug 1386959 - [Form Autofill] Add probe for form autofill availability.

https://reviewboard.mozilla.org/r/168128/#review173282

::: browser/extensions/formautofill/bootstrap.js:71
(Diff revision 1)
>    if (!isAvailable()) {
>      Services.prefs.clearUserPref("dom.forms.autocomplete.formautofill");
> +    Services.telemetry.scalarSet("formautofill.availability", false);
>      return;
>    }
> +  Services.telemetry.scalarSet("formautofill.availability", true);

nit: put this line below `Services.prefs.setBoolPref("dom.forms.autocomplete.formautofill", true);`.
Attachment #8896833 - Flags: review?(lchang) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 5

10 months ago
mozreview-review
Comment on attachment 8896833 [details]
Bug 1386959 - [Form Autofill] Add probe for form autofill availability.

https://reviewboard.mozilla.org/r/168128/#review174182

::: toolkit/components/telemetry/Scalars.yaml:793
(Diff revision 3)
>  # The following section contains the form autofill related scalars.
> +formautofill:
> +  availability:
> +    bug_numbers:
> +      - 1386959
> +    description: A boolean to represent whether the formautofill is available

Can you expand on this description a little?

- Is it sent everytime you encounter a form on a page?
- Does a form need to contain a field of a particular type (e.g. text field) for this signal to get sent?
- Is it sent more than once per top-level page load (if there's more than one form on the page)?
- Is it sent if the formautofill pref is OFF?

::: toolkit/components/telemetry/Scalars.yaml:794
(Diff revision 3)
> +formautofill:
> +  availability:
> +    bug_numbers:
> +      - 1386959
> +    description: A boolean to represent whether the formautofill is available
> +    expires: never

This may send a lot of pings back to our servers.

Would it be reasonable to start with an expiry in say 6 releases and then renew the probe if we're still using the data?
Attachment #8896833 - Flags: review?(francois) → review-
(In reply to François Marier [:francois] from comment #5)
> Comment on attachment 8896833 [details]
> Bug 1386959 - [Form Autofill] Add probe for form autofill availability.
> 
> https://reviewboard.mozilla.org/r/168128/#review174182
> 
> ::: toolkit/components/telemetry/Scalars.yaml:793
> (Diff revision 3)
> >  # The following section contains the form autofill related scalars.
> > +formautofill:
> > +  availability:
> > +    bug_numbers:
> > +      - 1386959
> > +    description: A boolean to represent whether the formautofill is available
> 
> Can you expand on this description a little?
> 
> - Is it sent everytime you encounter a form on a page?
> - Does a form need to contain a field of a particular type (e.g. text field)
> for this signal to get sent?
> - Is it sent more than once per top-level page load (if there's more than
> one form on the page)?
> - Is it sent if the formautofill pref is OFF?

This is only recorded once per session to indicate if the form autofill feature is available in the build. We are rolling it out by locale and geolocation so we'll use this to filter out telemetry to look at users who actually have the feature.

> ::: toolkit/components/telemetry/Scalars.yaml:794
> (Diff revision 3)
> > +formautofill:
> > +  availability:
> > +    bug_numbers:
> > +      - 1386959
> > +    description: A boolean to represent whether the formautofill is available
> > +    expires: never
> 
> This may send a lot of pings back to our servers.
> 
> Would it be reasonable to start with an expiry in say 6 releases and then
> renew the probe if we're still using the data?

Since it's only once per session I guess this would no longer be a concern?
Flags: needinfo?(francois)
(In reply to Matthew N. [:MattN] (huge backlog; PM if requests are blocking you) from comment #6)
> This is only recorded once per session to indicate if the form autofill
> feature is available in the build. We are rolling it out by locale and
> geolocation so we'll use this to filter out telemetry to look at users who
> actually have the feature.

Ok great. Maybe we can rephrase the description to something like:

  A boolean sent once per session to represent whether the formautofill is available in the build

> > Would it be reasonable to start with an expiry in say 6 releases and then
> > renew the probe if we're still using the data?
> 
> Since it's only once per session I guess this would no longer be a concern?

Yes that's fine. You can ignore that comment.
Flags: needinfo?(francois)
Status: NEW → ASSIGNED
Priority: -- → P1
Comment hidden (mozreview-request)
(Assignee)

Comment 9

10 months ago
mozreview-review-reply
Comment on attachment 8896833 [details]
Bug 1386959 - [Form Autofill] Add probe for form autofill availability.

https://reviewboard.mozilla.org/r/168128/#review174182

> Can you expand on this description a little?
> 
> - Is it sent everytime you encounter a form on a page?
> - Does a form need to contain a field of a particular type (e.g. text field) for this signal to get sent?
> - Is it sent more than once per top-level page load (if there's more than one form on the page)?
> - Is it sent if the formautofill pref is OFF?

Description is rephrased per your suggestion, thanks.

Comment 10

10 months ago
mozreview-review
Comment on attachment 8896833 [details]
Bug 1386959 - [Form Autofill] Add probe for form autofill availability.

https://reviewboard.mozilla.org/r/168128/#review174774

datareview+
Attachment #8896833 - Flags: review?(francois) → review+

Comment 11

10 months ago
Pushed by lchang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/837485049120
[Form Autofill] Add probe for form autofill availability. r=francois,lchang

Comment 12

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/837485049120
Status: ASSIGNED → RESOLVED
Last Resolved: 10 months ago
status-firefox57: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment on attachment 8896833 [details]
Bug 1386959 - [Form Autofill] Add probe for form autofill availability.

Approval Request Comment
[Feature/Bug causing the regression]: Form autofill
[User impact if declined]: There will be no reliable way to know how many users have autofill available (en-US + US geo.) which will affect stats we can check
[Is this code covered by automated tests?]: No, simple telemetry probe
[Has the fix been verified in Nightly?]: No, not necessary since it's trivial
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Simple telemetry probe
[String changes made/needed]: None
Attachment #8896833 - Flags: approval-mozilla-beta?

Comment 14

10 months ago
Comment on attachment 8896833 [details]
Bug 1386959 - [Form Autofill] Add probe for form autofill availability.

Add telemetry for autofill. Beta56+.
Attachment #8896833 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 15

10 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/72f3e9b7c6ea
status-firefox56: affected → fixed
(In reply to Matthew N. [:MattN] (huge backlog; PM if requests are blocking you) from comment #13)
> [Is this code covered by automated tests?]: No, simple telemetry probe
> [Has the fix been verified in Nightly?]: No, not necessary since it's trivial
> [Needs manual test from QE? If yes, steps to reproduce]: No

Setting qe-verify- based on Matthew's assessment on manual testing needs.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.