1470336 - The FxA signin form on about:welcome discards metrics query params

Status: VERIFIED FIXED

(Firefox :: General, defect, P1)

Description

[Ryan Kelly [:rfkelly]]

The new about:welcome page contains a signin form that submits to Firefox Accounts:

  https://dxr.mozilla.org/mozilla-central/rev/681eb7dfa324dd50403c382888929ea8b8b11b00/browser/extensions/activity-stream/content-src/components/StartupOverlay/StartupOverlay.jsx#65

This form declares some metrics query parameters in the <form> action attribute, and some more parameters as <input> elements in the form body, like so:

    <form method="get" action="https://accounts.firefox.com?entrypoint=activity-stream-firstrun&utm_source=activity-stream&utm_campaign=firstrun" target="_blank" rel="noopener noreferrer" onSubmit={this.onSubmit}>
        <input name="service" type="hidden" value="sync" />
        <input name="action" type="hidden" value="email" />
        <input name="context" type="hidden" value="fx_desktop_v3" />
        <input name="email" type="email" required="true"/>
    </form>

Unfortunately, forms do not support splitting up query parameters like this.  When the form is submitted, the query string from the "action" attribute is discarded and values from the <input> elements are appended, as described here:

  https://stackoverflow.com/questions/1116019/submitting-a-get-form-with-query-string-params-and-hidden-params-disappear

So result is that metrics params like entrypoint=activity-stream-firstrun are not passed through to FxA, and we will not be able to provide metrics on the performance of this form.

I recommend moving all the query parameters into additional <input type="hidden"> fields in the form body.

Comment 1

[Chris More [:cmore]]

Thanks for filing the bug rfk! From my ex-engineer days, how you described it above is exactly how I used to handle passing parameters through a form get action. Should be an easy fix.

Comment 2

[Chris More [:cmore]]

Hey Tim: If the current about:welcome lands on release on fx62 in Sept and this change requested above is made in the near future, can this update make it on the 62 release train or will it have to wait for a following release? Thanks!

Comment 3

[Erica Wright [:ewright]]

Thank you for pointing this out, I didn't know this!

Comment 4

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4209

Comment 5

[[github robot]]

Commits pushed to master at https://github.com/mozilla/activity-stream

https://github.com/mozilla/activity-stream/commit/52d9992d662df4b92ad17ef74527c475f1f8615a
Bug 1470336 - Add hidden fields to stop params from disappearing in the url.

https://github.com/mozilla/activity-stream/commit/67b1056d5c19e9ac80700bb946b72cc6ab8e2470
Merge pull request #4209 from ericawright/formfields

Bug 1470336 - Add hidden fields to stop params from disappearing in t…

Comment 6

[Ed Lee :Mardak]

Looking to export these changes for 62 in bug 1470588.

Comment 7

[Ed Lee :Mardak]

https://hg.mozilla.org/mozilla-central/rev/9dcaad7a978f

Comment 8

[Ciprian Muresan [:cmuresan], Ecosystem QA]

I have verified that the issue is no longer reproducible on Windows 10 x64, Mac 10.13.5 and Arch Linux x64. Now, all the metrics parameters are found in the URL after attempting to sign in.

@Ed, I get the impression that this fix would require an uplift, is this the case?

Comment 9

[Ed Lee :Mardak]

[Tracking Requested - why for this release]: about:welcome will be on in 62, so we'll need accurate metrics

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

Can you request uplift to beta? Thanks!

Comment 11

[Erica Wright [:ewright]]

Comment on attachment 8987062 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4209

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1448918
[User impact if declined]: We will not receive entry-point information for fxa metrics upon sign up and sign in.
[Is this code covered by automated tests?]:no
[Has the fix been verified in Nightly?]:yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]:n/a
[Is the change risky?]:no
[Why is the change risky/not risky?]:invisible form fields, no change to the user
[String changes made/needed]:none

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8987062 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4209

Verified in Nightly, let's uplift this for 62 beta 5 so that we'll have accurate metrics.

Comment 13

[Ed Lee :Mardak]

I'll attach a mozilla-beta patch / mozreview for the approved pull request.

Comment 14

[Ed Lee :Mardak]

Attachment: Bug 1470336 - The FxA signin form on about:welcome discards metrics query params.

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

Comment 15

[Ed Lee :Mardak]

Comment on attachment 8989274 [details]
Bug 1470336 - The FxA signin form on about:welcome discards metrics query params.

This was previously reviewed by ursula in comment 5 and approved in comment 12.

Comment 16

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/releases/mozilla-beta/rev/7d3146d065fe