Delay activation on initial loading; src="" changes reset portal BC #253
Conversation
README.md
Outdated
| portal.activate(); | ||
| ``` | ||
|
|
||
| activation of the already-loaded content will happen immediately, and the navigation to the new content will happen in the newly-activated browsing context. In these cases, the promise returned by `activate()` will generally fulfill, as it is almost always possible to activate the already-loaded content. (The exceptions are edge cases like if another user-initiated navigation, or another portal activation, is already ongoing.) |
There was a problem hiding this comment.
I still think this bypasses CSP rules, but I guess we can continue debating it in the issues.
There was a problem hiding this comment.
Hmm, didn't we decide that assigning src made a new browsing context? If so, I'd have expected that browsing context to be used, and the old one to be detached immediately when src is assigned. That is, I would expect activate to send you to the last URL assigned to src (+ redirects).
There was a problem hiding this comment.
Oh, right, I forgot about that consideration. I do agree it'll be simpler to just discard when src="" is assigned. OK, so the example I'm really thinking of is an in-page navigation.
This will require a reasonably-big update to this PR. And I'll try to update the spec with discard-on-src-change as well.
|
|
||
| (What, exactly, the `<portal>` element displays in this state is still under discussion: [#251](https://github.com/WICG/portals/issues/251).) | ||
|
|
||
| Attempting to activate a closed portal will fail. Activation can also fail if another navigation is in progress, as discussed [above](#session-history-navigation-and-bfcache). In all of these cases, the promise returned by the `activate()` method will be rejected, allowing page authors to gracefully handle the failure with a custom error experience. |
There was a problem hiding this comment.
I'm still a little worried that portal activation rejections can tell you something about a page that you didn't already know, but we can work on that in issues.
README.md
Outdated
| portal.activate(); | ||
| ``` | ||
|
|
||
| activation of the already-loaded content will happen immediately, and the navigation to the new content will happen in the newly-activated browsing context. In these cases, the promise returned by `activate()` will generally fulfill, as it is almost always possible to activate the already-loaded content. (The exceptions are edge cases like if another user-initiated navigation, or another portal activation, is already ongoing.) |
There was a problem hiding this comment.
Which context the navigation occurs in depends on how we want to interpret activate in this case.
If activate cancels the ongoing navigation in the portal's new context and restarts it in the activated context at the top level, then yes it happens in the newly-activated browsing context.
If activate promotes both the existing context and the context with the pending navigation to the top level, then the navigation continues in said context. When it matures, we navigate the top level to the new context.
That said, that's probably excessive detail for the purposes of this section. The specifics of this are more likely to affect things like CSP, as Jake mentioned. I might just rephrase this to "the navigation to the new content will happen at the top level."
|
left a comment in response to Jake's on line 338; GitHub doesn't seem to want to notify? |
domenic
changed the title
|
Alright, I've kind of morphed this into a bigger change. Now it includes spec changes, and tackles #197 at the same time. I'd like @jeremyroman's review in particular for spec mechanics. If @kjmcnee has time his re-review would also be appreciated, but I think conceptually there's not too much delta from when he last looked at it; we just incorporated the src=""-changes-cause-reset which had previously been discussed. |
|
LGTM. This makes sense to me (obviously we still have to make corresponding impl changes). |
Still LGTM. Sorry for the delay. |
Closes #197 by causing all changes to
src=""to close the portal browsing context, and re-create it.Closes #227 by causing
activate()to delay promise resolution/rejection until the initial load in a new browsing context has finished.Old content
@jakearchibald, @kjmcnee, please take a look. Other reviewers also appreciated.Two main things to consider:
Is this the behavior we want? I know @jakearchibald was skeptical about the "After the initial load" behavior, but I think it's better, and I believe @kjmcnee indicated that it's implementable. The alternative is to delay activation like we do with initial load, but that seems like a worse user experience.
Is this explainer structure reasonable? I'm wondering if, e.g., we should move all discussion of activation failure from the bfcache section into here, or merge this with the bfcache section somehow. Also the flow feels a bit weird, in that we have to spend so much time up front explaining closed portals. But I couldn't find a better place to put it.
Preview | Diff