Automatic browser scrolling corrupts router scroll history

[agundermann]

As far as I can tell the implementation of the scrolling behaviors is flawed. But first of all, some steps to reproduce:

1. Run and open up the "master-details" example (Chrome or Firefox)
2. Scroll down a bit
3. Click on Ryan (or any other link)
4. Click browser back button
5. Click browser forward button

After 5), the expected behavior would be to scroll to the top again. Instead, the router will scroll to the position set in 2).

Some console.logs to understand what's going on (Chrome):

1) Initial render "/"
Router.run, current scroll: {"x":0,"y":0}

2) Manual scrolling "/"
scroll event to: {"x":0,"y":100} 

3) Clicking link from "/" to "/contacts/ryan"
Scroll History is now: {"/":{"x":0,"y":100}}
Router.run, current scroll: {"x":0,"y":100}
ImitateBrowserBehavior: scrolling to 0, 0
scroll event to: {"x":0,"y":0}

4) Back button from "/contacts/ryan" to "/"
// this is where things start to fall apart..
// expected: {"/":{"x":0,"y":100},"/contact/ryan":{"x":0,"y":0}} 
Scroll History is now: {"/":{"x":0,"y":100},"/contact/ryan":{"x":0,"y":100}}
Router.run, current scroll: {"x":0,"y":100}
ImitateBrowserBehavior: scrolling to 0, 100
scroll event to: {"x":0,"y":100}

5) Forward button from "/" to "/contacts/ryan"
Scroll History is now: {"/":{"x":0,"y":0},"/contact/ryan":{"x":0,"y":100}}
Router.run, current scroll: {"x":0,"y":0}
ImitateBrowserBehavior: scrolling to 0, 100
scroll event to: {"x":0,"y":100}

The problem is that browsers (at least Firefox and Chrome) will restore their own recorded scroll positions before triggering any events (I believe this is the same for hashchange and popstate), causing the router to store these newly set positions for the previous path. (Things are even worse if you render asynchronously..)

Some thoughts about solving this problem

Since there's no "beforepopstate", the first workaround that occurred to me was to listen to the scroll event and store from there (instead of on dispatch). Unfortunately, browsers will trigger the scroll event themselves after back button navigation and it seems there is no reasonable way to distinguish them from user-initiated scroll events.

So I thought about simply ignoring scroll events for a short time after popstate. Problem with that is that Firefox triggers the scroll event even before popstate. Chrome, for comparison, triggers it afterwards, but restores the scroll position beforehand, and it seems to back off if the scroll position was manually set before popstate finishes (hence there's only one scroll event after back/foward in the above log).

Those problems made me think that maybe the router should not do anything on popstate events, and people should make sure to fully restore content synchronously (at least after popstate). Sadly, Firefox doesn't play along: since it restores the scroll position before popstate, and apparently doesn't do anything afterwards, it will clamp scroll positions to the previous/current page size, i.e. when navigating from a small to a taller page, it will not scroll beyond the small page's height.

Is this really such a mess? Hopefully I'm missing something here.

[gaearon]

Interesting! I can't repro this with HistoryLocation but it does seem to happen with HashLocation.

[gaearon]

This is how I tried to fix this for HistoryLocation: #524

I wonder how hashchange is different from popstate in this case..

[gaearon]

I suggest we take pragmatic approach and do the following:

• For PUSH actions, we record scroll position since we can do this reliably (browsers won't mess with it)
• For POP actions, don't even try because we can't really do that for the reasons you mentioned

This means we will only restore scroll on Back, but will always scroll to top on Forward. I think it's a sensible compromise, and I think it doesn't make UX worse in any meaningful way.

This would involve changing action !== LocationActions.REPLACE to action === LocationActions.PUSH on this line.

[gaearon]

#708

[gaearon]

Thank you for bringing this up.
Can you verify that applying changes from #708 fixes the problem in all cases?

Is this really such a mess?

Yes.

[agundermann]

Interesting! I can't repro this with HistoryLocation but it does seem to happen with HashLocation

Oh, you're right. Apparently, popstate and hashchange are more different than I thought.

So I think the situation is as follows:

Browser & Location	position change	scroll event
Chrome - HashLocation	before hashchange	after hashchange
Chrome - HistoryLocation	after popstate	after popstate
Firefox - HashLocation	before hashchange	before hashchange
Firefox - HistoryLocation	before popstate	after popstate

😢

I suggest we take pragmatic approach and do the following..

Well, it does fix the case I described. I'm not convinced it's a good solution though, since it will not record any position changes that aren't followed by a push.

Take this case for example:

1. Run and open up the "master-details" example (Chrome or Firefox)
2. Click on Ryan (or any other link)
3. Scroll down a bit
4. Click browser back button
5. Click browser forward button

(Same as original, but now scrolling on second page)
Before your change, this was failing for the same reasons as the original one. Now it fails because the position change isn't being recorded to begin with.

[gaearon]

Before your change, this was failing for the same reasons as the original one. Now it fails because the position change isn't being recorded to begin with.

What do you define as “failing” in this case? What I see is, after clicking Forward, scroll position is reset to top. I don't think this is a huge issue and it's an intentional tradeoff we can make.

The biggest bummer would be losing Back scroll position. IMO we can afford always scrolling to top on Forward because it doesn't degrade usability as much, but at least gives us consistent behavior without clearly bad behavior (such as scrolling one page to a saved position of another page).

[agundermann]

It can also be inconsistent with "back" when scrolling on a page visited before, clicking forward, and then back again. I can't think of a real-world scenario where I would this though, so perhaps you're right about this being the solution to go with.

[gaearon]

It can also be inconsistent with "back" when scrolling on a page visited before, clicking forward, and then back again.

Can you put this into reproducible steps? I'm trying to imagine this but I can't see it going wrong yet.
Even if it's not common, I'd still rather know that there's an issue..

[agundermann]

1. Run and open up the "master-details" example (Chrome or Firefox)
2. Click on Ryan (or any other link)
3. Click browser back button
4. Scroll down
5. Click browser forward button
6. Click browser back button

Result: Scrolls to top (or whatever scroll position on 2))
Expected: Position after 4)

Basically it goes wrong whenever you scroll on a page, leave it without push (without clicking on a link) and then come back to it again (via browser back, forward or whatever).

[gaearon]

I see now.. This isn't a big problem IMO but it isn't very nice either.
Let's hear what others have to say about it!

[ryanflorence]

@gaearon you are the scroll behavior master, I defer any decision to you, unless @mjackson has the time to get caught up on this thread, cause I know I don't right now..

[gaearon]

OK, I'll give this and #690 more thought on Monday and try to come up with something to fix both.

[gaearon]

#690 is definitely unrelated.

@mjackson @rpflorence

TLDR: We can only reliably record scroll position (so we can later restore it) after user-initiated transition like transitionTo or Link click. We can't reliably record scroll position after history is popped (either due to popstate or hashchange). There is just no way to do that across browsers, and now we get it wrong with some combinations of locations and browsers (e.g. Chrome+HashLocation, Firefox+HashLocation, Firefox+HistoryLocation).

Therefore I suggest we merge #708.

This gives us consistent "scroll to top" on Forward instead of buggy "try to restore something I haven't actually memorized correctly" behavior we have now.

[gaearon]

Thank you for raising this again.

[majido]

@gaearon I am working on a small new addition to History API (shipping in chrome 46) which allows web apps to opt out of browser's default scroll restoration. I think it can help resolve some of the scroll restoration issues you were dealing with. I appreciate any feedback you may have given your experience working around these issues.

[VonD]

@majido i was still having some issues with this, and history.scrollRestoration = 'manual' totally fixed them for me. Many thanks!

[majido]

@VonD that is great to hear. Just note that at the moment history.scrollRestoration is Chrome only which means that workarounds are necessary for other browsers.