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

Add notes about ordering and other specs's steps in "reactivating a document" and "unloading document cleanup steps" #8897

Merged
merged 5 commits into from
Mar 24, 2023

Conversation

rakina
Copy link
Member

@rakina rakina commented Feb 16, 2023

The "reactivating a document" steps will be run when a document regains full activity after restoring it from BFCache. This PR adds a note to those steps allows us to make the recommendation to listen to fully active state changes in the BFCache guide better.

Note that the former only covers the fully active -> fully active listener for BFCache cases, and not the fully active -> non fully active change. The latter case can already be implemented by adding "unloading document cleanup steps" (and to filter out documents that will be destroyed, the specs can check for the salvageable state). This PR also adds a comment to that function regarding the ordering, as mentioned in #8897 (comment) and #8906.

cc @annevk @domenic @fergald @smaug----

  • At least two implementers are interested (and none opposed):
    • Chromium
    • WebKit

/browsing-the-web.html ( diff )
/document-lifecycle.html ( diff )
/references.html ( diff )

These steps will be run when a document regains full activity after restoring it from BFCache. This does not currently introduce any behavior changes within the HTML spec, but allows other specs to define the steps they will take when a document becomes fully active again, e.g. send updates to the renderer about the current Geolocation state, etc.

Fixes whatwg#8872.
@domenic
Copy link
Member

domenic commented Feb 17, 2023

Apologies for not participating in the discussion in #8872.

Although allowing other specs to hook into these parts seems good, I think these days we are trying to shy away from generic hooks other specs can override, because such hooks make the ordering indeterminate (which is very observable for cases like this, where e.g. events are fired.)

Instead, our modern preference is to insert specific steps into HTML that call out to the other specs, and as part of that require tests on the relative ordering, etc. You can see work in that direction for a very similar setup in #8501 . Indeed, the "update the visibility state" algorithm which you moved around as part of this PR touches on that work:

It would be better if specification authors sent a pull request to add calls from here into their specifications directly, instead of using the page visibility change steps hook, to ensure well-defined cross-specification call order. As of the time of this writing the following specifications are known to have page visibility change steps, which will be run in an unspecified order: Device Posture API and Web NFC.

So I think the bfcache guide should encourage people to send PRs calling into their specs, and tell them exactly which algorithm in HTML they need to modify. It should not encourage people to define "regaining document full activity steps".

(I'll open a separate issue to track applying the same process to "unloading document cleanup steps".)

Add notes about ordering in reactivating the Document and unloading document cleanup steps
@rakina
Copy link
Member Author

rakina commented Feb 28, 2023

Thanks @domenic! Yeah as I was writing this PR the ordering problem did make me a bit uncomfortable.. encouraging other specs to add into a list of clearly ordered steps in the HTML spec makes sense to me. I've changed this PR to add notes to both "reactivating a document" and "unloading document cleanup steps" instead.

@rakina rakina changed the title Add "regaining document full activity steps" Add notes about ordering and other specs's steps in "reactivating a document" and "unloading document cleanup steps" Feb 28, 2023
source Show resolved Hide resolved
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So sorry for the long delay here. This looks great, and thanks for tackling adding a note about #8906 as well. Let's just finish off the reference list.

source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
@domenic domenic merged commit e9bb6cf into whatwg:main Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants