Skip to content

[css-contain] Should scrollIntoView scroll to content-visibility: hidden subtree elements #6529

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

Closed
vmpstr opened this issue Aug 19, 2021 · 4 comments

Comments

@vmpstr
Copy link
Member

vmpstr commented Aug 19, 2021

content-visibility: hidden property skips its contents and notably says that they shouldn't be available to user agent features like find-in-page nor should they be focusable and selectable.

It also says that if the layout work is done, then the user-agent should retain as much of that state as possible in order to make revealing the contents faster.

scrollIntoView() says (in step 6) that as long as the element has a layout object, we scroll it into view.

The problem is that for content-visibility: hidden contents, we may have a layout object or we may not, depending on whether rendering work as ever been done in that subtree. This means that the behavior of scrollIntoView is inconsistent.

I propose that we call out scrollIntoView explicitly in content-visibility section under hidden and say that calling scrollIntoView on content-visibility: hidden subtrees has no effect (or something similar)

/cc @tabatkins @frivoal

@tabatkins
Copy link
Member

tabatkins commented Aug 20, 2021

I agree; we shouldn't let this be non-deterministic.

@vmpstr
Copy link
Member Author

vmpstr commented Aug 24, 2021

I think this may be a little more subtle than I first described. The question is do these hidden elements have a layout box? Well, we do want getBoundingClientRect to compute the right value for them since one of the use cases is to measure layout in a hidden subtree. So, when we call getBoundingClientRect we may update the rendering and generate a layout box.

However, we don't want to do this for fragment link navigation (or any user controlled behavior), since these elements are not meant to be exposed to the user in any way. Fragment link algorithm, step 3.4 uses scroll into view though, so we kind of don't want scroll into view to do anything whether or not there is a layout box generated here.

So the initial proposal is still the same: scroll into view or any scroll into view reliant algorithm has no effect on content-visibility hidden subtree, but the reasoning I think is a bit more subtle

@chrishtr
Copy link
Contributor

The question is do these hidden elements have a layout box?

Yes, the content below a content-visibility: hidden element have boxes. This is very important to support the cached-rendering and measurement use cases.

I agree that we should make scrollIntoView do nothing for hidden elements, similar to how fragment navigations do nothing. The spec could say, in step 6, "If the element does not have any associated layout box, or is not available to user-agent features, then return.". The "not available..." copies the spec text that is already here.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed scrollIntoView() with content-visiblity:hidden, and agreed to the following:

  • RESOLVED: c-v:hidden prevents scrollIntView() and similar functionality from working on the subtree
The full IRC log of that discussion <TabAtkins> Topic: scrollIntoView() with content-visiblity:hidden
<TabAtkins> github: https://github.com//issues/6529
<leaverou2> Fine by me to bump, that gives me more time to join the call for the nesting issue too
<TabAtkins> vmpstr: We have content-visiblity:hidden that hides the content of the subtree, and spec says that the contents should not be available to UA features like focus, find-in-page, etc
<TabAtkins> vmpstr: I think one of the features it should be restrited from is fragment nav
<TabAtkins> vmpstr: I don't want to scroll into the hidden content
<bradk> Is content-visibility in a draft?
<TabAtkins> vmpstr: But the current fragment nav text points to scrollIntoView() for defining it. and sIV() says we scroll as long as there's a layout box
<TabAtkins> vmpstr: My hope is we can prevent sIV() for c-v:hidden subtrees
<astearns> bradk: https://drafts.csswg.org/css-contain-2/#content-visibility
<TabAtkins> vmpstr: I think last comment from Chris is that we should change sIV() to say "if it doesn't ahve a layout box, or is not available to UA features, return"
<bradk> @astearns thanx
<TabAtkins> TabAtkins: I support this
<chris> rrsagent, here
<RRSAgent> See https://www.w3.org/2021/08/25-css-irc#T16-10-24
<leaverou2> Does that mean we also don’t scroll to disabled form controls since they can’t be focused?
<TabAtkins> vmpstr: To be specific we gen the layout box in the subtree when we *need* to, such as to getBoundingClientRect() on the subtree
<TabAtkins> vmpstr: So the layout box *is* theoretically there, but it's generated on demand. We just don't want that to happen here.
<TabAtkins> Rossen_: Can we get a tighter definition for "available to UA features"? It's very broad
<TabAtkins> Rossen_: What would be the better scoped version of that? Is it just for focusing?
<chrishtr> q+
<TabAtkins> vmpstr: I think it should be "behaving similar to display:none", it's not just focus
<TabAtkins> vmpstr: display:none *also* doesn't have a layout box, but this does
<TabAtkins> chrishtr: We can scope it by "if you ahve a content-visiblity:hidden ancestor"
<TabAtkins> Rossen_: Okay so if it's hidden or has an ancestor
<TabAtkins> chrishtr: Only ancestor. If the element itself has it, it's still there on the page like normal
<leaverou2> But ancestor can’t be overridden by another ancestor lower down
<leaverou2> s/can’t/can/
<leaverou2> Perhaps if the computed value of the parent is c-v:hidden?
<TabAtkins> TabAtkins: I think best is to define the term somewaht generally, and say that c-v:hidden is the only way to trigger the term currently
<Rossen_> q?
<TabAtkins> Rossen_: Sounds reasonable
<Rossen_> ack chrishtr
<leaverou2> Agreed as well
<TabAtkins> Rossen_: Any more comments?
<TabAtkins> RESOLVED: c-v:hidden prevents scrollIntView() and similar functionality from working on the subtree

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

No branches or pull requests

5 participants