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

Fix defaultUrl sending hash content #1132

Merged
merged 1 commit into from
Oct 9, 2017

Conversation

luisrudge
Copy link
Contributor

No description provided.

@luisrudge luisrudge added this to the v10-Next milestone Sep 29, 2017
@luisrudge luisrudge force-pushed the fix-default-redirect-url-without-hash branch from 389d0d3 to c4e1a55 Compare September 29, 2017 21:37
@@ -266,7 +266,7 @@ function extractAuthOptions(options) {
// if responseType was not set and there is a redirectUrl, it defaults to code. Otherwise token.
responseType = typeof responseType === 'string' ? responseType : redirectUrl ? 'code' : 'token';
// now we set the default because we already did the validation
redirectUrl = redirectUrl || window.location.href;
redirectUrl = redirectUrl || `${window.location.origin}${window.location.pathname}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this work on all browsers? Maybe it's better to have a function that splits the string in # and keeps the first left part.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what part? strings interpolation or using location.origin and pathname?

@luisrudge luisrudge force-pushed the fix-default-redirect-url-without-hash branch from c4e1a55 to aaa56b3 Compare October 9, 2017 14:04
@luisrudge luisrudge merged commit 5f2b67d into master Oct 9, 2017
@luisrudge luisrudge deleted the fix-default-redirect-url-without-hash branch October 19, 2017 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants