- From: Majid Valipour <majidvp@chromium.org>
- Date: Wed, 29 Apr 2015 21:22:34 +0000
- To: Jonas Sicking <jonas@sicking.cc>, Olli Pettay <Olli@pettay.fi>
- Cc: WHATWG <whatwg@whatwg.org>, Majid Valipour <majidvp@chromium.org>, Ian Hickson <ian@hixie.ch>
On Tue, Apr 28, 2015 at 3:24 PM Jonas Sicking <jonas@sicking.cc> wrote: > On Tue, Apr 28, 2015 at 9:16 AM, Majid Valipour <majidvp@google.com> > wrote: > > On Mon, Apr 27, 2015 at 9:54 PM Jonas Sicking <jonas@sicking.cc> wrote: > >> > >> Jumping in at the end here. > >> > >> As I've said before, I like the general idea of giving pages more > >> control over scroll restoration, but I don't think we should tie this > >> to pushState()/replaceState()/onscroll. > >> > >> My proposal is instead that we add an API like > >> > >> history.restoreScroll = boolean; > > > > > > Interesting. I believe Simon has also proposed a similar API for which > you > > can find my original objections here. > > > > One of my objections was that any proposed API should give developers > > control of scroll restoration per individual history entry. Initially I > > assumed a single boolean flag cannot provide that control but now I > believe > > what you are proposing here can in fact provide per-entry control. More > on > > this later. > > > > My other objection was that because scroll restoration behaviour is > > ultimately tied to a specific history entry, the API to control it should > > reflect this underlying fact. IMO a flag on history object hides this > fact > > while original proposal does not. > > > > I understand that for pages that do not create any state other than their > > default initial state (e.g., infinite scrollers) setting > > history.restoreScroll is simpler than using > > history.replaceState({restorScroll: false}). But I think this additional > > complexity is not prohibitive and can be justified when the upside is an > API > > that better explains the underlying behaviour and is simpler for pages > that > > do create multiple states using history.pushState. > > I agree that the difference between > > history.restoreScroll = false; > and > history.replaceState({restoreScroll: false}); > > is not that big and mainly comes down to taste. I like the former more. > > I agree it comes down to taste. In fact there are three API proposed: 1. Using a fourth optional dictionary parameter on {push|replace}State: backward compatible and consistent. 2. New history.push, history.replace methods accepting a bool flag: cleaner API that allows us to clean minor old warts such as making 'url' required and 'title' optional (or even remove it). 3. Options dictionary on history object that contain boolean flags (history.options.restoreScroll): Simplified usage for pages that don't use pushState API. There has been support for all three in particular Spec editor favoring #1. Soon I am hoping to get feedback for #1 by implementing it in Blink behind a runtime flag. Note that all three options are extensible enough to allow exposing recorded scroll position in future. >> This property would default to true. Whenever pushState() is called, > >> or the user navigates away from the current page, for example by > >> clicking a <a href=...> the value of history.restoreScroll is copied > >> into the session-history-entry data that the browser keeps internally. > >> As soon as the new session-history-entry is created, restoreScroll is > >> set to true again. > > > > So history.restoreScroll is copied into current history entry anytime a > new > > history entry is created (or replaced). If my reading is correct then > this > > makes sense and it can give per history entry control. The value of > > history.restoreScroll is reset to its default when a new document is > loaded. > > > > Consider this case where page A uses pushState once and then navigates to > > page B. This creates three history entries: 2 for A and one for B. > > > > A --> Ai (*) ==> B > > where: > > --> pushState > > ==> navigation to new page > > (*) history.restoreScroll is set to 'false' > > > > This is what I think should happen: > > on A load: history.restoreScroll is reset to default value: TRUE. > > history entry is created for A. > > > > on -->: current entry (A) is updated with current restoreScroll: > TRUE > > history entry is created for Ai > > > > on (*): history.restoreScroll is set to 'FALSE'. > > > > on ==>: current entry (Ai) is updated with current restoreScroll: > > FALSE > > > > on B load: history.restoreScroll is reset to default value: TRUE > > history entry for B is created. > > > > A gets TRUE > > Ai gets FALSE > > B gets TRUE > > Agreed. > > >> Additionally we could enable passing a boolean to pushState(), and > >> this value would be used as the new initial value for restoreScroll. > >> So something like pushState({ url: myURL, restoreScroll: false });. > >> This would simply be syntax sugar for |pushState("", "", myURL); > >> history.restoreScroll = false;|. > > > > It is a nice syntax sugar but I don't feel it adds enough value to > justify > > doing this at browser level. It can be done in Javascript by > > framework/poly-fill authors. > > I'm fine either way. > >> I think that's all that's needed. > > > > Yes. Both APIs are equally expressive and one may be built on top of the > > other. > > > > I suggest two minor changes though: > > > > 1. We specifically proposed using a dictionary for options with scroll > > restoration being one of its parameters. This allows us to add additional > > history entry control parameters in the future. For example we can expose > > zoom restoration similarly if there is enough demand. > > 2. As suggested earlier in the thread it is better to use a name that > > suggests more strongly that the author is expected to restore the > position > > themselves. > > > > So: > > > > history.options = { > > willRestoreScroll = false > > } > > I'm not strongly opinionated on naming. Are there other APIs in the > platform which are named similarly? > Not as far as I can tell based on a quick grep on blink IDL files. > >> In order to make it easier for pages we could also expose a > >> history.restoredScrollPosition which is a readonly property which the > >> UA writes to any time it restores a session history entry, which is > >> the scroll position that it would have restored scrolling to if the > >> page hadn't disabled scroll restoration for the given session history > >> entry. > >> > >> This is isn't strictly needed though since the page can simply use > >> sessionStorage and update the scroll position in a onscroll handler. > > > > > > Storing position is sessionStorage works well. It has the advantage that > the > > the same logic can handle scroll restoration for both history and > > non-history navigations (e.g., same page being loaded as a result of > > clicking back button or a link). > > But what happens if the user navigates > > A.html ===> B.html ===> A.html ===> C.html > > How do you ensure that the second A.html page doesn't overwrite the > saved scroll position of the first A.html session history entry? I.e. > how do you ensure that you use different key names in the > sessionStorage? > I believe this does requires A to attach user state to its history entries in order to differentiate between them. The state could contains either the scroll position (bypassing sessionStorage altogether) or a unique key for session storage. I agree that exposing ''restoredScrollPosition" helps eliminate the boilerplate that is required to do this. I think this is useful to have. > > Providing scroll position is nice to have but not required. I think it > > requires additional spec work to exactly define the behaviour of scroll > > restoration across different browsers. Right now different vendors have > > different behaviour when it comes to restoring scroll position for > iframes, > > and inner scrollables etc. So it is not really clear how exactly this > > 'scroll position' should look like. > > > > Getting this boolean flag should not be blocked on providing scroll > > position. > > Agreed. > > / Jonas >
Received on Wednesday, 29 April 2015 21:23:03 UTC