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: prevent error when local storage isn't available #1405

Closed
wants to merge 2 commits into from

Conversation

esinner9
Copy link

@esinner9 esinner9 commented Jan 4, 2022

This allows you to completely disable localStorage while working in an iframe

This allows you to completely disable localStorage while working in an iframe
@bmulholland
Copy link
Contributor

Can you please add a comment explaining why this order is necessary?

@esinner9
Copy link
Author

esinner9 commented Jan 5, 2022

When a nuxt app is used inside of an iframe with this library, you will receive the following error regardless of whether you have localStorage set to false or not. Also, this error only occurs when you are using an incognito window with Third party cookies disabled.

[ERROR] [AUTH] DOMException: Failed to read the 'localStorage' property from 'Window': Access is denied for this document.

When you switch the order, it evaluates whether or not localStorage has been disabled or not first. This allows you to use your nuxt app in an iframe.

@bmulholland
Copy link
Contributor

Thanks for the explanation. Please add that as a comment in the code so that this need is stored where it will be seen for future changes.

@bmulholland bmulholland changed the title Switch order of if check for localStorage fix: prevent error when local storage isn't available Jan 6, 2022
@bmulholland
Copy link
Contributor

Thanks again for the fix. I started by looking at how to clarify the comment -- for code quality, it's critical that we explain why this happened -- and ended up finding a more reliable check. I've done that in #1415, which I hope fixes your issue. Please let me know if it does.

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