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

Add scroll restoration preference to history traversal #278

Closed
wants to merge 4 commits into from
Closed

Add scroll restoration preference to history traversal #278

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4787a83
Add scroll restoration preference to history traversal
majido Oct 21, 2015
ed411e9
Address review feedback from @foolip
majido Oct 26, 2015
9a825ec
Improve terminology for scroll restoration mode
majido Oct 29, 2015
a165462
Address review feedback from @zcorpan
majido Nov 12, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
112 changes: 89 additions & 23 deletions source
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80353,6 +80353,10 @@ x === this; // true</pre>
This prevents values from being displayed incorrectly after a history traversal when the user had
originally entered the values with an explicit, non-default directionality.</p>

<p>An entry's <dfn>scroll restoration preference</dfn> indicates whether the user agent should
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it a preference in the sense that it may not do what you asked for? I'm not sure what a better name would be, though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does what you asked for which is to determine who is responsible for scroll restoration. If you think preference may not be accurate we can use 'mode' which I have seen used elsewhere in the spec.

Another alternative is to use 'flag' given the binary nature of auto/manual. In that case I think we should use 'manual scroll restoration flag' and refer to it as getting set or unset.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "scroll restoration mode" sounds good.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great. Fixed in ed411e9.

restore the persisted scroll position (if any) when traversing to it.</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should also say above that a "session history entry" consists of this. So explicitly cross-reference this definition from there.



Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A newline too many here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ed411e9.

<p>Entries that consist of <span data-x="state object">state objects</span> share the same
<code>Document</code> as the entry for the page that was active when they were added.</p>

Expand Down Expand Up @@ -80387,8 +80391,11 @@ x === this; // true</pre>
<!--TOPIC:DOM APIs-->
<h4>The <code>History</code> interface</h4>

<pre class="idl">interface <dfn>History</dfn> {
<pre class="idl">enum <dfn>ScrollRestoration</dfn> { "<span data-x="dom-ScrollRestoration-auto">auto</span>", "<span data-x="dom-ScrollRestoration-manual">manual</span>" };

interface <dfn>History</dfn> {
readonly attribute unsigned long <span data-x="dom-history-length">length</span>;
attribute <span data-x="dom-ScrollRestoration">ScrollRestoration</span> <span data-x="dom-history-scroll-restoration">scrollRestoration</span>;
readonly attribute any <span data-x="dom-history-state">state</span>;
void <span data-x="dom-history-go">go</span>(optional long delta = 0);
void <span data-x="dom-history-back">back</span>();
Expand All @@ -80407,6 +80414,14 @@ x === this; // true</pre>

</dd>

<dt><var>window</var> . <code data-x="dom-history">history</code> . <code subdfn data-x="dom-history-scroll-restoration">scrollRestoration</code></dt>

<dd>

<p>The <span>scroll restoration preference</span> of the current entry in the <span>joint session history</span>.</p>

</dd>

<dt><var>window</var> . <code data-x="dom-history">history</code> . <code subdfn data-x="dom-history-state">state</code></dt>

<dd>
Expand Down Expand Up @@ -80510,6 +80525,17 @@ x === this; // true</pre>

<p>The actual entries are not accessible from script.</p>

<p>The <dfn><code data-x="dom-history-scroll-restoration">scrollRestoration</code></dfn> attribute
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here the concept and the attribute need to be more clearly separated. This should say roughly "The scrollRestoration attribute of the History interface must return the scroll restoration preference of the current entry in the session history. On setting, the scroll restoration preference of the current entry in the session history must be set to the new value.

It would be the scroll restoration preference that has a default.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it really be "current entry of the joint session history" as opposed to "current entry in the session history"? When I looked at this I wasn't quite sure if they always mean the same thing, but the definition of replaceState() uses "current entry in the session history" so that seems like the same one. @annevk?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Based on my reading, the joint session history includes the history of all nested frames so current entry of the joint session history may belong to a nested frame.

``history.scrollRestoration` should only expose scroll restoration mode values that belong to the current document. So, similar to replaceState it should use "current entry in the session history". wdyt?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I switched all references for scroll restoration mode from "current entry of the joint session history" to "current entry in the session history" in 9a825ec.

default value is "<dfn><code subdfn data-x="dom-ScrollRestoration-auto">auto</code></dfn>"
which means the user agent is responsible for restoring the scroll position upon navigation. By
setting the attribute to "<dfn><code subdfn data-x="dom-ScrollRestoration-manual">manual</code></dfn>"
the page is taking responsibility for restoring scroll position and the user agent does not
attempt to do so automatically.</p>

<p>Setting the <code data-x="dom-history-scroll-restoration">scrollRestoration</code> updates the
<span>scroll restoration preference</span> of the current entry in joint session history. Any
future entry added to joint session history of the document uses the new value.</p>

<p>The <dfn><code data-x="dom-history-state">state</code></dfn> attribute of the
<code>History</code> interface must return the last value it was set to by the user agent.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than the attribute having an initial value, I think that it's the scroll restoration mode that always has a value, and the attribute merely reflects that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was the intention of this sentence. In particular it refers to Scroll restoration mode which was the subject of the previous sentence.

Perhaps I can rephrase this as:
"If unspecified, the <span>scroll restoration mode</span> value of a new entry is "<code data-x="dom-ScrollRestoration-auto">auto</code>".

Note that where appropriate (e.g., 7.6.1 Navigating across documents) the text asks for specific scroll restoration mode values for any new entry which may be either auto, or the same as the current entry.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to suggested rephrase in 9a825ec. Happy to change it if it is still problematic.

Initially, its value must be null.</p>
Expand Down Expand Up @@ -80715,8 +80741,9 @@ x === this; // true</pre>

<li><p>Add a <span>state object</span> entry to the session history, after the <span>current
entry</span>, with <var>cloned data</var> as the <span>state object</span>, the given
<var>title</var> as the title, and <var>new URL</var> as the <span>URL</span>
of the entry.</p></li>
<var>title</var> as the title, <var>new URL</var> as the <span>URL</span>
of the entry, and current history <span data-x='dom-history-scrollRestoration'>scrollRestoration</span>
value as the <span>scroll restoration preference</span>.</p></li>

<li><p>Update the <span>current entry</span> to be this newly added entry.</p></li>

Expand Down Expand Up @@ -80865,6 +80892,24 @@ State: &lt;OUTPUT NAME=I>1&lt;/OUTPUT> &lt;INPUT VALUE="Increment" TYPE=BUTTON O

</div>

<div class="example">
<p>Most pages want to use the same scroll restoration value for all of their history
entries. To achieve this they should set the <code data-x="dom-history-scroll-restoration">scrollRestoration</code>
attribute as soon as possible (e.g., in first script tag in the document's
<code>&lt;head&gt;</code>) to ensure that any entry added to the history session gets the desired
scroll restoration preference.</p>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/first script tag/the first <code>script</code> element/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 9a825ec

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rewrap this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in a165462.

<pre>&lt;head&gt;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<code>head</code> element

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 9a825ec.

&lt;script&gt;
let canControlScrollRestoration = 'scrollRestoration' in window.history
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't need the window. prefix to history.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ed411e9

if (canControlScrollRestoration)
history.scrollRestoration = 'manual';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this example be reduced to just this line? Setting history.scrollRestoration when it's not supported would just be a no-op. (Unless there's something useful to do depending on canControlScrollRestoration later.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to make the example show case the feature detection. This is mainly why I used a variable 'canControlScrollRestoration' instead of just checking for the condition.

Alternatively we can do which still suggests feature detection but simpler:

if ('scrollRestoration' in history) // detect scroll restoration preference is supported
   history.scrollRestoration = 'manual';

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would one do based on the feature detection? Is it something like this?

if (canControlScrollRestoration) {
  restoreScrollManually();
} else {
  hacksToPreventHavocFromAutomaticScrollRestoration()
}

To a reader that doesn't have a clue how to use the variable, keeping it around seems a bit wasteful.

If a combination of if ('scrollRestoration' in history) history.scrollRestoration = 'manual'; and a later if (history.scrollRestoration === 'manual') doStuff() is what one would expect in real code, that might work as an example.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this for an example:
http://output.jsbin.com/bobizo/3

It shows a simple method for recording and restoring scroll position when scroll restoration mode is 'manual'. I made it so that it is similar to current examples.

&lt;/script&gt;
&lt;/head&gt;
</pre>
</div>




<h4>The <code>Location</code> interface</h4>
Expand Down Expand Up @@ -82006,7 +82051,8 @@ State: &lt;OUTPUT NAME=I>1&lt;/OUTPUT> &lt;INPUT VALUE="Increment" TYPE=BUTTON O
</li>

<li><p>Append a new entry at the end of the <code>History</code> object representing the new
resource and its <code>Document</code> object and related state.</p></li>
resource and its <code>Document</code> object, related state, and default history <span>scroll
restoration preference</span> of "<code>auto</code>".</p></li>

<li><p><span>Traverse the history</span> to the new entry. If the navigation was initiated
with <span>replacement enabled</span>, then the traversal must itself be initiated with
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be: "<code data-x="dom-ScrollRestoration-auto">auto</code>"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a165462.

Expand Down Expand Up @@ -82346,9 +82392,9 @@ State: &lt;OUTPUT NAME=I>1&lt;/OUTPUT> &lt;INPUT VALUE="Increment" TYPE=BUTTON O
<span>top-level browsing context</span>'s <span>document family</span>.</p></li>

<li><p>Append a new entry at the end of the <code>History</code> object representing the new
resource and its <code>Document</code> object and related state. Its <span>URL</span> must be set
to the address to which the user agent was <span data-x="navigate">navigating</span>. The title
must be left unset.</p></li>
resource and its <code>Document</code> object, related state, and current history <span>scroll
restoration preference</span>. Its <span>URL</span> must be set to the address to which the user
agent was <span data-x="navigate">navigating</span>. The title must be left unset.</p></li>

<li><p><span>Traverse the history</span> to the new entry, with the <i>non-blocking events</i> flag
set. This will <span>scroll to the fragment
Expand Down Expand Up @@ -82608,27 +82654,15 @@ State: &lt;OUTPUT NAME=I>1&lt;/OUTPUT> &lt;INPUT VALUE="Increment" TYPE=BUTTON O
entry</var>. Otherwise, let <var>hash changed</var> be false.</p></li>

<li><p>If the traversal was initiated with <dfn>replacement enabled</dfn>, remove the entry
immediately before the <var>specified entry</var> in the session history.</p>
immediately before the <var>specified entry</var> in the session history.</p></li>

<li><p>If the <var>specified entry</var> is not <span>an entry with persisted user
state</span>, but its URL has a fragment identifier, <span>scroll to the fragment
identifier</span>.</p></li>

<li>

<p>If the entry is <span>an entry with persisted user state</span>, the user agent may update
aspects of the document and its rendering, for instance the scroll position or values of form
fields, that it had previously recorded.</p>

<!-- see similar paragraphs in the textarea and input sections -->
<p class="note">This can even include updating the <code data-x="attr-dir">dir</code> attribute
of <code>textarea</code> elements or <code>input</code> elements whose <code
data-x="attr-input-type">type</code> attribute is in either the <span
data-x="attr-input-type-text">Text</span> state or the <span
data-x="attr-input-type-search">Search</span> state, if the persisted state includes the
directionality of user input in such controls.</p>

</li>
<li><p>If the entry is <span>an entry with persisted user state</span>, the user agent may
<span data-x="restore persisted user state">restore persisted user state</span> and update
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the text content is the same, the data-x attribute is not needed. That's true in the <dfn> below as well, where the inner <span> can be omitted entirely. (I think, not tested.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted. Fixed in ed411e9.

aspects of the document and its rendering.</p></li>

<li><p>If the entry is a <span>state object</span> entry, let <var>state</var> be a
<span>structured clone</span> of that state object. Otherwise, let <var>state</var> be
Expand Down Expand Up @@ -82676,6 +82710,38 @@ State: &lt;OUTPUT NAME=I>1&lt;/OUTPUT> &lt;INPUT VALUE="Increment" TYPE=BUTTON O

</div>

<h5> Persisted user state restoration</h5>
<div w-nodev>

<p>When the user agent wants to <dfn><span data-x="restore persisted user state">restore persisted
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove leading space

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a165462.

user state</span></dfn> from a history entry, it updates aspects of the document and its
rendering, for instance the scroll position or values of form fields, that it had previously
recorded. To do so it has to run the following steps immediately:</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/wants to/is to/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a165462.


<ol>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these three lines should just say "must run the following steps immediately:".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. It is a verbose and unnecessary given that it describes the algorithm that follows. Fixed in a165462.

<li><p>If the entry has a <span>scroll restoration preference</span>, let <var>scrollRestoration</var>
be that.</p></li>

<li><p>If <var>scrollRestoration</var> is "<code data-x="dom-ScrollRestoration-auto">manual</code>"
the user agent should not restore the scroll position for the document otherwise it may.</p></li>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm missing an "otherwise" here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a165462.


<li><p>Set <code data-x="dom-history-scroll-restoration">history.scrollRestoration</code> to
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/auto/manual/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a165462.

<var>scrollRestoration</var>.</p></li>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like an incomplete sentence. Can you make it ", otherwise, it may do so."


<li><p>User agent may now update other aspects of the document and its rendering, for instance
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this seems wrong way of doing this. Getting scrollRestoration is defined to "return the scroll restoration mode of the current entry in the session history.", so you should just make sure that the current entry is the right one, and not talk about assigning to history.scrollRestoration.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct. I removed the unnecessary step 3. The current entry is properly updated as part of the #traverse-the-history algorithm. So it is not needed here. Fixed in a165462.

values of form fields, that it had previously recorded.</p></li>

</ol>
</div>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a bit hand-wavy. I don't know right now what it should say instead though.

Note that "may" is a RFC2119 keyword -- I think in a "must" algorithm typically if something is optional, say "Optionally, do X" instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at it a bit more, it seems to me this algorithm should be part of #traverse-the-history-by-a-delta in some way. [Not confident about this though.]

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This essentially reflects the existing language in "7.6.10 History traversal - step 9" which is

"If the entry is an entry with persisted user state, the user agent may update aspects of the document and its rendering, for instance the scroll position or values of form fields, that it had previously recorded."

The main change is that I have tied scroll position restoration with scroll restoration mode. The rest is the same. I will update to use "Optionally, do X".

Not sure what exactly is hand-wavy about it except that it does not specify what other aspect of rendering user-agent may have recorded before. I assumed this is by design but happy to make adjustments.

As for it being part of #traverse-the-history-by-a-delta. It is not necessary,because that uses #traverse-the-history which does include an step that executes the persisted user state restoration algorithm here.


<!-- see similar paragraphs in the textarea and input sections -->
<p class="note">This can even include updating the <code data-x="attr-dir">dir</code> attribute
of <code>textarea</code> elements or <code>input</code> elements whose <code
data-x="attr-input-type">type</code> attribute is in either the <span
data-x="attr-input-type-text">Text</span> state or the <span
data-x="attr-input-type-search">Search</span> state, if the persisted state includes the
directionality of user input in such controls.</p>

<h5>The <code>PopStateEvent</code> interface</h5>

Expand Down