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.

Sign up for GitHub

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

Jump to bottom

Turbo history navigation may behave oddly in Safari #810

Open
manuelpuyol opened this issue Dec 6, 2022 · 9 comments
Open

Turbo history navigation may behave oddly in Safari #810

manuelpuyol opened this issue Dec 6, 2022 · 9 comments

Comments

@manuelpuyol
Copy link
Contributor

Some context in WICG/interventions#21 (comment)

Looks like Safari has an heuristic where if a script runs history.pushState after 500ms of the last user interaction, Safari will flag that history state as spammy and it will skip it when clicking the back button.

Here's a quick repro, using Charles Proxy to throttle the network, making requests take more than 500ms:

Screen.Recording.2022-12-06.at.10.48.21.AM.mov

The only code change I made was

--- a/src/tests/fixtures/one.html
+++ b/src/tests/fixtures/one.html
@@ -9,6 +9,7 @@
   </head>
   <body>
     <h1>One</h1>
+    <p><a id="same-origin-link" href="/src/tests/fixtures/two.html">Same-origin link</a></p>
 
     <!--styles ensure that the element will be scrolled to top when navigated to via an anchored link -->
     <a name="named-anchor"></a>

Notes

  • Running history.back() works fine, so creating a test-case is hard since page.goBack() uses the history API instead of actually clicking the back button
  • Maybe this is solvable by pushing the state earlier, but I remember we had issues with that in the past
  • This behavior only happens in Safari. I'm using version 16.0, not sure if this bug is present in other versions
@gingerlime
Copy link

I'm not sure if we're seeing the same thing, but definitely spotted some odd behaviour with back button with Safari on iOS. It's very hard to reproduce and not on all devices either (one person from our team experiences it with iPhone 12, others don't). Back button seems to "jump" unexpectedly to the homepage even after navigating into a few "inner" pages.

Downgrading turbo to v7.1 seems to solve it for this particular case.

Very hand-wavey, I know, but thought I'd chime in as we're totally puzzled by this.

@gingerlime
Copy link

We were able to reproduce it intermittently with throttling the network connection as well on Safari iOS (using Browserstack with network throttling to 3G).

@gingerlime
Copy link

gingerlime commented Dec 12, 2022 •
edited
Loading

I would very much like to git bisect it, to find the culprit, but not sure how to build turbo from source and then apply it in our package.json. We don't use typescript internally, so would probably need to launch some kind of an external build and then use the built artefacts in our codebase. Any help would be appreciated :)

@manuelpuyol
Copy link
Contributor Author

I'm pretty sure the culprit is #601
It changes Turbo to only pushState when the fetch request is done, and if the fetch takes too long we see the bug here.

Unfortunately, that PR is solving another scroll bug, so the fix here isn't simply reverting it :/

@gingerlime
Copy link

Thanks for the extra context @manuelpuyol. I wasn't aware of it. I thought identifying the issue was the first step, but looks like we're already there. If there's anything I can do to help, please let me know.

@joker-777
Copy link

I can reproduce the same problem.

@manuelpuyol
Copy link
Contributor Author

@seanpdoyle I'd love to hear if you have any ideas about this issue. We've had multiple user tickets about it now and I can't find a good solution

@sammovale
Copy link

sammovale commented Mar 17, 2023 •
edited
Loading

Facing the same issue since upgrading turbo to the most latest version. Back button on Safari Mac seems to not work at all

@JoseInTheArena
Copy link

Hi @seanpdoyle! It's been a while since this was reported. Any chance y'all look into this or should we assume this is not a priority that'll be picked up anytime soon?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@joker-777 @gingerlime @sammovale @JoseInTheArena @manuelpuyol