1020156 - Need a way to have about:accounts open with a default email address.

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Mark Hammond [:markh] [:mhammond]]

For the sync migration, UX calls for us to open about:accounts with the email address set to a specific value.  ie, we will find the email address specified in the current legacy-sync setup, then open about:accounts such that it defaults to creating an account with this address.  The user will still be able to change the value - it's just a suggestion.

cc zaach as I suspect this will require a change to about:accounts *and* to the back-end.

Comment 1

[Karl Thiessen [:kthiessen, he/him]]

Adding myself to cc list, since I suspect that jbonacci and I will be splitting the testing duty for this feature.

Comment 2

[Mike Hoye [:mhoye]]

Flagging as diamond. Zaach, feel like mentoring this if somebody wants to pick it up?

Comment 3

[Zachary Carter [:zaach]]

I've opened a content-server issue here: https://github.com/mozilla/fxa-content-server/issues/1258

I assume this will work similar to force auth, where the browser appends an "email=" query parameter to the URL.

Comment 4

[Shane Tomlinson [:stomlinson]]

I am taking the content server portion of this.

Comment 5

[Zachary Carter [:zaach]]

The server portion of this is ready. You can open the sign up page with an email query parameter, i.e. `/signup?email=<email-to-prefill>`.

I assume we'd use this for all legacy-sync users who would visit about:accounts. Are there more UX details for someone interested in writing the about:accounts patch?

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Thanks Zach/Shane. The client side of this patch currently depends on other bugs (likely bug 1019985), but once we tackle those this should be more straightforward.

Comment 7

[Drew Willcoxon :adw]

Attachment: Forward URL params from about: URI to remote URL

This cleans up URL param handling and in the process fixes this bug, if I understand it right.  All URL params except for "action" from the about: URI are forwarded on to the remote page, including "email".  So you can open about:account?email=[e-mail] and it'll fill in the email address.

I don't know why wrapper.init try-caught constructing the URL and setting the iframe src.  Bug 913199 seems to be where this was introduced, but I didn't see a reason.  I wouldn't expect that to throw, so I removed it.

If this looks OK, I'll move on to tests next.

Comment 8

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8514581 [details] [diff] [review]
Forward URL params from about: URI to remote URL

Review of attachment 8514581 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

Comment 9

[Drew Willcoxon :adw]

Attachment: Forward URL params from about: URI to remote URL + tests

This is the same as the previous but has two new tests.  They seem kind of redundant since the existing tests hit these paths...  And an end-to-end test that makes sure the email param is copied to the text field in the page would be nice, but I don't think that's possible.  Or should our automated tests use the test servers from bug 1014411?

Comment 10

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8515291 [details] [diff] [review]
Forward URL params from about: URI to remote URL + tests

Review of attachment 8515291 [details] [diff] [review]:
-----------------------------------------------------------------

While end-to-end tests would be ideal, I think the test coverage is OK as it stands.

::: browser/base/content/aboutaccounts/aboutaccounts.js
@@ +334,3 @@
>        });
> +      break;
> +    default:

I think this switch statement is going to cause you merge issues due to bug 1079835, but I'll let you use your judgement how to resolve that.

Comment 11

[Drew Willcoxon :adw]

Attachment: Forward URL params from about: URI to remote URL + tests, unbitrotted

Indeed, here's an unbitrotted patch that still uses the switch statement and accommodates bug 1079835.  Carrying forward the r+.

Comment 12

[Drew Willcoxon :adw]

https://hg.mozilla.org/integration/fx-team/rev/ee0f03650a55

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ee0f03650a55

Comment 14

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Setting for Tracy. We'll pick it up if we have the time.

Comment 15

[Bogdan Maris, Desktop QA]

I see this bug has automated tests, is there need for manual testing? If so can you please offer some guidance? Thanks

Comment 16

[Drew Willcoxon :adw]

Thanks Bogdan, I think this is well covered by the automated test and my own and others' manual testing in the past.