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

Links in amp-sidebar stopped working in Chrome #6585

Closed
sebastianbenz opened this issue Dec 9, 2016 · 6 comments
Closed

Links in amp-sidebar stopped working in Chrome #6585

sebastianbenz opened this issue Dec 9, 2016 · 6 comments

Comments

@sebastianbenz
Copy link
Contributor

sebastianbenz commented Dec 9, 2016

Examples:

Steps to reproduce:

Click on any of the links in the menu in https://ampbyexample.com. The amp-sidebar closes, but doesn't open the link.

Reproduceable in Chrome on Desktop and Android (Version 54.0.2840.98).
Works in Safari.

This got originally reported here:

ampproject/amp-by-example#521

Update: AMP Version 1481049920709

@murwell
Copy link

murwell commented Dec 9, 2016

About an hour ago, the links in the side-bar just stopped working for my site.

It seemed to have occurred after I had used the AMP Cache URL Format Tool on AmpbyExample. This could be a coincidence. Or not.

The links had been working properly earlier this evening.

I did notice that if I open the link in a new tab, it will follow the link.

@oliverini
Copy link

Can confirm all sites using amp-sidebar (including the project website - https://ampbyexample.com/components/amp-sidebar/ and all sites using Wordpress Accellerated Mobile Pages plugin ) have stopped working on Chrome today.

@mkhatib
Copy link
Contributor

mkhatib commented Dec 9, 2016

This seems like its due to the actual change to close the sidebar when clicking a link.

This is causing a history-pop event and at the same time the browser is trying to navigate away - the navigation gets cancelled by the browser because of history manipulation it seems (it literally says cancelled in the network tab).

I could reproduce something similar in this jsbin (though admittedly might not be the exact same thing) - but if you open network and preserve logs you'll notice that the Go to Google navigation got cancelled.

This might be related to timers firing after a navigation happens and browser not killing them as soon as a navigation happen (this might be the expected behavior not sure).

A quick fix would be to limit the close-sidebar when clicked to local-navigations only (which we handle their history ourselves unlike normal links navigation). This is really what we want with this anyway since the user is navigating away from the document - keeping the sidebar open won't matter.

I'll have a quick fix tomorrow and cherry pick it - or otherwise we should just rollback this change.

@mkhatib mkhatib self-assigned this Dec 9, 2016
@mkhatib mkhatib added this to the Current milestone Dec 9, 2016
@sebastianbenz
Copy link
Contributor Author

How long would it take to rollback?

@cramforce
Copy link
Member

The respective version was rolled back. Confirmed fixed on ABE.

Will keep this open for fix to land.

@letanure
Copy link

letanure commented Dec 9, 2016

Thank you!

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

No branches or pull requests

6 participants