Closed
Bug 1030227
Opened 8 years ago
Closed 8 years ago
FxA is not firing onlogin for RPs such as FMD which have called watch()
Categories
(Firefox OS Graveyard :: FxA, defect)
Tracking
(blocking-b2g:2.0+, firefox31 wontfix, firefox32 fixed, firefox33 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)
People
(Reporter: spenrose, Assigned: spenrose)
References
Details
Attachments
(1 file, 1 obsolete file)
3.12 KB,
patch
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Updated•8 years ago
|
blocking-b2g: --- → 2.0?
Assignee | ||
Comment 1•8 years ago
|
||
As a follow-up, I think the getAssertion().then(rp.doLogin()) bit should be broken out into a common routine, but ONLOGIN -> request() seems safe to me for now. Thanks Jed!
Attachment #8446008 -
Flags: review?(jparsons)
Assignee | ||
Comment 2•8 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=69934249e181
Updated•8 years ago
|
Target Milestone: --- → 2.0 S5 (4july)
Updated•8 years ago
|
blocking-b2g: 2.0? → 2.0+
Comment on attachment 8446008 [details] [diff] [review] 1030227-fire-onlogin.patch Review of attachment 8446008 [details] [diff] [review]: ----------------------------------------------------------------- I don't think we have tests for any of fxaccounts:onlogin, :onlogout, or :onverified, do we? We ought to ensure that they result in onlogin() etc. being fired for an RP. And we should ensure that these sequences are on the checklist of manual tests to perform. I agree that some refactoring could be done with getAssertion().then(rp.doLogin()). One little fix, apart from which, this looks good to me! Thanks, Sam. ::: toolkit/identity/FirefoxAccounts.jsm @@ +90,5 @@ > for (let [rpId,] of this._rpFlows) { > this.doLogout(rpId); > } > break; > case "quit-application-granted": After the test for ONVERIFIED_NOTIFICATION, can you remove the observers for ONLOGIN and also ONLOGOUT, which seems to have been missed in some prior patch?
Attachment #8446008 -
Flags: review?(jparsons) → review+
Assignee | ||
Comment 4•8 years ago
|
||
Thanks Jed, especially for catching that leak. Agreed on all points.
Attachment #8446008 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 5•8 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/1b0e65192c5d
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1b0e65192c5d
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Comment 7•8 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/f0fb42043629
status-b2g-v2.0:
--- → fixed
status-b2g-v2.1:
--- → fixed
status-firefox31:
--- → wontfix
status-firefox32:
--- → fixed
status-firefox33:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•