Closed
Bug 981210
Opened 10 years ago
Closed 10 years ago
FxAccountsManager.jsm and consumers: change "accountId" attribute to "email"
Categories
(Firefox :: Firefox Accounts, defect)
Tracking
()
RESOLVED
FIXED
Firefox 32
People
(Reporter: jhirsch, Assigned: spenrose)
References
Details
Attachments
(1 file, 1 obsolete file)
18.91 KB,
patch
|
Details | Diff | Splinter Review |
Right now, when I login using the fxa test client, the FxAccountsIACHelper.openFlow() callback gets: {"email": "foo@bar.com", "success": true, "verified": true} but if I then do FxAccountsIACHelper.getAccounts(), the response is: {"accountId": "foo@bar.com", "verified": true} We should use accountId everywhere for consistency.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → spenrose
Assignee | ||
Comment 1•10 years ago
|
||
Jed and I chatted about this and noticed that the UI refers to "email" and the value of the field must be a orutable email, so we're inclined to go with "email." Thoughts?
Flags: needinfo?(6a68)
Assignee | ||
Comment 3•10 years ago
|
||
This comes down to a single attribute name in services/fxaccounts/FxAccountsManager.jsm which gets copied all over the place. So it's a straightforward find-and-replace for both gecko and gaia, but I'm a little concerned about coordinating the paired patches. Jared, how would you feel about tweaking the consumers of FxAccountsIACHelper to look for email and fall back to accountId? Then I could update gecko safely. At the same time I'd like to move all this to nice-to-have. So a new gaia bug to that effect would block 974121 and this bug, and this one would block 974096. Once this bug lands, we could then remove the fallback.
Flags: needinfo?(6a68)
Assignee | ||
Comment 4•10 years ago
|
||
Per IRC with Jared, moving this to nice-to-have.
Reporter | ||
Comment 5•10 years ago
|
||
Hey Sam - sorry for the lack of response here. I'll fix this inside the settings patch (we can skip opening another bug). Would you mind opening a bug to update the system app?
Assignee | ||
Updated•10 years ago
|
Status: NEW → ASSIGNED
Flags: needinfo?(spenrose)
Summary: JSON response after login flow should use 'accountId', not 'email' → FxAccountsManager.jsm and consumers: change "accountId" attribute to "email"
Assignee | ||
Comment 7•10 years ago
|
||
This is a pure search-and-replace, except for the bit in FxAccountsMgmtService.jsm to catch accountId. Once this patch lands, we can take accountId out of Gaia, and then remove that kludge. Thanks Jed!
Attachment #8414771 -
Flags: review?(jparsons)
Comment 8•10 years ago
|
||
Comment on attachment 8414771 [details] [diff] [review] 981210-accountId-to-email.patch Review of attachment 8414771 [details] [diff] [review]: ----------------------------------------------------------------- That looks good to me. I searched services/ and b2g/ and don't find any other instances that need changing. Thanks, Sam!
Attachment #8414771 -
Flags: review?(jparsons) → review+
Assignee | ||
Comment 9•10 years ago
|
||
Attachment #8414771 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 10•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d05f273a2d1d
Keywords: checkin-needed
Comment 11•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d05f273a2d1d
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Updated•6 years ago
|
Product: Core → Firefox
Updated•6 years ago
|
Target Milestone: mozilla32 → Firefox 32
You need to log in
before you can comment on or make changes to this bug.
Description
•