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 MathJax not rendering in Chrome when sessionStorage is disabled. #560

Closed
wants to merge 1 commit into from

Conversation

merijn
Copy link

@merijn merijn commented Sep 8, 2013

If a user has disabled data storage, then accessing "window.sessionStorage" for the typeof throws a SecurityError, stopping MathJax from rendering. The solution is to simply wrap the access to window.sessioStorage in a try/catch and return "undefined" if it throws an exception.

@fred-wang
Copy link
Contributor

I just saw your message on IRC... thanks for your patch! The changes look good to me but I'd like to have Davide's point of view for coding style and so on. @dpvc: can you review that?

@dpvc
Copy link
Member

dpvc commented Sep 10, 2013

I suspect the whole check for Chrome is no longer needed (this was to get around a bug in Chrome 5 (beta), apparently), so I would guess it is safe to remove the whole thing. I'll have to check to make sure the stylesheets can be detected in current versions of Chrome.

The stylesheet detection was pretty fragile (the second most fragile part of MathJax after font detection), and probably deserves looking at again, now that it is 5 years later and browsers have changed a lot. There are some Safari2 things in there, too, as I recall, that could be removed.

@merijn
Copy link
Author

merijn commented Sep 10, 2013

Either would be fine with me, the biggest problem I run into now is that MathJax doesn't render with cookies disabled, as disabling cookies also disables sessionStorage in Chrome.

@pkra
Copy link
Contributor

pkra commented Sep 10, 2013

Could we turn this into an issue instead? (It would also solve the problem of a CLA if this isn't the way to solve the underlying issue...).

@ghost ghost assigned dpvc Sep 17, 2013
@fred-wang
Copy link
Contributor

I'm adding this to the milestone and assigning this to Davide.

@pkra
Copy link
Contributor

pkra commented Sep 17, 2013

Just a reminder: we can't merge this until we sort out a CLA.

@dpvc
Copy link
Member

dpvc commented Sep 17, 2013

I'm expecting to remove the test entirely, so it will not be a merge (sorry merljn). So I don't think a CLA will be needed.

@fred-wang
Copy link
Contributor

I'm closing this and I opened a normal issue instead: #584

@fred-wang fred-wang closed this Sep 25, 2013
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.

None yet

4 participants