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

What happens if a portal is navigated during activation? #220

Open
jakearchibald opened this issue Jun 25, 2020 · 4 comments
Open

What happens if a portal is navigated during activation? #220

jakearchibald opened this issue Jun 25, 2020 · 4 comments
Assignees
Labels
design work needed An acknowledged gap in the proposal that needs design work to resolve

Comments

@jakearchibald
Copy link
Collaborator

This can happen via:

portal.activate();
portal.src = differentURL;

Or location.href = differentURL within the portal.

It isn't clear if these should even have the same solution. I think we decided in the past that setting the src on a portal is effectively like creating a new portal, as in it resets all portal state. Whereas a navigation within a portal is not in control of the outer page.

Options:


Setting src is ignored on a portal during activation. Creates a nasty race condition that developers can't really reason about.


Portals cannot have their src changed once activate() is called. I guess you'd need a property to indicate that the portal is in this state.


A new browsing context is created every time the src is changed on a portal. The previous context is closed, unless it's activating.

When activate() is called, it's the current browsing context that's activated. If the src is changed during activation, it works, but it's the previous context that continues to activate.

There might be some trickiness here, since the activating context seems detached from the document. Does that create the same problems as keeping iframes alive when they're outside the document?


As above, but when src is change, a new browsing context is only created if the current browsing context is activating.


Setting src aborts any ongoing activation associated with that portal.

cc @kjmcnee as you may have already solved this in the implementation.

@jakearchibald jakearchibald added the design work needed An acknowledged gap in the proposal that needs design work to resolve label Jun 25, 2020
@jeremyroman
Copy link
Collaborator

I think my instinct is that this should continue activating whatever browsing context was associated at that time, and setting src should do whatever we decide it should do (likely open a new browsing context separate from the the one being activated). But I'm not certain that's optimal.

@domenic domenic self-assigned this Sep 28, 2020
@domenic
Copy link
Collaborator

domenic commented Sep 28, 2020

#253 has a solution for location.href = differentURL within the portal. It also implements

A new browsing context is created every time the src is changed on a portal. The previous context is closed, unless it's activating.

but I don't think the consequences are exactly as described in the following parts of the OP.

In particular, activate() works in parallel. So given

portal.src = "https://example.com/";
portal.onload = () => {
  portal.activate();
  portal.src = "https://example.org/"; // (*)
};

The guest browsing context activation happens in parallel to the src setting, which would close that BC. This seems weird and racy and probably broken.

I think the desired behavior here is that activate() activates the https://example.com/ browsing context, which adopts the predecessor. That predecessor's active document contains a <portal> element, which per (*) got closed, and is starting to load a new https://example.org/ browsing context.

To implement this, I think we need to move some of the in-parallel parts of activate() earlier, to happen synchronously? If, by the time (*) is reached, we have already "promoted" the https://example.com/ browsing context out of the portal, so that the portal appears closed (i.e. has a null guest browsing context), then things will work as expected. So in particular I'm thinking maybe step 4 of https://pr-preview.s3.amazonaws.com/WICG/portals/pull/253.html#activate-a-portal-browsing-context ? (Which would require some alternate way of detecting closure in step 3, hmm...)

@jeremyroman, does this match your reading?


Another case, for comparison: no waiting for load. Given

portal.src = "https://example.com/"; // (1)
portal.activate();                   // (2)
portal.src = "https://example.org/"; // (3)

what will happen is pretty clear after #253:

  • (1) creates a new initially-empty browsing context and starts loading https://example.com/ in it.

  • (2) waits for the https://example.com/ browsing context to either load or lose its host element.

  • (3) closes the https://example.com/ browsing context and creates a new https://example.org/ one, so the being-activated https://example.com/ browsing context loses its host element.

So activate() rejects.

@jeremyroman
Copy link
Collaborator

I think my preferred mental model is consistent with yours -- the activate pertains to the contents at the time. I think it is defensible for a subsequent assignment to src to racily cancel the activation as part of closing the associated browsing context, but the subsequently assigned URL (example.org) should not activate.

Yeah, I could see lifting the first few steps out of the "in parallel" block.

@kjmcnee
Copy link
Collaborator

kjmcnee commented Oct 2, 2020

Agreed with the observed behaviour of setting src following a call to activate:

  • creating a new context and the activation of the first context proceeding if the activation would succeed without needing to defer
  • racing with activation if activation is deferred

The same behaviour of preceding with activation if it would immediately succeed seems appropriate for similar cases like activating, then removing the portal element.

I'm not sure what the best way to express this in spec land is, but I'm a bit nervous about describing these as synchronous. I could imagine keeping track of a "pending activation browsing context" which would cause us to not close the context when performing other operations. This would only need to last until we receive word that we're immediately succeeding, immediately failing, or deferring activation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design work needed An acknowledged gap in the proposal that needs design work to resolve
Projects
None yet
Development

No branches or pull requests

4 participants