Closed Bug 1386959 Opened 7 years ago Closed 7 years ago

[Form Autofill] Add probe for form autofill availability

Categories

(Toolkit :: Form Manager, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- fixed
firefox57 --- fixed

People

(Reporter: steveck, Assigned: steveck)

References

(Blocks 1 open bug)

Details

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

Attachments

(1 file)

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.
Whiteboard: [form autofill:M4][autofill-metrics]
Whiteboard: [form autofill:M4][autofill-metrics] → [form autofill:M3][autofill-metrics]
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 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 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 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+
Pushed by lchang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/837485049120
[Form Autofill] Add probe for form autofill availability. r=francois,lchang
https://hg.mozilla.org/mozilla-central/rev/837485049120
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
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 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+
(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.