-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Session history indexing is broken #2985
Comments
I suspect this in part is a duplicate of #1454, but not marking it as such yet since this contains a lot of information too. |
It's more than that. It's a objection to exposing |
Sure, but the correct fix is to fix the spec per #1454, from what I understand. |
Does that solve the indexing problem, though? Navigating (or worse, removing) subframes can still remove chunks from the session history, even with the #1454 approach. That means either gaps in the indices or reindexing. In practice, Gecko reindexes, which makes it impossible to implement a sane (There are other things that can lead to removals of stuff from session history, starting with expiration of too-old session history entries. Right now I think the spec requires that they all be kept, but browsers tend to ignore "leak all the memory" spec requirements like that in practice, and I wish specs would stop having them.) |
I've posted #3460 for removing history.index from the spec for now. |
As that's merged, closing this. |
This reverts f8e293f. Although there was multi-implementer interest at the time, implementations have not materialized, and Gecko has objected in whatwg#2985 that they are not able to implement in their architecture.
@bzbarsky just curious:
Why's it ok to update |
|
@jakearchibald if |
@bzbarsky sure, but you can say the same for |
Yeah, and again, I think we should make that less reliable. It's not like the history API has won any design awards. It's something that at some point got exposed and sites rely on to some extent, but it's also not fully interoperable or necessarily sensible given all the cross-origin leaks. |
Cool. It just seemed from this thread that |
Exactly. In practice, the only uses we've seen for |
Gotcha. Thanks for clarifying. |
https://html.spec.whatwg.org/multipage/history.html#history defines things as following:
One problem with this is that the spec doesn't define whether entry indices are static or dynamic. That is, does removing entries from the joint session history affect the indexing of entries that had larger indices?
As a concrete example, say we have a page X, with child browsing contexts A and B which load documents Y and Z respectively. Y has child browsing context C. C initially loads document W. The following steps happen:
After step 1, I believe the only entry in the joint session history is the one for the toplevel page (but am not sure about this; it's not clear what the "most recently added" thing is in this case). Its index is presumably 0.
After step 2, there are two entries in the joint session history: The one for W and the one for W'. Their indices are either [0, 1] (dynamic) or [1, 2] (static).
After step 3, there are three entries in the joint session history: W, Z, Z'. Their indices are either [0, 1, 2] (dynamic) or [1, 3, 4] (static).
After step 4, Y is no longer fully active, so the session history of C no longer participates in the joint session history. The joint session history now contains Z, Y', with indices either [0, 1] (dynamic) or [3, 5] (static).
So far so unclear, but now we have problems with both indexing versions. In the dynamic version, the entry corresponding to "right before step 3" has its index change from 1 to 0. Since
.index
is exposed, this seems pretty suboptimal. It certainly makes it impossible to actually use the exposed.index
for anything sanely.In the static version,
history.back()
is broken. That's because it tries to traverse the history by a delta of -1, and in step 1 of https://html.spec.whatwg.org/multipage/history.html#traverse-the-history-by-a-delta the index of the current entry is 5, 5-1 is 4, the number of items is 2, so the traverse steps are aborted.The text was updated successfully, but these errors were encountered: