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

Should document.open() on an iframe really add history entries? #1484

Closed
RByers opened this issue Jul 1, 2016 · 5 comments
Closed

Should document.open() on an iframe really add history entries? #1484

RByers opened this issue Jul 1, 2016 · 5 comments
Labels
compat Standard is not web compatible or proprietary feature needs standardizing topic: history

Comments

@RByers
Copy link

RByers commented Jul 1, 2016

The spec seems to imply that a history entry should be added when opening an iframe. Edge and Firefox do this, but Chrome doesn't (blink bug). Changing blink to match the spec looks like it will result in a worse user experience (having to click back more often on some sites). Should we change the spec instead?

Test page.

Note that I don't know much about history myself, just moving this issue from blink to the spec where it belongs. Blink's history expert is japhet@chromium.org but I don't see a GitHub account for him. /cc @domenic @majido.

This is also related to a larger class of issues: WICG/interventions#21

@domenic domenic added the compat Standard is not web compatible or proprietary feature needs standardizing label Jul 1, 2016
@domenic
Copy link
Member

domenic commented Jul 1, 2016

Can you explain the problem in a bit more detail?

document.open() in the general case (and when its "replace" boolean argument is not set to true) is basically meant to blow away the document and show new content. So having it create a new history entry makes sense to me. People will want to be able to press "back" to get the old document back. It's similar to navigation of an iframe in that way. When you navigate in an iframe, you have to click back more times, but that's not a bad thing.

Or is this about the initial "about:blank" document? The spec already takes care of that though, saying that if the only session history entry is the about:blank document, it should be replaced regardless of the replace boolean.

@RByers
Copy link
Author

RByers commented Jul 1, 2016

Sorry I'm just copying the context from the blink bug - take a look there and talk with japhet@ and @travisleithead. From my quick read it sounded like sites were doing programatic navigation of iframes which users did NOT expect to show up in their history.

This seems consistent with @ojanvafai's argument that back/forward should only navigate to history entries created in response to a user gesture. But if we're changing our back/forward behavior and UI treatment (and NOT the information exposed via the history API) then perhaps this isn't actually a spec issue at all but a product UI issue?

@domenic
Copy link
Member

domenic commented Jul 1, 2016

If other browsers are interested in making the change, I think we're happy to change the spec; document.open() is such a ... bad thing ... anyway, so I don't particularly plan to fight for an interpretation of what it's supposed to do.

However, I think it is slightly user-hostile to not create a history entry when someone uses this DOM-level-0 method of "navigating" to a new page. (I think of document.open() as kind of a DOM-level-0 mashup of document.body.innerHTML = with history.pushState, myself.) I agree that maybe as part of a larger effort to not create new history entries without a user gesture, this one might go too. (As you said, I think it's up in the air whether that actually affects history.length, or is just a product UI issue.) But in the meantime keeping it consistent makes sense.

I'm curious whether Gecko and Edge are interested in changing. I guess @travisleithead might be because of the compat problems they're experiencing? @smaug----, thoughts?

@smaug----
Copy link

I agree that maybe as part of a larger effort to not create new history entries without a user gesture, this one might go too.

What is that? Are we trying to kill also pushState? Or is this just an UI thing?
Sounds interesting to deal with that issue in UI level. If someone manages to figure out how, I think it should be spec'ed. (and I'm all for trying to make the current session history handling for user saner.)
But this isn't about this issue at all.

From my quick read it sounded like sites were doing programatic navigation of iframes which users did NOT expect to show up in their history.

Well, pages are doing programmatic navigations all the time, either via .location or via document.open() etc. And pages are also using history.go() in very unexpected ways, leading to surprising bugs. (like the somewhat recent Google Ads bug where its scripts where doing history.go(-1) leading to top level navigation to previous page.)
But the issue is in no way bound to document.open(). Actually, I don't think I've seen that kinds of bugs filed on bugzilla related to document.open() (except reload case, but I consider that an UI bug). But could have of course missed some.

One thing to remember here is that blink is broken also in other ways when dealing with document.open(), like, it doesn't create a new Window object.
So one option is to just fix document.open() in blink/webkit and keep other implementations work per the spec (and the way the API has traditionally worked).

I don't yet have an opinion on this issue. It is possible that I've got used to the weirdness of document.open.
In general I'd prefer to keep those ancient DOM0 APIs which are considered rather bad and almost deprecated to be defined as they have always been defined, and not try to change them.

@domenic
Copy link
Member

domenic commented Aug 10, 2016

Per offline discussion with japhet@, we'll keep the spec as-is. There's still a lot of Blink work to do on aligning document.open() with the spec, but that's kind of separate. And similarly there's a separate question about mitigating the addition of history entries that are not tied to a user gesture.

@domenic domenic closed this as completed Aug 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compat Standard is not web compatible or proprietary feature needs standardizing topic: history
Development

No branches or pull requests

4 participants