Closed
Bug 1470336
Opened 6 years ago
Closed 6 years ago
The FxA signin form on about:welcome discards metrics query params
Categories
(Firefox :: General, defect, P1)
Firefox
General
Tracking
()
People
(Reporter: rfkelly, Assigned: ewright)
References
Details
User Story
Attachments
(2 files)
52 bytes,
text/x-github-pull-request
|
lizzard
:
approval-mozilla-beta+
|
Details | Review |
59 bytes,
text/x-review-board-request
|
Mardak
:
review+
|
Details |
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•6 years ago
|
||
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•6 years ago
|
||
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!
Flags: needinfo?(tspurway)
Assignee | ||
Comment 3•6 years ago
|
||
Thank you for pointing this out, I didn't know this!
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → ewright
Comment 4•6 years ago
|
||
Comment 5•6 years ago
|
||
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•6 years ago
|
||
Looking to export these changes for 62 in bug 1470588.
Status: NEW → RESOLVED
Iteration: --- → 62.4 - Jul 2
Closed: 6 years ago
Flags: needinfo?(tspurway)
Priority: -- → P1
Resolution: --- → FIXED
Comment 7•6 years ago
|
||
Comment 8•6 years ago
|
||
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•6 years ago
|
||
[Tracking Requested - why for this release]: about:welcome will be on in 62, so we'll need accurate metrics
tracking-firefox62:
--- → ?
Flags: needinfo?(edilee)
Assignee | ||
Comment 11•6 years ago
|
||
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
Flags: needinfo?(ewright)
Attachment #8987062 -
Flags: approval-mozilla-beta?
Comment 12•6 years ago
|
||
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.
Attachment #8987062 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 13•6 years ago
|
||
I'll attach a mozilla-beta patch / mozreview for the approved pull request.
Flags: needinfo?(edilee)
Updated•6 years ago
|
User Story: (updated)
Flags: needinfo?(edilee)
Comment hidden (mozreview-request) |
Comment 15•6 years ago
|
||
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.
Attachment #8989274 -
Flags: review?(usarracini) → review+
Comment 16•6 years ago
|
||
bugherder uplift |
You need to log in
before you can comment on or make changes to this bug.
Description
•