Putting this in Core::DOM since I'm playing with history.pushState. I have a page that's using pushState: http://z.jbalogh.me/history.html. If I scroll down the page and then hit back I get taken to the top of the page. There doesn't seem to be a way to prevent this, even when catching the popState event.

Scroll position is a function of the history entry, no? In particular, if you pushState a url with an anchor ref we should scroll to it, I should think. That said, the fact that going forward again doesn't scroll back down seems like a bug.

(In reply to comment #1) > In particular, if you pushState a url with an anchor ref we should scroll to > it, I should think. I don't believe this is implied by the spec as it exists now, although we could of course change it if we wanted to. http://www.whatwg.org/specs/web-apps/current-work/multipage/history.html#dom-history-pushstate > That said, the fact that going forward again doesn't scroll back down seems > like a bug. I agree.

You can also see this on the tabs under "Browse Add-ons" on https://preview.addons.mozilla.org/z/en-US/firefox/. We try to use push/popState when you switch tabs, but going back jumps to the top of the page.

Again, scroll position is session-history-entry-specific. If you pushstate, then scroll, then pop, I would expect us to scroll back to where we were when you pushed...

This should block, I think. I have a fix that seems to work; just writing a test now.

Here's a screencast showing the problem going backwards: http://screencast.com/t/YTU5ZDc1.

(In reply to comment #6) > Here's a screencast showing the problem going backwards: > http://screencast.com/t/YTU5ZDc1. I asked stephen to make one of his videos to show my original issue: he's scrolled halfway down the page clicking on the tabs (which is when we pushState). When he hits Back it jumps to the top of the page.

Yes; the point is that this is expected behavior...

(In reply to comment #8) > Yes; the point is that this is expected behavior... It should scroll you back to where you were before the pushState. I think his point is that it scrolls you back to the top regardless. At least, that's what I see in the video.

(In reply to comment #9) > (In reply to comment #8) > > Yes; the point is that this is expected behavior... > > It should scroll you back to where you were before the pushState. I think his > point is that it scrolls you back to the top regardless. At least, that's what > I see in the video. Yes. If we're updating scroll position for going forward, it seems like the same should be done for going back.

There are actually two issues here. One is in the spec: Since a pushState doesn't cause a history traversal, you never save the scroll position. Hixie agrees this is a spec bug; we'll get it fixed. The other is that going back and forward between history entries which are related by a pushState doesn't respect the saved scroll state for either of them.

This patch fixes both issues from my previous comment.

> It should scroll you back to where you were before the pushState. Yes, absolutely.

This seems like bad behavior in a new FF4 feature.

Filed a spec bug: http://www.w3.org/Bugs/Public/show_bug.cgi?id=10437

Hmm. So normal navigations didn't use to save the scroll pos in the history entry? Where did it get saved then? And can we remove that other saving location now, perhaps as a followup?

Comment on attachment 469185 [details] [diff] [review] Patch v1 r=bzbarsky as long as you test scroll pos restoration on normal page navigations well.

(In reply to comment #16) > Hmm. So normal navigations didn't use to save the scroll pos in the history > entry? Where did it get saved then? Is this owned by the presentation shell? See nsDocShell::GetCurScrollPos() and nsDocShell::GetRootScrollFrame().

This patch attempts to test scroll pos restoration on a normal load. It fails, sadly, and I'm not sure why. I've tried doing what I think are the same steps outside mochitest, and that works as i expect. I'll look at it in the morning, I guess.

> Is this owned by the presentation shell? Could be, sure.

All right; I managed to get it to work by spinning the event loop more. The behavior is pretty strange if you don't spin the event loop enough, but in my experience that's true in general for history stuff, so it doesn't concern me too much.

(In reply to comment #3) > You can also see this on the tabs under "Browse Add-ons" on > https://preview.addons.mozilla.org/z/en-US/firefox/. We try to use > push/popState when you switch tabs, but going back jumps to the top of the > page. Jeff, that page isn't currently using pushState, is it? I get the same wonky scroll behavior there as I did before this fix, although http://z.jbalogh.me/history.html works fine for me.

Yes, we pushState whenever you click on a tab. The code is at [1]. We pushState as soon as you hit the page and then again on each click. [1]: http://github.com/jbalogh/zamboni/blob/master/media/js/zamboni/homepage.js

Oh, I see what's going on. When the page loads, it pushStates to ?browse=featured. So then when I go back, I jump up to the top of the page. I'd recommend using replaceState for that initial load. Does the scrolling look good to you in the latest nightly?

(In reply to comment #25) > I'd recommend using replaceState for that initial load. Does the scrolling > look good to you in the latest nightly? Looks great to me, thanks!

(Spec has been updated accordingly.)