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

scrollRestoration & popstate #1042

Open
laughinghan opened this issue Apr 13, 2016 · 14 comments
Open

scrollRestoration & popstate #1042

laughinghan opened this issue Apr 13, 2016 · 14 comments
Labels
needs implementer interest Moving the issue forward requires implementers to express interest normative change topic: history

Comments

@laughinghan
Copy link

Currently (in Chrome 48 and Firefox 45 at least), if you go to this page and:

  • Click the link,
  • then scroll down a little (so that, say, “N. David Tilton” is at the top of the screen),
  • then hit the Back button,
  • then hit the Fwd button,

...you will end up at “N. David Tilton” again, because the browser remembers the scroll position at the time you hit the Back button.

Now say you want to replicate that behavior but with smooth scrolling, using history.scrollRestoration = 'manual'. The natural way to do this is for your handler for the popstate event to use the session objects:

onpopstate = function(e) {
  smoothScrollTo(e.state.scrollTop);
};

For that to work, your click handler for the link has to save the current scrollTop before smooth-scrolling to the target:

theLink.onclick = function(e) {
  history.replaceState({scrollTop: document.body.scrollTop}, '', location);

  e.preventDefault();
  var target = document.getElementById(this.hash.slice(1));
  history.pushState({scrollTop: target.offsetTop}, '', this.hash);
  smoothScrollTo(target.offsetTop);
};

That makes the Back button work. But to make the Fwd button work like in the example above, your popstate handler also has to save the current scrollTop:

onpopstate = function(e) {
  history.replaceState({scrollTop: document.body.scrollTop}, '', location); // REPLACES THE WRONG STATE
  smoothScrollTo(e.state.scrollTop);
};

(Ending up with this.)
And therein lies the problem: by the time popstate has fired, the state has already been popped, and rather than writing to the history entry from before hitting the Back button like we want to make the Fwd button work, that history entry is first popped off and history.replaceState() writes to the history entry underneath that, after we’ve gone Back.

Now, popstate has been around forever, so we can’t just brazenly make breaking changes to it. But, scrollRestoration isn't even yet supported by all browsers. What if setting history.scrollRestoration = 'manual' caused popstate to fire before the history entry has been popped off? Note that this would also be more consistent with the vast majority of DOM events like keydown/keypress/keyup/mousedown/mouseup/click, all of which fire before the relevant action (and they necessarily must so that you can cancel that action with preventDefault()).


Test case (automated & manual) for the desired browser behavior: http://output.jsbin.com/talumi

@annevk
Copy link
Member

annevk commented Apr 13, 2016

Paging @majido and @smaug---- for opinions.

@annevk annevk added normative change needs implementer interest Moving the issue forward requires implementers to express interest labels Apr 13, 2016
@majido
Copy link
Member

majido commented Apr 13, 2016

As you pointed out, the fundamental problem is that when the popstate event is dispatched we have access only to the current history entry and not the previous one.

For the usecase of storing scroll position what I have tended to do is to maintain a map of scroll position outside of the state object associated with the entry. Here is a simplified example that stores it in memory. In fact saving the state elsewhere is a well know solution. This does not address the fundamental limitation but gets the job done.

Another alternative that we have considered that can help with this is to actually expose the browser recorded scroll position.

What if setting history.scrollRestoration = 'manual' caused popstate to fire before the history entry has been popped off?

It feels wrong to me to have a different timing for popstate based on the scroll restoration mode of the entry. It is is also hard to reason about. This may fix this particular problem but does not address the fundamental limitation. I think what we need is a beforepopstate event that fires synchronously before the state is popped. Similar to beforeunload, the event is not preventable but gives a page author a chance to update the state or DOM before navigation.

I have seen similar 'onLeave' hooks provided by routing frameworks used on the web. So there is some indication of a need here. I am not sure if this is evidence enough to justify a new event though.

@Yay295
Copy link
Contributor

Yay295 commented Apr 13, 2016

You are not supposed to be able to see the history state of any other page. That would be a security/privacy issue. I agree with majido that a beforepopstate event would be a better solution.

@smaug----
Copy link

Could popstate event contain more information? like oldState or so, would that help here?
I'd rather not add more events. (the whole scrollRestoration should have perhaps been events based but too late to modify that API)

@laughinghan
Copy link
Author

You are not supposed to be able to see the history state of any other page. That would be a security/privacy issue.

I think this is a misunderstanding—no one's proposed changes involve seeing the history state of another page. My proposal was if you did .replaceState(state0) then .pushState(state1), and then the visitor hit the Back button, when popstate fires, history.state is still state1 rather than being state0 already. All this happens on the same page.

@laughinghan
Copy link
Author

@majido:

For the usecase of storing scroll position what I have tended to do is to maintain a map of scroll position outside of the state object associated with the entry.

Makes sense, I had an inkling of this kind of workaround like this last night but was too tired to try it. We’re all in agreement that there is a problem here that currently has to be worked-around, though, right?

It feels wrong to me to have a different timing for popstate based on the scroll restoration mode of the entry. It is is also hard to reason about. This may fix this particular problem but does not address the fundamental limitation. [...] I am not sure if [we can] justify a new event though.

I think this can address the fundamental limitation, the problem was my use-case-specific choice of tying it to scroll restoration mode. I too am not a fan of piling onto our ever-growing list of DOM events.

I think it's worth noting how odd the current default behavior is (history.scrollRestoration === 'auto' by spec): currently, when the popstate event handler is run after having hit the Back (or Fwd) button, the history state has been popped, but the scroll position hasn't been restored yet—the browser is halfway through the process of going back (or forwards) in history. This is reportedly consistent across Firefox, Chrome, and Edge.

Also, there used to be beforecut, beforecopy, and beforepaste events—we killed those in favor of plain old cut/copy/paste that, like keydown/mousedown/etc, all happen before the action. According to Dottoro, at least, beforeunload seems to be the only well-supported event prefixed with before.

With all that in mind, I propose a history.popstateBehavior, with 3 values:

  • 'legacy': current scrollRestoration === 'auto' behavior
  • 'autoScrollRestore': a sensible modification to the current scrollRestoration === 'auto' behavior where the popstate event handler is run before both the history state has been popped and the scroll position has been auto-restored
  • 'manualScrollRestore': no auto-restoration of scroll position; popstate event handler still runs before the history state has been popped.

I'd also be happy if this could be tied to something like the next DOCTYPE, but I assume that's out of the question?

@smaug----
Copy link

when the popstate event handler is run after having hit the Back (or Fwd) button, the history state has been popped, but the scroll position hasn't been restored yet—the browser is halfway through the process of going back (or forwards) in history.

IIRC I filed a spec bug to make spec to follow implementations.

@laughinghan
Copy link
Author

@smaug----:

Could popstate event contain more information? like oldState or so, would that help here?

That wouldn't help with the example I gave, where the API consumer expects to be able to save information to the current history state with history.replaceState() (unless event.oldState is the live object that's stored in session history? Currently the state objects appear to be serialized when they're recorded: http://jsbin.com/qinocab/edit?js,console,output ). Edit: Note that the suggested workarounds all involve keeping around a parallel stack of session objects and and waiting to pop them off until after saving stuff there, because that's the contract that API consumers need.

Also, MDN on .pushState() has a note about serialization:
Note: In Gecko 2.0 (Firefox 4 / Thunderbird 3.3 / SeaMonkey 2.1) through Gecko 5.0 (Firefox 5.0 / Thunderbird 5.0 / SeaMonkey 2.2), the passed object is serialized using JSON. Starting in Gecko 6.0 (Firefox 6.0 / Thunderbird 6.0 / SeaMonkey 2.3), the object is serialized using the structured clone algorithm. This allows a wider variety of objects to be safely passed.

(the whole scrollRestoration should have perhaps been events based but too late to modify that API)

It's only implemented in latest Firefox and Chrome but not in Safari or IE/Edge, I hope it's not too late? I'm proposing a change to it here, after all.

IIRC I filed a spec bug to make spec to follow implementations.

Mind if I ask for a link?

@smaug----
Copy link

It's only implemented in latest Firefox and Chrome but not in Safari or IE/Edge, I hope it's not too late? I'm proposing a change to it here, after all.

a change is fine, but re-designing the scrollRestoration to be fully events based, may not be.
Though, it would have been nice if the API would have let one to call preventDefault() on the popstate to prevent scrolling. And perhaps history.state should have been restored right after popstate_ dispatch. Then one would have access to the old state, and popstate would have pointer to the new one.

But, ok, perhaps at this point the safest, though a bit ugly approach is to explicitly just dispatch a simple event 'beforepopstate' somewhere before step 12 in https://html.spec.whatwg.org/multipage/browsers.html#history-traversal

@laughinghan
Copy link
Author

And perhaps history.state should have been restored right after popstate_ dispatch.

Exactly! Exactly the behavior I'm proposing for values of history.popstateBehavior other than the default of 'legacy'.

@majido
Copy link
Member

majido commented Apr 18, 2016

(the whole scrollRestoration should have perhaps been events based but too late to modify that API)

It's only implemented in latest Firefox and Chrome but not in Safari or IE/Edge, I hope it's not too late? I'm proposing a change to it here, after all.

We spent a lot of time trying to make an event-based API work. I even wrote an initial draft spec but we ran into a fundamental issue on having this event-based API work for cross-doc navigations where scroll position should be restored content scripts are loaded.

The reason an event works in this situation is because the problem you are trying to fix is only manifest in same-doc navigation case.

when the popstate event handler is run after having hit the Back (or Fwd) button, the history state has been popped, but the scroll position hasn't been restored yet—the browser is halfway through the process of going back (or forwards) in history.

IIRC I filed a spec bug to make spec to follow implementations.

I think you are referring to #39 which proposes that we match spec with actual implementations:
Spec'ed behaviour :
a. Pop state (update latest entry and history.state)
b. Restore scroll position and other UI state
c. Run onpopstate handler

Actual implementations:
a. Pop state
c. Run onpopstate handler
b. Restore scroll position and other UI state

If I understand @laughinghan proposal correctly, history.popstateBehavior = autoScrollRestore will allow to request the order to be c,b,a. I have a few objection to this design:

1- The event dispatch timing may now be before/after the actual event and is determined based on a dynamic value. I cannot think of any other event that has this property. It means that to reliably use this event the developer has to check this value and behave accordingly. Imagine if a third-party library changes this value!

2- This fixes the problem of accessing the previous state before transition but now you don't have access to upcoming state. The best solution I can think of is to use a setTimeout which adds a task at the end of queue. I suspect this should give you some guarantee that it is executed after current tasks (including updating the state) are executed. It is workable but not very appealing

Also the naming is odd. What you are describing is should require two state history.popstateDispatchTiming={before || after} with 'after' being the default legacy value. Tying it to scroll restoration seems unnecessary and confusing.

Also, there used to be beforecut, beforecopy, and beforepaste events—we killed those in favor of > plain old cut/copy/paste that, like keydown/mousedown/etc, all happen before the action.

Yes, I think dispatching event before action makes sense specially when event is cancelable. But that ship has sailed long ago for popstate. I suspect changing the order is going to be web-incompatible.

According to Dottoro, at least, beforeunload seems to be the only well-supported event prefixed with before.

Well, that is a good precedent specially when it comes to navigation related events. There is also befoeinput & input pair of events. One is dispatched before composition events get applied and one is dispatched after they do.

I agree that adding a new event shouldn't be taken lightly but I don't see how the proposed solution is any better. Any other ideas?

@Yay295
Copy link
Contributor

Yay295 commented Apr 18, 2016

What if the scroll position was kept in the history state automatically as a read-only variable, even when history.scrollRestoration = 'manual', so the browser would make sure the right value was saved and you could just get it from the history state? The downside being that this is a rather specific solution, and there may be other similar issues that this would not fix.

@majido
Copy link
Member

majido commented Apr 19, 2016

What if the scroll position was kept in the history state automatically as a read-only variable, even when history.scrollRestoration = 'manual', so the browser would make sure the right value was saved and you could just get it from the history state?

See my earlier comment on this.

This is a good solution for the specific usecase of scroll position. I am inclined to think that there is more long-term value for web platform, if we spend the effort instead and attempt to fix the more fundamental issue here i.e., accessing the state before and after a history navigation. That actually makes history.state a viable option to store page state.

@laughinghan
Copy link
Author

@majido: Thank you for your detailed responses :)

the crazy popstateBehavior idea

If I understand @laughinghan proposal correctly, history.popstateBehavior = autoScrollRestore will allow to request the order to be c,b,a.

Exactly!

1- [...] It means that to reliably use this event the developer has to check this value and behave accordingly. Imagine if a third-party library changes this value!

Doesn't history.scrollRestoration already have exactly this problem? But yes it sucks.

I do think it's worth noting that it sucks in a very limited way. A popstate handler would only be affected if it does one of:

  • calls history.replaceState()
  • reads history.state instead of .state on the event object
  • reads location

Note that the "simplified example" that you provided would be unaffected (but superfluous). History.js, your "well known solution", does read location, but could easily account for this (even easier if the popstate event object had a .location, which it should).

2- This fixes the problem of accessing the previous state before transition but now you don't have access to upcoming state.

I should have been more clear—the popstate handler is passed an event object with a .state property which is the new, upcoming history state, and I wouldn't want to change that. This would be consistent with e.g. the keydown/keypress and cut/copy/paste events, whose .which/.code/.keyIdentifier and .clipboardData have information on the upcoming change to the textarea, but whose handlers are called before the change actually happens.

Also the naming is odd. [...] Tying it to scroll restoration seems unnecessary and confusing.

True; but conversely, it's also unnecessary and confusing to allow manual scroll restoration when history.state is not a viable option to store page state.

I suspect changing the order is going to be web-incompatible.

I suspect you’re right, but I feel like I have to at least try to present the strongest possible case for somehow "fixing" popstate. If it’s not enough, well, at least we thought it through.

naming the new event

Well, that is a good precedent specially when it comes to navigation related events.

Isn't beforeunload a super weird event with special behavior regarding confirm() dialogs and stuff?

beforeinput

beforeinput hasn't been implemented in any browser (gecko, chromium, webkit doesn't even have a ticket) or have an MDN page, seems to be considered still in flux. Aren't there already compositionstart/compositionupdate events? Like, in production?

Can we at least not call it beforepopstate, and instead call it like, statechange (like hashchange) or historychange or historytraversal or something, and officially deprecate popstate?

other ideas

Any other ideas?

Sure.

@smaug---- already suggested that the popstate event object could have a .oldState or .prevState, which would work if we made sure it wasn't a copy but was a live reference to the state object that, when updated, would update the history entry. That would be inconsistent with history.state being read-only though, and go against the grain of current implementations with their serialization and whatnot.

There could be a global history.replacePrevState() function, or the popstate event object could have a .replacePrevState() or .saveState() or .persistState() method, similar to language in the spec:

When a user agent is required to traverse the history to a specified entry, [...]

    1. If appropriate, update the current entry in the browsing context's Document object's History object to reflect any state that the user agent wishes to persist. [For example, some user agents might want to persist the scroll position, or the values of form controls.]

Well, authors might want to persist state too!

Is that better or worse than a historytraversal event that happens at a sensible time consistent with all our other events (and deprecating popstate)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs implementer interest Moving the issue forward requires implementers to express interest normative change topic: history
6 participants