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

<iframe> and the History API #184

Open
hayatoito opened this issue Jul 6, 2015 · 95 comments
Open

<iframe> and the History API #184

hayatoito opened this issue Jul 6, 2015 · 95 comments

Comments

@hayatoito
Copy link
Member

@hayatoito hayatoito commented Jul 6, 2015

@annevk
Copy link
Member

@annevk annevk commented Feb 16, 2016

@rniwa what is Apple doing here?

@annevk annevk changed the title [Shadow]: Figure out how session history should work for <iframe>s in shadow DOM (bugzilla: 27325) <iframe> and the History API Feb 16, 2016
@annevk
Copy link
Member

@annevk annevk commented Feb 16, 2016

@smaug---- so I guess the problem is all the state exposed on History? If we did not expose that, would we have to expose it somehow on ShadowRoot? That each gets its own history? Presumably defaulting to no history at all for v1?

That kind of approach makes sense if you don't want to influence your container, but if you're a simple layout component that breaks expectations. Not sure if there's a good answer here. (We could make it depend on open vs closed...)

@smaug----
Copy link

@smaug---- smaug---- commented Feb 16, 2016

For open case I could possibly live with normal nested browsing context handling.
And for closed...no session history at all? I think that is the easiest for now. history.pushState/replaceState would do what? and what about history.go(x)? Throw?
I think throw is the only sane option, except perhaps for go(0)
How should the UI work? If shadow-iframe loads page X and then Y is loaded there, what should back button do? I guess nothing. That shouldn't be too bad. The shadow-iframe container component could manually call push/replaceState when needed, or shadow-iframe.src = "some new page"

Given the recent example, bug in Google Ads where their scripts caused the container page to go back, I'd prefer to never expose history state even in open case, but at least in closed case the state shouldn't be shared.

@annevk
Copy link
Member

@annevk annevk commented Feb 17, 2016

I would be okay with blocking history from "shadow-iframe" by default. And then later figuring out if we want to make that work. @hayatoito @rniwa?

@annevk
Copy link
Member

@annevk annevk commented Feb 17, 2016

I'm not sure if throwing would be best by the way. It depends on what kind of things end up being embedded. If you throw a bunch of existing pages will likely break when embedded in this way.

@smaug----
Copy link

@smaug---- smaug---- commented Feb 17, 2016

but if the page called go(<not_zero>), it sure would expect something to happen, and if we wouldn't throw, the page would be broken in some other way.
Or pushState(). After that one would expect history.state to be the thing passed to pushState.

This is tricky, but I'm not sure we have any other good option to effectively disable all history API in shadow-iframes.
Hmmm, crazy idea. Could we actually hide History interface and .history propery in the browsing contexts under shadow-iframes?

@hayatoito
Copy link
Member Author

@hayatoito hayatoito commented Mar 15, 2016

I would be okay with blocking history from "shadow-iframe" by default. And then later figuring out if we want to make that work. @hayatoito @rniwa?

I am okay with this. Blocking history from "shadow-iframe" by default should be a safe option.

[Updated] On the second thought, go() should work, IMO. Let me think further.

@hayatoito
Copy link
Member Author

@hayatoito hayatoito commented Mar 15, 2016

My opinion is:

  • iframes in a shadow tree does not have any affect on window.history. Thus, states of iframes in a shadow trees is not exposed to window.history. In other words, window.history is owned exclusively by a document tree.
  • go() should work as usual.

We might want to have another APIs, such as DocumentOrShadowRoot.history, if we want to make that work somehow later.

@annevk
Copy link
Member

@annevk annevk commented Mar 15, 2016

How can go() work if such <iframe>s don't have an effect on the history?

@hayatoito
Copy link
Member Author

@hayatoito hayatoito commented Mar 15, 2016

go() works in a history of a document tree. So I guess calling window.history.go() from a (developer of) shadow tree is a bad practice and does not bring a desired behavior in most cases.

@smaug----
Copy link

@smaug---- smaug---- commented Mar 15, 2016

To be a tad more precise, go() works in browsing context tree.

Not putting shadow dom iframe loads to session history, but having go() to work would lead to very surprising behavior. The browsing context in iframe could expect its page loads are in session history, but go() wouldn't actually know about those loads so it would always affect to some ancestor or sibling browsing context, never the browsing context in iframe.

(go() is super error prone API to use :/ )

@hayatoito
Copy link
Member Author

@hayatoito hayatoito commented Apr 1, 2016

Yeah, I think we can not have a perfect solution here. We have to compromise for window.history because it's globally accessible.

@hayatoito hayatoito closed this in 08793be Apr 8, 2016
@annevk
Copy link
Member

@annevk annevk commented Apr 8, 2016

That doesn't really solve this issue though, does it? I guess we still have the corresponding issue against the HTML Standard to define what should happen here...

@hayatoito
Copy link
Member Author

@hayatoito hayatoito commented Apr 8, 2016

Let me reopen. Should we define how go() works?

@hayatoito hayatoito reopened this Apr 8, 2016
@annevk
Copy link
Member

@annevk annevk commented Apr 8, 2016

Yeah, and all the other members.

@TakayoshiKochi
Copy link
Member

@TakayoshiKochi TakayoshiKochi commented Apr 13, 2016

So the conclusion here is to leave the history API work as is, even in closed shadow root,
and no blocking for shadow-iframes?
i.e. any <iframe> navigation history will be included in joint session history.

As dimitri's comment pointed out, whether shadow DOM is detectable is not a concern.

I'll work on a draft for pull request.

@hayatoito
Copy link
Member Author

@hayatoito hayatoito commented Apr 14, 2016

That does not seem the conclusion here... ;)

Let me explain the context:

I and @TakayoshiKochi discussed this topic yesterday, and we got a rough consensus:
"Let window.history API work as is. <iframe> in a shadow tree will contribute a top-level browsing context's joint session history."

@annevk
Copy link
Member

@annevk annevk commented Apr 14, 2016

That conclusion seems directly opposed to comments from folks from Apple and Mozilla that very much want encapsulation at all cost, even if it means losing features like document.currentScript.

@hayatoito
Copy link
Member Author

@hayatoito hayatoito commented Apr 14, 2016

Yeah, this is the opposite. Actually, I have changed my mind.

Yesterday, I tried to have a good proposal, but I can not have any concrete proposal if we are to block iframe in a shadow tree. That made me in a trouble. Some of the concerns in #184 (comment) can not be resolved.

After that, I am starting to tend to agree with Dimitir's comment in this particular case:

there is no big concern if In this perspective, exposing information or making Shadow DOM presence detectable is not as big of a concern.

Maybe I might change my mind again, but as of now, I am fine with "not blocking at all".

@smaug----
Copy link

@smaug---- smaug---- commented Apr 14, 2016

I'm very worried about exposing shadow iframes in the the light DOM's session history. It would make using history API in light DOM super error prone: it would become really hard for light DOM to know what the current state actually is.
History API is error prone enough even without shadow DOM. The recent incident where Google Ads started to do random history.go(-1) and forcing browser to go to the previous top level page (since go(-1) was called at wrong state or something) is just a hint of that.

@TakayoshiKochi
Copy link
Member

@TakayoshiKochi TakayoshiKochi commented Apr 15, 2016

@smaug---- is the concern shadow-dom specific?

I may not be understanding the case you quoted. In my understanding the injected Google Ads script mistakenly ran history.go(-1) which expected to navigate some <iframe> to go back to its history, but the history was empty and the main document went back, right?
This can easily happen if there are more than one injected <iframe>s and script tries to go back and forth independently for each <iframe>.

That's a reasonable concern but it should be addressed at nested browsing context and history API's behavior about joint session history, not at the level of shadow tree boundary.

@rniwa
Copy link
Contributor

@rniwa rniwa commented Apr 15, 2016

Let's say a social network site starts using shadow DOM and an iframe for its own widget to display comments. Then, you don't want navigations within that iframe to affect the navigation history of the embedding page. It DOES happen today but I don't think that's really intended / desirable.

@smaug----
Copy link

@smaug---- smaug---- commented Jun 20, 2016

I could rephrase that a bit. s/Mozilla/smaug/ and a question is here that why would we be ok to break encapsulation here when we're trying to achieve it elsewhere?

And functionally it is very surprising if the light dom can affect to the state of shadow iframes via history API, and even more surprising is if different shadow trees can affect to each others via history API.

@TakayoshiKochi
Copy link
Member

@TakayoshiKochi TakayoshiKochi commented Jun 20, 2016

We understand your concern.

The point we'd argue is that "history API usages cause unexpected behavior" (current behavior) vs "splitting session history for shadow iframes causes unexpected behavior for browser UI back/forward navigation".

As I understand, no one has come up with a clear solution to the question posed in #184 (comment).
(I tried in several comments but failed, which is a shame)

In @smaug---- 's #184 (comment)

Session history transaction list would just contain entries for various session histories. And if history.go(...) was used, that would need to skip entries not for that particular session history.
UI would operate on the transaction list in the normal way.

"UI would operate on the transaction list" is the tricky part.
For example, when an iframe did history.go(-1), it would create a new transaction.
Then UI back button would forward the iframe?

I don't think there are infinite options how UI back button would behave in this case,
but there are only a few possible ways. As far as we explored for Blink, they were either
too complex to implement (if not impossible) or more confusing behavior to users.

The most plausible idea I came up with was that such separated session history for
shadow iframes would not be added to the list for UI back/forward button. This turned
out to be implementable, but I got comments internally that users would get confused.

So we are comparing cost/benefit of implementing the session history split vs
keeping the current behavior.

Keeping the current behavior, basically no cost, but has negative benefit that you already
mentioned several times. Pro is that there is no change in iframe and history API behavior,
in a sense.

On the other hand, splitting the session history, we foresee some non-ignorable implementation
cost (plus cost for inventing/defining a reasonable behavior), clear benefit for encapsulation,
and uncertain con for unexpected inconsistency for UI back/forward button behavior.

We are in the pragmatic position to favor the former.

One problem with merging various session history trees in UI is that top level page's history.back()/forward()/go() wouldn't map to it anymore. I guess that is fine.

I'd agree that UI back/forward list and top-level session can diverge, but this is not the
point we are against.

@smaug----
Copy link

@smaug---- smaug---- commented Jun 20, 2016

The most plausible idea I came up with was that such separated session history for
shadow iframes would not be added to the list for UI back/forward button.

This is what I'd do. Users don't know what an "iframe" is. and iframe could be implement by the page using just random div element which doesn't add anything to the session history.

This turned
out to be implementable, but I got comments internally that users would get confused.

Why? If user doesn't even know there is an iframe to navigate back<->forward, what is there to get confused?

I would, at least initially, make shadow iframes to have separate session history so that we don't break existing pages using session history, yet don't leak information about shadow iframes to other shadow iframes or light DOM and not expose shadow history in UI.
Then, if we later figure out that shadow iframe history needs to be exposed to the UI, we try to come up with some solution.
That is my pragmatic proposal for this issue.

(In my mind not fixing session history for shadow DOM case is similar to the v0 shadow DOM where is-in-document issue was totally not figured out at all.)

@TakayoshiKochi
Copy link
Member

@TakayoshiKochi TakayoshiKochi commented Jun 20, 2016

This turned
out to be implementable, but I got comments internally that users would get confused.

Why? If user doesn't even know there is an iframe to navigate back<->forward, what is there to get confused?

Suppose there are 2 components, one with Shadow DOM and the other without Shadow DOM
otherwise identical. Even when history API is used only within <iframe> in the components,
once history API is used, browser UI back/forward button behaves differently if shadow-iframe
history isn't exposed to UI. Users cannot distinguish them from what they appear. Therefore
it gets confusing.

I don't have the backing data to support this, such as how frequently this pattern would be used in real world, unfortunately. But I guess the situation happens for polyfilled Shadow DOM and native Shadow DOM for the time being until everyone implements.

Having separate list and not exposing history to UI doesn't come as free. Even when we can
ignore the cost for implementation, users may have to pay the cost for incompatible behavior,
in turn for developer (history API user, component developer)'s convenience and sanity.

So the question is whether we (browser implementors) can impose the (possible) pain to the users,
or the benefit of the separation wins against the pain.

@trusktr
Copy link

@trusktr trusktr commented Jun 28, 2016

I haven't read too far past #184 (comment), but for number 3 to be possible, which is

Design iframe elements differently inside shadow trees so they do not put properties on the global object and don't interfere with the history of the browsing context. Could be quite a bit of work, I haven't had time to investigate yet.

then this means that Nodes that are distributed into a ShadowDOM tree (into slot elements) must somehow be aware that they have been distributed into a ShadowDOM tree (even if the tree is closed) in order to decide to behave differently than when not in a ShadowDOM tree. There is currently no part of webcomponents spec (as far as I'm aware, please let me know if otherwise) that would explain how a Node can possibly be aware of such a thing as having been distributed into a shadow tree.

So, this leads me to link here to an idea I made earlier: distributedCallback. The slot argument to distributedCallback is null when the shadow tree that the element is distributed into is closed. This method could possibly be the explanation as to how an <iframe> can possibly know to behave differently inside a shadow tree, if that route is taken with iframes.

@smaug----
Copy link

@smaug---- smaug---- commented Jun 28, 2016

Possible answer B: the new history state replaces Entry3.

Entry1: A1
Entry2: A2 + B1 + C1
Entry3': A2 + B2 + C1
Entry4: A2 + B2 + C2

I'm leaning to this. (I got the same idea and then realized that perhaps it was proposed already)
The replacement happens only when history.back()/go(<0) is explicitly used in shadow browsing contexts. If that doesn't happen, the UI works just the way it works now, since there is the session history transaction list of session history entry trees

@TakayoshiKochi
Copy link
Member

@TakayoshiKochi TakayoshiKochi commented Jun 30, 2016

Thanks @smaug---- for giving more thought on this.

How feasible do you feel for implementing it in Gecko/Firefox?
I guess it is implementable in Chromium/Blink, but with much effort - it needs huge refactoring;)

I'm curious what do you think if you get back to Entry3' state now, and if iframe B navigates B3,
a new entry (Entry3'' = A2 + B3 + C1) will be created. Usually going back the history and make a new
navigation, will prune the old forward history, in this case Entry4, which means we lose "C2".
But still in iframe C's history list, C2 exists, so history.forward() in C could navigate to C2.
Do you think you allow UI forward button to make C1->C2 navigation (with keeping B3 in
iframe B) in this case?

Entry1: A1
Entry2: A2 + B1 + C1
Entry3': A2 + B2 + C1
Entry3'': A2 + B3 + C1 <== new entry after Entry3'
Entry4: A2 + B2 + C2 <== ???

@hayatoito
Copy link
Member Author

@hayatoito hayatoito commented Jul 21, 2016

I am now triaging issues. Is there any update on this issue?

@TakayoshiKochi
Copy link
Member

@TakayoshiKochi TakayoshiKochi commented Sep 13, 2016

@smaug---- any update on this?

@smaug----
Copy link

@smaug---- smaug---- commented Sep 13, 2016

Sorry, no. I'm hoping this can be chatted during TPAC. But I'll try to look at this before that anyhow.

@TakayoshiKochi
Copy link
Member

@TakayoshiKochi TakayoshiKochi commented Sep 14, 2016

Thanks for the update. I'll be at TPAC next week.
Let's chat about this.

@hayatoito
Copy link
Member Author

@hayatoito hayatoito commented Nov 30, 2017

@TakayoshiKochi , @rniwa
Is there any progress on this? I think Mozilla wants to know that status of this issue.

@rniwa
Copy link
Contributor

@rniwa rniwa commented Dec 3, 2017

We didn't have a time to discuss it at TPAC 2017.

@smaug----
Copy link

@smaug---- smaug---- commented Dec 6, 2017

This was discussed at TPAC 2016, and the approach we agreed to try out was going to be implemented in webkit, IIRC, to get more experience on this issue before spec'ing it.

@rniwa
Copy link
Contributor

@rniwa rniwa commented Dec 6, 2017

Yeah, sorry, I haven't had a time to implement it. If Gecko or some other engine is interested in implementing the proposed approach, please move ahead & give us any feedback.

@annevk
Copy link
Member

@annevk annevk commented Dec 7, 2017

So do you disable <iframe> altogether or are we going to run into compatibility issues?

@smaug----
Copy link

@smaug---- smaug---- commented Dec 7, 2017

Yeah, I don't know what blink and webkit do now. Probably something somewhat random, which exposes the existence of shadow DOM to the outside world by using the same session history.

Did anyone take pictures of the plan we draw 2016?
It was separate session history for shadow iframes, since otherwise iframes in different shadow DOMs could end up affecting each others.

@smaug----
Copy link

@smaug---- smaug---- commented Dec 7, 2017

But, if there are two iframe in a shadow DOM, should those use the same session history?

@rniwa
Copy link
Contributor

@rniwa rniwa commented Feb 19, 2018

We're willing to make backwards incompatible changes on our end to match the new spec if any.

@rniwa
Copy link
Contributor

@rniwa rniwa commented Oct 25, 2018

Here's a strawman proposal:

  1. In the global navigation history, look for the history item where the navigated frame is the new (destination) state.
  2. Rewrite the history items between the current history item and the item found in (1) so that the navigated frame's state is of the destination state.
  3. Insert new history items where all the frames except the navigated frame are identical to that of the current history item, and the navigated frame is of states between the current frame's state and the destination state.
  4. Navigate back to the new state created in (3) which matches the destination state.
@marcoscaceres
Copy link
Member

@marcoscaceres marcoscaceres commented Oct 25, 2018

screenshot 2018-10-25 10 14 13

screenshot 2018-10-25 10 14 32

@rniwa
Copy link
Contributor

@rniwa rniwa commented Oct 25, 2018

TPAC F2F Consensus / action item: Take back the above proposal and see each browser engine can implement it or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants
You can’t perform that action at this time.