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

[css-contain-2] Specify that the first content-visibility: auto visibility check happens immediately #5439

Merged
merged 1 commit into from
Aug 20, 2020

Conversation

vmpstr
Copy link
Member

@vmpstr vmpstr commented Aug 17, 2020

When a new content-visibility: auto element is added, the check to determine
its visibility needs to occur in the same frame to avoid a flash of blank content.

This PR adds a clarification to this effect.

@vmpstr
Copy link
Member Author

vmpstr commented Aug 17, 2020

/cc @tabatkins @chrishtr

@@ -1292,7 +1292,20 @@ Restrictions and Clarifications {#cv-notes}
and not cause any forced layouts.
</div>

4. For the purposes of scrolling operations,
4. The initial determination of visibility for ''content-visibility: auto'' must
happen in the same frame that determined an existence of a new
Copy link
Contributor

Choose a reason for hiding this comment

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

What about nested content-visibility: auto ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm. I think this is a good question. I would say that the wording stands: if the nested auto is the first time we're evaluating visibility then it should happen in the same frame.

The problem here is that we want to avoid a flash of blank. It's different if this was off-screen and then enters the screen in which case there are other mechanisms to prevent the flash (margin)

WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

Right now your implementation uses a loop in the update-the-rendering steps that is equivalent to a ResizeObserver, but stops after one loop. To implement the semantics you're suggesting we'd have to have a loop like this:

while (true) {
  updateStyleAndLayout();
  if (!thereAreNewlyDiscoveredContentVisibilityAutoElements())
    break;
  deliverIntersectionObservationsToNewlyDiscoveredContentVisibilityAutoElements();
}

As implemented above this will have at most as many while loops as there are nestings incontent-visibility: auto.

I think this is fine actually; the looping semantics and code structure are exactly the same as ResizeObserver, except the observer is a special sync-on-first-frame IntersectionObserver callback. (Note that ResizeObserver also has a don't-infinite-loop requirement that it goes deeper into the DOM on each while loop iteration, which is similar to the invariant above about nested content-visibility:auto.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that's pretty much the intent (and how it works in Chrome)

@tabatkins do you mind taking a quick look as well?

Copy link
Member

Choose a reason for hiding this comment

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

This seems fine to me from a spec perspective; the detail of implementation there is above me.

@@ -1292,7 +1292,20 @@ Restrictions and Clarifications {#cv-notes}
and not cause any forced layouts.
</div>

4. For the purposes of scrolling operations,
4. The initial determination of visibility for ''content-visibility: auto'' must
happen in the same frame that determined an existence of a new
Copy link
Member

Choose a reason for hiding this comment

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

This seems fine to me from a spec perspective; the detail of implementation there is above me.

@frivoal
Copy link
Collaborator

frivoal commented Dec 16, 2020

resolution to accept this change recorded here: https://logs.csswg.org/irc.w3.org/css/2020-12-16/#e1381652

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

Successfully merging this pull request may close these issues.

4 participants