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

Firing "page exit" events on portal activation #225

Open
jakearchibald opened this issue Jul 7, 2020 · 7 comments
Open

Firing "page exit" events on portal activation #225

jakearchibald opened this issue Jul 7, 2020 · 7 comments
Labels
design work needed An acknowledged gap in the proposal that needs design work to resolve spec todo A nitty-gritty detail that needs spec work

Comments

@jakearchibald
Copy link
Collaborator

I'm looking to make portal activations more like navigations, and part of this would be to fire the pagehide, unload event etc etc before a portal can fully activate. Any objection to this?

If we do that, it seems like portal.activate() should fulfill in the same task as those events.

@jakearchibald jakearchibald added spec todo A nitty-gritty detail that needs spec work design work needed An acknowledged gap in the proposal that needs design work to resolve labels Jul 7, 2020
@domenic
Copy link
Collaborator

domenic commented Jul 7, 2020

To be clear, you'd be firing it on the page that's being "taken over"? If so, that makes sense to me.

@jakearchibald
Copy link
Collaborator Author

To be clear, you'd be firing it on the page that's being "taken over"?

Yep!

@domenic
Copy link
Collaborator

domenic commented Jul 7, 2020

I guess we might want to limit ourselves to just pagehide since unload/beforeunload are kind of deprecated? But maybe this isn't the right place to make that stand, and asymmetry would just be confusing...

Would we allow unload/beforeunload handlers to cancel the event? And in the beforeunload case, would canceling cause the prompt?

@jeremyroman
Copy link
Collaborator

unload doesn't fire on pages going into bfcache, right? So if the portal context isn't actually unloaded then presumably we should go no further than beforeunload.

@jakearchibald
Copy link
Collaborator Author

I'll double check, but I think unload does fire on pages going into the bfcache, although it isn't salvageable if there's a single unload listener.

Having said that, I think Safari does something different.

@kjmcnee
Copy link
Collaborator

kjmcnee commented Oct 7, 2020

haraken was wondering offline that since unload handlers are already unreliable, whether it would make sense to standardize portals to unconditionally not run unload handlers.

The three possibilities for the predecessor following activation are:

  1. adoption where it would not make sense to run unload handlers since the document is still alive
  2. bfcache which does not run unload handlers
  3. closing where it does make sense to run the unload handlers as this looks like a conventional navigation without bfcache

Of course, (3) assumes the user agent fires unload in the conventional case.

See https://docs.google.com/document/d/19n-DYAFzw1aIYPfVL07246Nf640QvSPUctUajI5hSgs/edit?usp=sharing for how bfcache interacts with unload handlers in chrome.
Further reading: https://www.igvita.com/2015/11/20/dont-lose-user-and-app-state-use-page-visibility/

It seems like it would be easier to reason about "portal activation never fires unload" over "portal activation fires unload unless the page is adopted or eligible for bfcache, or the user agent chooses not to." But as domenic suggested above, making that change would introduce inconsistency with conventional navigations.

beforeunload probably still makes sense to fire. Canceling the event would presumably cause the activate promise to reject. However if the behaviour described in the above link is still accurate, safari mobile might choose to not do that.

@domenic
Copy link
Collaborator

domenic commented Oct 12, 2020

We've since updated the spec to match Chrome and Safari, which do not fire unload for cases where a page goes into the bfcache: whatwg/html@b3b7add

I like the idea of never firing unload for portal activations. It seems to be on the way out; there's various work ongoing to deprecate it, in general.

However, we do need to be sure to fire pagehide/pageshow, since those are the recommended replacements.

I'm not a big fan of beforeunload either, but the tradeoff for breaking it is not as good, probably. So keeping it seems reasonable to me.

/cc @rakina

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 spec todo A nitty-gritty detail that needs spec work
Projects
None yet
Development

No branches or pull requests

4 participants