Skip to content
This repository has been archived by the owner on Mar 15, 2018. It is now read-only.

Only register one FxA message handler (bug 1080347) #732

Conversation

mstriemer
Copy link
Contributor

https://bugzilla.mozilla.org/show_bug.cgi?id=1080347

Register the FxA message handler when login.js is loaded instead of when the "Sign In" button is clicked so we only ever register one handler.

@@ -105,6 +105,13 @@ define('login',
}
}));

window.addEventListener('message', function (msg) {
if (!msg.data || !msg.data.auth_code || msg.origin !== settings.api_url) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we check capabilities.fallbackFxA()?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably a good idea

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should never get called if that's the case so I guess we can skip registering this then. We'll need to wait for site_config to load first though.

@washort
Copy link
Contributor

washort commented Oct 17, 2014

r+ if you add a check for fallback fxa to add the handler

@@ -306,6 +309,7 @@ define('login',
// This lets us change the cursor for the "Sign in" link.
persona_def.reject();
z.body.addClass('persona-loaded');
registerFxAPostMessageHandler();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why can this not happen in startLogin?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it will register a callback each time startLogin is called and we'll send the token to zamboni as many times as you've clicked "Sign In" this session.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would probably help if you indicated what you want the comment to say :-)

@mstriemer mstriemer closed this in 4db1a92 Oct 17, 2014
@mstriemer mstriemer deleted the one-event-handler-to-rule-them-all-1080347 branch October 17, 2014 21:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants