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 3 commits
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 92 additions & 23 deletions source
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80353,6 +80353,9 @@ 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 mode</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.


<p>Entries that consist of <span data-x="state object">state objects</span> share the same
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.

<code>Document</code> as the entry for the page that was active when they were added.</p>

Expand Down Expand Up @@ -80387,8 +80390,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 +80413,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 mode</span> of the current entry in the <span>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 +80524,22 @@ 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
of the History interface, on getting, must return the <span>scroll restoration mode</span> of the
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.

current entry in the <span>session history</span>. On setting, the <span>scroll restoration mode
</span> of the current entry in the <span>session history</span> must be set to the new
value. <span>Scroll restoration mode</span> may be one of the following:</p>
<dl>
<dt>"<dfn><code subdfn data-x="dom-ScrollRestoration-auto">auto</code></dfn>"</dt>
<dd>The user agent is responsible for restoring the scroll position upon navigation.</dd>
<dt>"<dfn><code subdfn data-x="dom-ScrollRestoration-manual">manual</code></dfn>"</dt>
<dd>The page is responsible for restoring the scroll position and the user agent does not attempt
to do so automatically</dd>
</dl>

<p>If unspecified, the <span>scroll restoration mode</span> of a new entry must be set to
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.

"<code data-x="dom-ScrollRestoration-auto">auto</code>".</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.
Initially, its value must be null.</p>
Expand Down Expand Up @@ -80715,8 +80745,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 the <span>scroll restoration mode</span> of the current entry in the
<span>session history</span> as the scroll restoration mode.</p></li>

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

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

</div>

<div class="example">
<p>Most applications want to use the same <span>scroll restoration mode</span> value for all of
their history entries. To achieve this they should set the <code data-x="dom-history-scroll-restoration">scrollRestoration</code>
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.

attribute as soon as possible (e.g., in the first <code>script</code> element in the document's
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.

<code>head</code> element) to ensure that any entry added to the history session gets the desired
scroll restoration mode.</p>
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


<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.

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;
if ('scrollRestoration' in history)
history.scrollRestoration = 'manual';
&lt;/script&gt;
&lt;/head&gt;
</pre>
</div>




<h4>The <code>Location</code> interface</h4>
Expand Down Expand Up @@ -82006,7 +82054,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 the default <span>scroll
restoration mode</span> of "<code>auto</code>".</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.

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.


<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
Expand Down Expand Up @@ -82346,9 +82395,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 +82657,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>
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.


<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>restore persisted user state</span> and update
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 +82713,38 @@ State: &lt;OUTPUT NAME=I>1&lt;/OUTPUT> &lt;INPUT VALUE="Increment" TYPE=BUTTON O

</div>

<h5> Persisted user state restoration</h5>
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.

<div w-nodev>

<p>When the user agent wants to <dfn>restore persisted user state</dfn> from a history entry, it
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.

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.

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.


<ol>

<li><p>If the entry has a <span>scroll restoration mode</span>, let <var>scrollRestoration</var>
be that.</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>If <var>scrollRestoration</var> is "<code data-x="dom-ScrollRestoration-auto">manual</code>"
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.

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.

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


<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.

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.

<var>scrollRestoration</var>.</p></li>

<li><p>User agent may now update other aspects of the document and its rendering, for instance
values of form fields, that it had previously recorded.</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 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.


</ol>
</div>

<!-- 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