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 for popstate firing on page load. #1425

Merged
merged 1 commit into from Oct 15, 2012

Conversation

Projects
None yet
5 participants
@ghempton
Member

ghempton commented Oct 3, 2012

On Chrome popstate is fired on page load (see http://stackoverflow.com/questions/6421769/popstate-on-pages-load-in-chrome). This causes the initial route to be entered twice. This PR fixes that.

I would add a test for this, but due it it requiring a fresh page load, it is very non-trivial.

@rlivsey

This comment has been minimized.

Show comment
Hide comment
@rlivsey

rlivsey Oct 3, 2012

Contributor

I can confirm that this fixes the problem in my app, cheers

Contributor

rlivsey commented Oct 3, 2012

I can confirm that this fixes the problem in my app, cheers

@wagenet

This comment has been minimized.

Show comment
Hide comment
@wagenet

wagenet Oct 3, 2012

Member

@ghempton Sounds good. I assume you've also tested this on browsers that don't trigger the initial popState?

Member

wagenet commented Oct 3, 2012

@ghempton Sounds good. I assume you've also tested this on browsers that don't trigger the initial popState?

@ghempton

This comment has been minimized.

Show comment
Hide comment
@ghempton

ghempton Oct 3, 2012

Member

@wagenet yup, I've tested on FF which doesn't trigger on page load.

Member

ghempton commented Oct 3, 2012

@wagenet yup, I've tested on FF which doesn't trigger on page load.

@digitaltoad

This comment has been minimized.

Show comment
Hide comment
@digitaltoad

digitaltoad Oct 3, 2012

Contributor

I have a branch that I'm working on to use replaceState on HistoryLocation init. Would this still be an issue at that point?

Contributor

digitaltoad commented Oct 3, 2012

I have a branch that I'm working on to use replaceState on HistoryLocation init. Would this still be an issue at that point?

@ghempton

This comment has been minimized.

Show comment
Hide comment
@ghempton

ghempton Oct 3, 2012

Member

@digitaltoad I think this fix will still be needed.

Member

ghempton commented Oct 3, 2012

@digitaltoad I think this fix will still be needed.

@wycats

This comment has been minimized.

Show comment
Hide comment
@wycats

wycats Oct 15, 2012

Member

@digitaltoad what is the status of your branch?

Member

wycats commented Oct 15, 2012

@digitaltoad what is the status of your branch?

wycats added a commit that referenced this pull request Oct 15, 2012

Merge pull request #1425 from ghempton/fix-popstate
Fix for popstate firing on page load.

@wycats wycats merged commit 0a4dd24 into emberjs:master Oct 15, 2012

1 check passed

default The Travis build passed
Details
@digitaltoad

This comment has been minimized.

Show comment
Hide comment
@digitaltoad

digitaltoad Oct 15, 2012

Contributor

I haven't submitted it due to testing. Not really sure the best way to test that replaceState is used without using replaceState.

Contributor

digitaltoad commented Oct 15, 2012

I haven't submitted it due to testing. Not really sure the best way to test that replaceState is used without using replaceState.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment