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

document.open() can make a document's URL go out of sync with its history entry #3885

Closed
TimothyGu opened this issue Aug 7, 2018 · 4 comments · Fixed by #3946
Closed

document.open() can make a document's URL go out of sync with its history entry #3885

TimothyGu opened this issue Aug 7, 2018 · 4 comments · Fixed by #3946

Comments

@TimothyGu
Copy link
Member

Currently, the document open steps has

Change document's URL to the URL of the responsible document specified by the entry settings object.

However, it doesn't change the URL of its history entry.

@domenic and @rniwa agree that this is a pretty bad behavior. In particular, it breaks fragment navigation as currently spec'd, as well as some implementor expectation [1].

More visibly, this results in different behaviors when reloading a page that has been document.open()ed from another document:

  • Edge and Firefox are currently immune to this problem as they implement the reload override flag, proposed to be removed in Tracking issue for document.open() overhaul #3818
  • Chrome when reloading uses the URL of the current history entry. This will cause the document.open()ed document to go back to its original URL [1].
  • Safari when reloading uses the URL of the document (which is what location.reload() is spec'd to do). This will cause the document.open()ed document to go to the URL of outer document that called document.open().

With the constraint that we want to retain the URL-setting behavior of document.open(), we see two paths of going forward:

  • To preserve Safari reloading behavior:
    In document.open(): set the current history entry's URL to the new URL as well. This will make a reload go to that new URL.
  • To preserve Chrome reloading behavior:
    In document.open(): set the current history entry's URL to the new URL as well; save the original URL in the Document.
    In location.reload(): use that original URL for reloading, if it's present.

We prefer the first as it is significantly simpler to spec, and perhaps also to implement. Opinions?

/cc @smaug---- @bzbarsky

[1]: Chrome used to follow Safari's behavior, but during an internal refactoring location.reload() is made to use the history entry's URL rather than the document's URL under the assumption that they are the same. See chromium/chromium@cf62ba4.

@smaug----
Copy link
Collaborator

What do you mean with "outer document that called document.open()"? Does Safari use callee's document's URI?

@TimothyGu
Copy link
Member Author

@smaug---- I meant the "entry settings object's responsible document's URL." If I have

<!-- a.html -->
<iframe src=b.html></iframe>
<button onclick="reloadFrame()">Reload</button>
<script>
  function reloadFrame() {
    frames[0].document.open();
    frames[0].document.write("New content");
    frames[0].document.close();
    frames[0].location.reload();
  }
</script>
<!-- b.html -->
Old content

The frame when the Reload button is clicked will go to a.html in Safari, b.html in Chrome, and the written document elsewhere.

@bzbarsky
Copy link
Contributor

bzbarsky commented Aug 8, 2018

I agree that ideally we'd have the document url and current history entry url match.

I just did some digging around, and I agree that this is the one place where it looks like the two can diverge at least in the Firefox implementation (modulo the fact that open() creates a new history entry).

That said, just mutating the URL in the history entry doesn't seem like the right thing to me. For example, consider a POST result that is then document.open()ed, changing its URL. If reloaded, should that send the same POST data but to a different URL?

In general, what should happen to the non-URL information history entries store? At first glance in Gecko that includes pushState/popState state, saved form data, POST data (not in the spec?), scroll positions, referrer (not in the spec?), srcdoc data (but in the spec reloading re-reads this from the iframe instead?), information about the origin that initially triggered the load (not in the spec, because that part of the spec is pretty broken imo). What information like that does Safari store in history entries? What happens to it on document.open()?

My preference if we want a Safari-like behavior is that document.open() creates a new history entry without POST data, with the desired URI, and replaces the old history entry in the history. Basically, do more or less what location.replace() does with the session history. Probably including dropping the entries that come after the current entry (the one we're replacing).

There's still the issue of ending up with multiple history entries that have the same document but are not really related to each other in the way that entries corresponding to pushState or fragment navigation are... How do we want to handle those cases where the spec compares documents and assumes them matching means the entries differ by a fragment or pushState?

@TimothyGu
Copy link
Member Author

TimothyGu commented Aug 10, 2018

@domenic and I discussed this, and the solution we came up with was that we could perhaps adopt the processing model of history.replaceState(). Instead of merely changing the URL of the current entry in session history, we could reset a few more states in that entry. The relevant spec steps for replaceState() are:

  1. Otherwise, if the method invoked was the replaceState() method:

    1. Update the current entry in the session history so that serializedData is the entry's new serialized state, the given title is the new title, and new URL is the entry's new URL.
  2. If the current entry in the session history represents a non-GET request (e.g. it was the result of a POST submission) then update it to instead represent a GET request.

  3. Set document's URL to new URL.

    Note: Since this is neither a navigation of the browsing context nor a history traversal, it does not cause a hashchange event to be fired.

where new URL in our case would be the URL of the responsible document of the entry settings object. Notably, this does not remove history entries after the current one, which makes it compatible with current Chrome and Safari behaviors. Reloading behavior would still be the same as Safari however.

This should solve @bzbarsky's concern about extra data stored in the history entry like POST data. In particular, since we are reusing an existing primitive rather than introducing a new one to the platform, the question of

How do we want to handle those cases where the spec compares documents and assumes them matching means the entries differ by a fragment or pushState?

should resolve itself.

TimothyGu added a commit to web-platform-tests/wpt that referenced this issue Aug 17, 2018
TimothyGu added a commit to TimothyGu/html that referenced this issue Aug 20, 2018
The current session history handling is removed and replaced by a
simpler history.replaceState-based model. This also removes the seldom
implemented "replace" parameter in document.open(), and converge with
Chrome and Safari by always replacing.

Tests: ...

Fixes whatwg#3564.
Fixes whatwg#3885.
TimothyGu added a commit to TimothyGu/html that referenced this issue Aug 20, 2018
The current behavior of adding a new entry to the session history is
removed, with the "replace" parameter behavior made the only option. To
fix whatwg#3885, we now reuse history.replaceState()'s history model, which
keeps the document's URL and the history entry URL in sync.

At the same time, we restrict the history state update (including
setting the document's URL) to be only run with fully active documents.
Firefox already bails out for non-fully active documents (including
those that are active documents), and it's not expected that there would
be much usage of the URL of non-fully active documents anyway.

This allows us to additionally remove the seldom implemented "replace"
parameter in document.open(), whose behavior is now the default
(aligning with with Chrome and Safari). The IDL is modified accordingly.

This PR also removes the "overridden reload" algorithm and related
concepts, which were previously only implemented in Firefox and Edge,
causing developer confusion evident in
https://bugzilla.mozilla.org/show_bug.cgi?id=556002.

Tests: web-platform-tests/wpt#12555
Tests: … (more coming soon)

Fixes whatwg#3564.
Fixes whatwg#3885.
TimothyGu added a commit to TimothyGu/html that referenced this issue Aug 21, 2018
The current behavior of adding a new entry to the session history is
removed, with the "replace" parameter behavior made the only option. To
fix whatwg#3885, we now reuse history.replaceState()'s history model, which
keeps the document's URL and the history entry URL in sync.

At the same time, we restrict the history state update (including
setting the document's URL) to be only run with fully active documents.
Firefox already bails out for non-fully active documents (including
those that are active documents), and it's not expected that there would
be much usage of the URL of non-fully active documents anyway.

This allows us to additionally remove the seldom implemented "replace"
parameter in document.open(), whose behavior is now the default
(aligning with with Chrome and Safari). The IDL is modified accordingly.

This PR also removes the "overridden reload" algorithm and related
concepts, which were previously only implemented in Firefox and Edge,
causing developer confusion evident in
https://bugzilla.mozilla.org/show_bug.cgi?id=556002.

Tests: web-platform-tests/wpt#12555
Tests: … (more coming soon)

Fixes whatwg#3564.
Fixes whatwg#3885.
TimothyGu added a commit to TimothyGu/html that referenced this issue Aug 23, 2018
The current behavior of adding a new entry to the session history is
removed, with the "replace" parameter behavior made the only option. To
fix whatwg#3885, we now reuse history.replaceState()'s history model, which
keeps the document's URL and the history entry URL in sync.

At the same time, we restrict the history state update (including
setting the document's URL) to be only run with fully active documents.
Firefox already bails out for non-fully active documents (including
those that are active documents), and it's not expected that there would
be much usage of the URL of non-fully active documents anyway.

This allows us to additionally remove the seldom implemented "replace"
parameter in document.open(), whose behavior is now the default
(aligning with with Chrome and Safari). The IDL is modified accordingly.

Tests: web-platform-tests/wpt#12555
Tests: web-platform-tests/wpt#12634
Tests: web-platform-tests/wpt#12636
Tests: web-platform-tests/wpt#12650

Fixes whatwg#3564.
Fixes whatwg#3885.
TimothyGu added a commit to TimothyGu/html that referenced this issue Aug 23, 2018
The current behavior of adding a new entry to the session history is
removed, with the "replace" parameter behavior made the only option. To
fix whatwg#3885, we now reuse history.replaceState()'s history model, which
keeps the document's URL and the history entry URL in sync.

At the same time, we restrict the history state update (including
setting the document's URL) to be only run with fully active documents.
Firefox already bails out for non-fully active documents (including
those that are active documents), and it's not expected that there would
be much usage of the URL of non-fully active documents anyway.

This allows us to additionally remove the seldom implemented "replace"
parameter in document.open(), whose behavior is now the default
(aligning with with Chrome and Safari). The IDL is modified accordingly.

Tests: web-platform-tests/wpt#12555
Tests: web-platform-tests/wpt#12634
Tests: web-platform-tests/wpt#12636
Tests: web-platform-tests/wpt#12650

Fixes whatwg#3564.
Fixes whatwg#3885.
TimothyGu added a commit to TimothyGu/html that referenced this issue Aug 23, 2018
The current behavior of adding a new entry to the session history is
removed, with the "replace" parameter behavior made the only option. To
fix whatwg#3885, we now reuse history.replaceState()'s history model, which
keeps the document's URL and the history entry URL in sync.

At the same time, we restrict the history state update (including
setting the document's URL) to be only run with fully active documents.
Firefox already bails out for non-fully active documents (including
those that are active documents), and it's not expected that there would
be much usage of the URL of non-fully active documents anyway.

This allows us to additionally remove the seldom implemented "replace"
parameter in document.open(), whose behavior is now the default
(aligning with with Chrome and Safari). The IDL is modified accordingly.

Tests: web-platform-tests/wpt#12555
Tests: web-platform-tests/wpt#12634
Tests: web-platform-tests/wpt#12636
Tests: web-platform-tests/wpt#12650

Fixes whatwg#3564.
Fixes whatwg#3885.
TimothyGu added a commit to TimothyGu/html that referenced this issue Aug 23, 2018
The current behavior of adding a new entry to the session history is
removed, with the "replace" parameter behavior made the only option. To
fix whatwg#3885, we now reuse history.replaceState()'s history model, which
keeps the document's URL and the history entry URL in sync.

At the same time, we restrict the history state update (including
setting the document's URL) to be only run with fully active documents.
Firefox already bails out for non-fully active documents (including
those that are active documents), and it's not expected that there would
be much usage of the URL of non-fully active documents anyway.

This allows us to additionally remove the seldom implemented "replace"
parameter in document.open(), whose behavior is now the default
(aligning with with Chrome and Safari). The IDL is modified accordingly.

Tests: web-platform-tests/wpt#12555
Tests: web-platform-tests/wpt#12634
Tests: web-platform-tests/wpt#12636
Tests: web-platform-tests/wpt#12650

Fixes whatwg#3564.
Fixes whatwg#3885.
domenic pushed a commit that referenced this issue Aug 23, 2018
The current behavior of adding a new entry to the session history is
removed, with the "replace" parameter behavior made the only option. To
fix #3885, we now reuse history.replaceState()'s history model, which
keeps the document's URL and the history entry URL in sync.

At the same time, we restrict the history state update (including
setting the document's URL) to be only run with fully active documents.
Firefox already bails out for non-fully active documents (including
those that are active documents), and it's not expected that there would
be much usage of the URL of non-fully active documents anyway.

This allows us to additionally remove the seldom implemented "replace"
parameter in document.open(), whose behavior is now the default
(aligning with with Chrome and Safari). The IDL is modified accordingly.

Tests: web-platform-tests/wpt#12555
Tests: web-platform-tests/wpt#12634
Tests: web-platform-tests/wpt#12636
Tests: web-platform-tests/wpt#12650

Fixes #3564.
Fixes #3885.
mustaqahmed pushed a commit to mustaqahmed/html that referenced this issue Feb 15, 2019
The current behavior of adding a new entry to the session history is
removed, with the "replace" parameter behavior made the only option. To
fix whatwg#3885, we now reuse history.replaceState()'s history model, which
keeps the document's URL and the history entry URL in sync.

At the same time, we restrict the history state update (including
setting the document's URL) to be only run with fully active documents.
Firefox already bails out for non-fully active documents (including
those that are active documents), and it's not expected that there would
be much usage of the URL of non-fully active documents anyway.

This allows us to additionally remove the seldom implemented "replace"
parameter in document.open(), whose behavior is now the default
(aligning with with Chrome and Safari). The IDL is modified accordingly.

Tests: web-platform-tests/wpt#12555
Tests: web-platform-tests/wpt#12634
Tests: web-platform-tests/wpt#12636
Tests: web-platform-tests/wpt#12650

Fixes whatwg#3564.
Fixes whatwg#3885.
mustaqahmed pushed a commit to mustaqahmed/html that referenced this issue Feb 15, 2019
The current behavior of adding a new entry to the session history is
removed, with the "replace" parameter behavior made the only option. To
fix whatwg#3885, we now reuse history.replaceState()'s history model, which
keeps the document's URL and the history entry URL in sync.

At the same time, we restrict the history state update (including
setting the document's URL) to be only run with fully active documents.
Firefox already bails out for non-fully active documents (including
those that are active documents), and it's not expected that there would
be much usage of the URL of non-fully active documents anyway.

This allows us to additionally remove the seldom implemented "replace"
parameter in document.open(), whose behavior is now the default
(aligning with with Chrome and Safari). The IDL is modified accordingly.

Tests: web-platform-tests/wpt#12555
Tests: web-platform-tests/wpt#12634
Tests: web-platform-tests/wpt#12636
Tests: web-platform-tests/wpt#12650

Fixes whatwg#3564.
Fixes whatwg#3885.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants