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 should the ordering of freeze/resume events be w.r.t. pagehide/pageshow for bfcached pages? #7

Open
philipwalton opened this issue Apr 26, 2018 · 11 comments

Comments

@philipwalton
Copy link

philipwalton commented Apr 26, 2018

The current ordering of events for visible in the unload process (that will be put into bfcache) is as follows:

  1. beforeunload
  2. pagehide
  3. visibilitychange

And when coming out of bfcache there's just a single event:

  1. pageshow

Assuming that bfcache-supporting browsers are going to also fire the freeze and resume events when pages go into and out of the bfcache, what should the order be?

My proposal would be this for the unload case:

  1. beforeunload
  2. pagehide
  3. visibilitychange
  4. freeze

And this for the resume case:

  1. resume
  2. pageshow

Having freeze fire after visibilitychange is consistent with our model that the FROZEN state follows the HIDDEN state.

NOTE: one catch here is that currently pagehide fires before visibilitychange in the unload flow, which isn't consistent with our state progression.

If we have to keep the current ordering of pagehide and visibilitychange for historical/compat reasons, then I think the above proposed ordering makes the most sense. On the other hand, if we can change the current ordering to better match our state progression, then I'd suggest this order:

  1. beforeunload
  2. visibilitychange
  3. pagehide
  4. freeze

And this for the resume case:

  1. resume
  2. pageshow
@spanicker
Copy link
Collaborator

I would also prefer the second ordering.
I don't expect compat issues and @toddreifsteck also seemed to agree (I believe pagevisibility firing there was added relatively recently)
@smaug---- any thoughts on this?

@annevk
Copy link

annevk commented May 2, 2018

I think this shows that the actions described in https://wicg.github.io/web-lifecycle/spec.html#additions-to-web-lifecycle-spec need to be written down somewhere (if they haven't been already). It would then be much easier to see how dispatching this event slots into those algorithms.

(The thing I was missing primarily is the task source for these actions, but I guess freeze/resume will just fire in the same task as whatever caused the switch?)

@spanicker
Copy link
Collaborator

Could you clarify the comment about "need to be written down somewhere"?
Happy to update / write documentation as needed, but need to understand what is missing.

BTW @philipwalton is working on the comprehensive diagram with (proposed) state transitions here:
https://docs.google.com/drawings/d/176gNTdF6fTf4UFrzGjGClV6fSpkyOvqxSclglsn1c_Y/edit

@spanicker
Copy link
Collaborator

Minor update in the suggested order:
beforeunload
visibilitychange
pagehide
freeze (as no further callbacks can run after freeze, as we are freezing the task queues)

And this for the resume case:
resume
pageshow

@annevk
Copy link

annevk commented May 4, 2018

Let's see... There's a set of user agent actions. Those actions queue a task (with a specified task source) to change the state of the document. That task then changes states, dispatches one or more events (and potentially does things based on how that dispatching goes), and maybe does some other things.

So aside from the events, and the relative event order, we also need to know the tasks, the task sources, and the overall description of the actions. Does that make sense?

@philipwalton
Copy link
Author

Per @spanicker's comment #7 (comment), I've updated my original comment to have this order be our ideal order, where freeze fires last.

@smaug----
Copy link

smaug---- commented May 4, 2018

freeze as last and resume as first event at least looks reasonable.
Firing visibilitychange earlier than currently doesn't sound too scary, but could we do that in a separate spec issue - I mean, it can be changed way before the rest of the lifecycle stuff is sorted out.

@rakuco
Copy link

rakuco commented Jan 20, 2021

I'd like to draw attention to this issue. AFAICS, the spec is currently saying the "freeze" event must be fired before both pagehide and visibilitychange, which essentially prevents those events from being fired when the page is being unloaded.

This affects specs like Screen Wake Lock, which queues a "release" event based on page visibility changes, as seen in this Chromium discussion.

@rakuco
Copy link

rakuco commented Jan 20, 2021

which essentially prevents those events from being fired when the page is being unloaded

I can see these events being fired in Blink, but I'm not sure if this is by design or not. In any case, other specifications like Screen Wake Lock that queue events on document visibility change do seem to be affected.

@rakina
Copy link
Member

rakina commented Jan 21, 2021

I think the Page Lifecycle spec text might be out of date, because it's referencing specific step numbers and we've seen some changes in that part of the spec recently. I think we need to either integrate this spec into the HTML spec (with the correct ordering - firing freeze last) or expose something like unloading document visibility change steps - I'm not sure which one's better.

@domenic
Copy link
Collaborator

domenic commented Jan 21, 2021

Integration into HTML would be ideal, but I'm not sure Page Lifecycle has multi-implementer interest, so it might not be possible. The best we could do in that situation would be to update the step number references to instead talk about the contents of the steps, similar to stuff like https://jeremyroman.github.io/alternate-loading-modes/#navigate-activation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants