1030227 - FxA is not firing onlogin for RPs such as FMD which have called watch()

Status: RESOLVED FIXED

(Firefox OS Graveyard :: FxA, defect)

Comment 1

[Sam Penrose]

Attachment: 1030227-fire-onlogin.patch

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!

Comment 2

[Sam Penrose]

https://tbpl.mozilla.org/?tree=Try&rev=69934249e181

Comment 3

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

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?

Comment 4

[Sam Penrose]

Attachment: 1030227-fire-onlogin.patch

Thanks Jed, especially for catching that leak. Agreed on all points.

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/b2g-inbound/rev/1b0e65192c5d

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1b0e65192c5d

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f0fb42043629