Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subscriptions: reset the state on action and limit viewers where linking can be done #14013

Merged
merged 1 commit into from Mar 15, 2018

Conversation

dvoytenko
Copy link
Contributor

No description provided.

// This is a very light veiwer resolution since there's no real security
// implication - this only affects on-platform preferences.
const viewerUrl = viewer.getParam('viewerUrl');
if (viewerUrl) {
Copy link
Member

Choose a reason for hiding this comment

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

why do we need 2 different blocks?
cant we just call viewer.getViewerOrigin().then(origin => { and assume that is resolves immediately if viewerUrl is already available?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The comment above explains it. Unfortunately this is a pretty tough situation. The action execution must be synchronous, otherwise we get hit with a popup blocker. Thus I'm trying to resolve one as soon as possible and the other can wait.

@dvoytenko dvoytenko merged commit 52af9d2 into ampproject:master Mar 15, 2018
@dvoytenko dvoytenko deleted the subs12 branch March 15, 2018 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants