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

All src changes should reload the portal #197

Closed
jakearchibald opened this issue May 13, 2020 · 11 comments · Fixed by #253
Closed

All src changes should reload the portal #197

jakearchibald opened this issue May 13, 2020 · 11 comments · Fixed by #253
Assignees
Labels
spec todo A nitty-gritty detail that needs spec work

Comments

@jakearchibald
Copy link
Collaborator

const portalURL = 'https://example.com/';
const portal = document.createElement('portal');
document.body.append(portal);
portal.src = portalURL + '#someData';
// Then, later:
portal.src = portalURL + '#someMoreData';

If changing the hash part of the URL does not cause a reload, then it creates a data channel between the page and the portal.

We should ensure that all src changes reload the portal.

@jeremyroman
Copy link
Collaborator

Yeah, I think you're probably right that we should do this, at least for cross-origin. The consistency win might justify doing it for the same-origin case, too.

@domenic domenic added the spec todo A nitty-gritty detail that needs spec work label May 20, 2020
@domenic
Copy link
Collaborator

domenic commented Jun 17, 2020

This is already the case in the spec. (And also already the case for iframes, I think, otherwise OOPIFs would be impossible?)

So maybe this is more of a "test todo" than a "spec todo"?

@jeremyroman
Copy link
Collaborator

Why would OOPIFs be impossible if the parent document could initiate same-document navigations? As far as I know that is possible.

@domenic
Copy link
Collaborator

domenic commented Jun 17, 2020

Sorry, I was thinking that the child would synchronously see the result, but of course there's no way to do that...

However, from what I can tell the HTML spec goes through the full navigate algorithm for any assignment to src="" on iframes, and I can't see anywhere that bails out early if the URL has only changed by a fragment. But, tests in real browsers seem to show that they do bail out early...

@domenic
Copy link
Collaborator

domenic commented Jun 17, 2020

Nevermind, I was just being dumb. It's in https://html.spec.whatwg.org/#navigating-across-documents:scroll-to-fragid.

OK, so I think portals are affected, since they go through the navigate algorithm in https://wicg.github.io/portals/#htmlportalelement-set-the-source-url-of-a-portal-element.

@domenic
Copy link
Collaborator

domenic commented Jun 17, 2020

Should changing src="" just close the previous browsing context and create a new one? I'm not sure what impact that would have, but it seems a bit cleaner...

@domenic
Copy link
Collaborator

domenic commented Aug 25, 2020

Ping for opinions on whether src-changing should close the previous browsing context or not. I'm especially interested in hearing if that would be observable in any way... if it's not, I might just switch to that for simplicity.

@kjmcnee
Copy link
Collaborator

kjmcnee commented Aug 25, 2020

The main difference I can think of is whether the portal is activatable immediately following resetting the src.
e.g.
...
portal.src = portalURL + '#someMoreData';
portal.activate();
When reusing the context, we would activate the content before the reload. If we recreate the context, we could not activate.

In any case, creating a new context also seems cleaner to me.

As an aside, doing this consistently for same and cross origin would make it so that same document navigations for same origin cases (e.g. loading a template page, then populating it), would need to be done with a combination of postMessage and self-navigation, rather than navigation via portal.src. Consistency would certainly be nice, though I'm not sure how common either of these approaches are today with existing content to know whether this would be cumbersome.

@domenic
Copy link
Collaborator

domenic commented Aug 27, 2020

Hmm. So this kind of reduces to #227. That's tricky.

@jeremyroman
Copy link
Collaborator

I'm not sure I think fragment navigation is the natural way to do that anyway -- you're reasonably likely to want to history.replaceState to update the URI anyway. The consequence of when activate can be called is real, though.

@kjmcnee
Copy link
Collaborator

kjmcnee commented Aug 28, 2020

This sounds like it relates to options for the question of knowing when to call activate, such as leaving the activation promise in a pending state until the new portal context is ready for activation.

@domenic domenic self-assigned this Sep 28, 2020
domenic added a commit that referenced this issue Sep 28, 2020
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec todo A nitty-gritty detail that needs spec work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants