Closed domenic closed 4 years ago
I would distinguish portal host initiated navigations from portal content initiated navigations, the latter being (1.iii), (3.iii), and (4.iii).
Especially with src changes causing reloads (#197), the portal host initiated navigations all produce a portal context that is empty (or effectively empty). At that point, it seems like we may as well discard such a context. An error page might work for some of these, but having an extra unactivatable portal state seems worth avoiding, plus having the page handle the discard rather than showing an error page seems like better UX.
For portal content initiated navigation, it seems sufficient to ignore/cancel the navigation, since we still have a usable portal context. Indeed, treating navigations that could be no-ops as ways for the portal context to self-destruct may be excessive, even though these are corner cases. However, a navigation that would be unacceptable even in a top level context (e.g. about:settings, about:flags) could be grounds for discarding.
My instinct is that things that shouldn't load should be not committed (much like a 204 response would). If we combine that with the idea that src always creates a new browsing context, then we should close that browsing context the moment its initial navigation cannot commit (because that was a bad URL, or redirected to something invalid, etc). i.e.,
Bad URL cases:
<portal src="data:text/html,foo"></portal>
Immediately closed (but we can short-circuit that).
<portal src="https://example.com/"></portal>
wherehttps://example.com/
301 redirects todata:text/html,foo
.
Closed when the redirect is found.
<portal src="https://example.com/"></portal>
wherehttps://example.com/
performslocation.href = "data:text/html,foo"
. (This category also includes other post-load navigations, e.g.<meta http-equiv="refresh">
orlinkToBadURL.click()
.)
Remains on https://example.com/?
Navigation error cases:
<portal src="https://example.com/404"></portal>
Hmm. My instinct would be "render the 404 page" (it's content of a sort), but I could be convinced that in a cross-origin prerendering case we would rather close, on the basis that if we were prerendering we would abandon prerendering at that point.
We're kinda saved from this being a way to probing vector by the uncredentialed restriction, I think.
AFAIK, HTTP 404 doesn't make iframes fire error
or other things like that.
<portal src="https://example.unresolvable"></portal>
Probably close on resolution failure.
<portal src="https://example.com/"></portal>
where the page is prevented from loading by CSP
Probably close.
<portal src="https://example.com"></portal>
but the user is offline
Probably close.
Downloads:
<portal src="https://example.com/content-disposition-attachment"></portal>
Close? I don't think we want downloads and there's no inline-disposition content to render.
<portal src="https://example.com/></portal>
wherehttps://example.com/
301 redirects to aContent-Disposition: attachment
page
Close.
<portal src="https://example.com/></portal>
wherehttps://example.com/
performslocation.href = "content-disposition-attachment"
or variants.
Remain on the same page, since at least we have a committed page? I'm kinda reluctant to remove the previously shown/valid page and throw the whole thing out, but could be convinced.
204 No Content:
<portal src="https://example.com/204"></portal>
Close.
<portal src="https://example.com/></portal>
wherehttps://example.com/
301 redirects to a 204 page
Close.
<portal src="https://example.com/></portal>
wherehttps://example.com/
performslocation.href = "204"
or variants.
As above, remain?
The spec's currently handling:
- (1.i) closes the portal
- (1.ii)-(1.iii) navigate the portal browsing context to the bad URL
- (2) navigates the portal to an error page
- (3.i) performs the download while leaving the portal browsing context in its initial empty state, I guess?
- (3.ii)-(3.iii) performs the download while leaving the portal browsing context on
https://example.com/
- (4.i) leaves the portal browsing context in its initial empty state, I think?
- (4.ii)-(4.iii) leaves the portal browsing on
https://example.com/
.We definitely need to fix (1.ii)-(1.iii), (3.i), and (4.i) in the spec.
We may also want to consider some unification. For example maybe all of (1) and (2) should close the portal, or maybe all of (1) and (2) should display an error page. For (3) and (4), things are a bit trickier, because (3.ii)-(3.iii) and (4.ii)-(4.iii)'s existing behavior is OK, but (3.i) and (4.i) is not. So should we special-case (3.i) and (4.i), or should we maybe treat all of (3) and (4) the same as (1) and (2), or...? (3) has other options, e.g. we could just ignore the
Content-Disposition
header in (3.i), or we could treat downloads as a no-op instead of causing an error/closing the portal.I'd love more folks' opinions on this. If I had to pick, I'd go with something simple of (1), (2), (3), and (4) all displaying an error page. (And then we need to spec what happens when you try to activate an error page...)
But @kjmcnee has done more thinking about the concerns related to error pages and the UX and security reasons for/against, and I don't fully remember the past discussions about these cases.
It looks like jeremyroman and my takes are the same.
It's not meaningful to activate "nothing" (a context that has no committed navigation) and activating an error page makes for a poor experience. I certainly think it would be worth it to avoid additional states in which a portal is unactivatable by simply discarding in the cases specified above. That seems like it would be easier for devs to reason about. I think there would also be UX benefits due to not exposing these corner cases to users, since they are not actionable.
With src changes creating new contexts, the logic is fairly straightforward to capture all of these cases. If a navigation fails to mature in a portal context which has yet to have another navigation mature, discard the context.
Awesome! I'll try to spec all this and see how far I get.
Related to #145, #247, #228, #185, #158, #23.
A "bad URL" is a non-HTTP(S) URL, including
data:
URLs orabout:blank
or similar. Portals attempt to prohibit these, and I think that's awesome; they add a lot of complication to iframes. I'll usedata:text/html,foo
as the example, but the same goes for any of them.Now, consider the following scenarios:
<portal src="data:text/html,foo"></portal>
<portal src="https://example.com/"></portal>
wherehttps://example.com/
301 redirects todata:text/html,foo
.<portal src="https://example.com/"></portal>
wherehttps://example.com/
performslocation.href = "data:text/html,foo"
. (This category also includes other post-load navigations, e.g.<meta http-equiv="refresh">
orlinkToBadURL.click()
.)<portal src="https://example.com/404"></portal>
<portal src="https://example.unresolvable"></portal>
<portal src="https://example.com/"></portal>
where the page is prevented from loading by CSP<portal src="https://example.com"></portal>
but the user is offline<portal src="https://example.com/content-disposition-attachment"></portal>
<portal src="https://example.com/></portal>
wherehttps://example.com/
301 redirects to aContent-Disposition: attachment
page<portal src="https://example.com/></portal>
wherehttps://example.com/
performslocation.href = "content-disposition-attachment"
or variants.<portal src="https://example.com/204"></portal>
<portal src="https://example.com/></portal>
wherehttps://example.com/
301 redirects to a 204 page<portal src="https://example.com/></portal>
wherehttps://example.com/
performslocation.href = "204"
or variants.The spec's currently handling:
https://example.com/
https://example.com/
.We definitely need to fix (1.ii)-(1.iii), (3.i), and (4.i) in the spec.
We may also want to consider some unification. For example maybe all of (1) and (2) should close the portal, or maybe all of (1) and (2) should display an error page. For (3) and (4), things are a bit trickier, because (3.ii)-(3.iii) and (4.ii)-(4.iii)'s existing behavior is OK, but (3.i) and (4.i) is not. So should we special-case (3.i) and (4.i), or should we maybe treat all of (3) and (4) the same as (1) and (2), or...? (3) has other options, e.g. we could just ignore the
Content-Disposition
header in (3.i), or we could treat downloads as a no-op instead of causing an error/closing the portal.I'd love more folks' opinions on this. If I had to pick, I'd go with something simple of (1), (2), (3), and (4) all displaying an error page. (And then we need to spec what happens when you try to activate an error page...)