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

localStorage.getItem crashing in iframes when running incognito mode #39

Closed
SjirkW opened this issue Sep 9, 2020 · 5 comments · Fixed by #40
Closed

localStorage.getItem crashing in iframes when running incognito mode #39

SjirkW opened this issue Sep 9, 2020 · 5 comments · Fixed by #40

Comments

@SjirkW
Copy link

SjirkW commented Sep 9, 2020

getDisabledOverrides (and probably other functions that use localStorage) is throwing an error

"Uncaught DOMException: Failed to read the 'localStorage' property from 'Window': Access is denied for this document."

in an iframe when running in incognito mode on chrome.

@joeldenning
Copy link
Member

Hi @SjirkW, thanks for reporting this. import-map-overrides relies on localStorage to know which modules are overridden, and I'm not sure if there's something we could replace it with that would work in incognito iframes? We need the overrides to be stored client side and persist when the page is reloaded. The other options I'm aware of are session storage cookies, and maybe client-side dbs supported by browsers?

Do you have a suggestion about what could be changed to fix this?

@frehner
Copy link
Member

frehner commented Sep 9, 2020

yeah, there is a setting in chrome (and potentially added by incognito mode) that blocks localStorage access. I think a good first step for us would be to be more safe around accessing localStorage (e.g. wrap in try/catch) but I'm not sure there is a good alternative for the moment.

see also

https://stackoverflow.com/questions/30481516/iframe-in-chrome-error-failed-to-read-localstorage-from-window-access-deni

bvaughn/react-devtools-experimental#292

@SjirkW
Copy link
Author

SjirkW commented Sep 10, 2020

I agree with frehner that wrapping it in a try/catch is the way to go for now.
The problem I'm having is that my entire application crashes because of this error so wrapping it in a try catch and just returning an empty array should solve that.

The only real solution I can think of is to store settings on the server when local storage is unavailable. This solution doesn't is probably not worth implementing.

@frehner
Copy link
Member

frehner commented Sep 10, 2020

Could maybe even just have an initial attempt to access local storage, and if it fails then just bail early - instead of wrapping all of them and handling the downstream effects. Seems like it would be easier that way

@joeldenning
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants