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 "Supporting BFCached Documents" standalone guide #1

Merged
merged 3 commits into from
Feb 7, 2023

Conversation

rakina
Copy link
Collaborator

@rakina rakina commented Dec 6, 2022

@rakina
Copy link
Collaborator Author

rakina commented Jan 11, 2023

Post-holiday ping :) I've just updated the README now as requested. The guide can already be read at https://w3ctag.github.io/bfcache-guide/ but I made this PR in case there are some suggestions from TAG/other reviewers.

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

I'm missing some guidance around "in parallel".

At least abstractly it seems that whenever you do "in parallel" followed by "queue a global task" there's an opportunity for a document to become inactive.

However, HTML will block task processing in that case until the document is active again so you can't really know.

I'm also vaguely reminded of an issue around promises here.

E.g., if we consider https://storage.spec.whatwg.org/#dom-storagemanager-persisted. Should that just work as-is or will any changes be needed?

index.bs Outdated Show resolved Hide resolved
APIs that hold non-exclusive resources
may be able to release the resource when the document becomes not fully active,
and re-acquire them when it becomes [=Document/fully active=] again
(Screen Wake Lock API is already <a href="https://w3c.github.io/screen-wake-lock/#handling-document-loss-of-full-activity">doing</a> the first part).
Copy link
Member

Choose a reason for hiding this comment

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

This is pretty poor example in that there's no explicit hook tying this to some algorithm that runs when the fully active state changes. Are there hooks for that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately we don't have hooks for "fully active" state changes. Maybe we should add that? Do you know what's the best way to do that?

Copy link
Member

Choose a reason for hiding this comment

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

I suspect we want something similar to HTML's "update the rendering" where specifications have an algorithm that HTML ends up invoking when it changes the state.

Choose a reason for hiding this comment

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

These are all valid points though not necessarily strong enough to hold back the forking of this document. My suggestion is to open separate issues against the newly forked BFCache document.

Copy link
Member

Choose a reason for hiding this comment

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

I find that a bit rude. I was asked for feedback here and only now you're telling me I should have been filing issues instead.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @atanassov, I think the merge of this PR wasn't urgently needed. The HTML file of the bikeshed file is already part of the gh-pages branch so the document was already accessible from https://w3ctag.github.io/bfcache-guide/. I had intended to merge the reviewed version of the document to that branch after all the issues are resolved and this got LGTMs from other browsers.

@annevk, I'm very sorry for the delay in addressing the reviews here and the sudden merge, I assure you that I will address the issues and update the guide. I have filed an issue for the "in parallel" one, and another issue for the "fully active" state change listener. Thanks a lot for reviewing this PR and all the help!!

@annevk
Copy link
Member

annevk commented Jan 13, 2023

(Also, thanks for pushing this! It's great that we give this some more attention.)

@rakina
Copy link
Collaborator Author

rakina commented Jan 23, 2023

Thanks for the review!

I'm missing some guidance around "in parallel".

At least abstractly it seems that whenever you do "in parallel" followed by "queue a global task" there's an opportunity for a document to become inactive.

However, HTML will block task processing in that case until the document is active again so you can't really know.

I'm also vaguely reminded of an issue around promises here.

E.g., if we consider https://storage.spec.whatwg.org/#dom-storagemanager-persisted. Should that just work as-is or will any changes be needed?

Yes, I think tasks that are posted might not actually be run until the document becomes active again (or if the document gets evicted, never run at all). Maybe we can add a section that mentions this explicitly just so that people are aware that this is possible? They will not run when the document is inactive, so hopefully there's no problems with things thinking it's an inactive document in a posted task? (I'm not really sure what the problem might be in your example)

@annevk
Copy link
Member

annevk commented Jan 27, 2023

Maybe the better example would be the persist() method in the same document. It's rather unclear whether or not the state ends up changing or the user gets prompted (or is still prompted when going back if they had not answered yet) depending on when the fully active transition happens. I mean, there is a certainly a way to read it that answers these questions, kinda, but I doubt it's accurate.

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

Successfully merging this pull request may close these issues.

4 participants