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

Attempt to use replaceState to validate we can use the History API. #1463

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Attempt to use replaceState to validate we can use the History API. #1463

wants to merge 1 commit into from

Conversation

stevemckenzie
Copy link

Checking if history (pushState) is supported will pass for Firefox Addons with the current implementation but is actually not supported. You cannot tell though until you attempt to actually use the API.

For more info, I originally reported this here: remix-run/react-router#535

Also note the TODO. Looking for suggestions for that.

@stevemckenzie
Copy link
Author

Can I get some feedback on this please :)

catch (e) {
return false;
}

// Return the regular check
return (window.history && 'pushState' in window.history);
Copy link
Member

Choose a reason for hiding this comment

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

we may as well return true at this point, since we would have had to have used replaceState

@patrickkettner
Copy link
Member

Really sorry for taking so long to get to a proper review. Total failure on my part :[

Digging in some, it seems like the firefox issue in #1270 was fixed as of version ~29. I would be much more comfortable using an iframe.

Hows that sound?

@ryanseddon
Copy link
Member

@stevemckenzie, @patrickkettner bump what do we need to do to get this merged?

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

3 participants