Closed
Bug 945278
Opened 11 years ago
Closed 11 years ago
Notify about 'getAssertion' errors
Categories
(Firefox OS Graveyard :: General, defect)
Firefox OS Graveyard
General
Tracking
(Not tracked)
RESOLVED
FIXED
1.4 S2 (28feb)
People
(Reporter: ferjm, Assigned: jedp)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
ferjm
:
review+
|
Details | Diff | Splinter Review |
The FxAccountsManager module introduced in bug 929386 exposes a .getAssertion() method that allows RPs to get fxa assertions via the navigator.mozId API.
In order to get an assertion, there must be a valid and verified account in the device. If there is no account signed in the device, the sign in UI flow is triggered, but this flow can end up with an error or with a non verified signed in account. In both cases we need to inform about the result (1) to the user and (2) to the developer.
For (1), IMHO the most critical case is the unverified account one, where the user is asked to create an account after clicking in a RP "Sign In" button and after creating (or login with) an account the assertion is not provided to the RP and there is no notification about it. I would suggest to introduce a new system dialog explaining that the user is trying to log in in a RP with an unverified account.
For (2), it would be great if we could add an 'onerror' callback to the current navigator.id API. This way, devs will know why the .request calls are failing and will be able to react properly.
Reporter | ||
Comment 1•11 years ago
|
||
Jed, Lloyd, I would love to hear your thoughts about a potential addition of a 'onerror' callback to the mozId.watch() call.
Assignee | ||
Comment 3•11 years ago
|
||
Hi, Fernando, would this be sufficient?
Lloyd, Sam, what do you think about the enhancement to the RP API?
Attachment #8363309 -
Flags: feedback?(ferjmoreno)
Assignee | ||
Comment 4•11 years ago
|
||
(If the patch doesn't apply cleanly for you, try applying the DOM API patch from Bug 929386 first)
Comment 5•11 years ago
|
||
I think we need it. +1 to patch direction.
(In reply to Jed Parsons (use needinfo, please) [:jedp, :jparsons] from comment #3)
> Lloyd, Sam, what do you think about the enhancement to the RP API?
Reporter | ||
Comment 6•11 years ago
|
||
Comment on attachment 8363309 [details] [diff] [review]
Add onerror handler to receive getAssertion errors
Thanks Jed!
Attachment #8363309 -
Flags: feedback?(ferjmoreno) → feedback+
Assignee | ||
Comment 7•11 years ago
|
||
Comment on attachment 8363309 [details] [diff] [review]
Add onerror handler to receive getAssertion errors
I'm hearing that this is an ok addition.
ferjm, can I ask you for r=ferjm this time? still look good to you?
Thanks!
j
Attachment #8363309 -
Flags: review?(ferjmoreno)
Reporter | ||
Comment 8•11 years ago
|
||
Comment on attachment 8363309 [details] [diff] [review]
Add onerror handler to receive getAssertion errors
Thanks Jed. LGTM. Is it possible to also add a test to toolkit/identity/tests/unit/test_firefox_accounts.js ?
Attachment #8363309 -
Flags: review?(ferjmoreno)
Flags: needinfo?(lhilaiel)
Assignee | ||
Comment 9•11 years ago
|
||
Good point. Yes.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → jparsons
Assignee | ||
Comment 10•11 years ago
|
||
Added test in toolkit/identity/tests/unit/test_firefox_accounts.js
Thanks for your review, Fernando!
Attachment #8363309 -
Attachment is obsolete: true
Attachment #8378469 -
Flags: review?(ferjmoreno)
Assignee | ||
Updated•11 years ago
|
Status: NEW → ASSIGNED
Reporter | ||
Updated•11 years ago
|
Attachment #8378469 -
Flags: review?(ferjmoreno) → review+
Comment 12•11 years ago
|
||
With the bug # fixed in the commit message.
https://hg.mozilla.org/integration/b2g-inbound/rev/7a3702cd921d
Flags: in-testsuite+
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.4 S2 (28feb)
Assignee | ||
Updated•11 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•