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

[cssom-view] API to expose skipped content state #9474

Closed
nt1m opened this issue Oct 16, 2023 · 5 comments · Fixed by #9549
Closed

[cssom-view] API to expose skipped content state #9474

nt1m opened this issue Oct 16, 2023 · 5 comments · Fixed by #9549
Labels
css-contain-2 Current Work

Comments

@nt1m
Copy link
Member

nt1m commented Oct 16, 2023

Currently there is no straightforward way to query the skipped content state, despite browsers having this info internally.

In the context of web extensions, it would be useful if you could query that state without invoking the layout of the element, which takes away from the content-visibility optimizations.

@vmpstr @rwlbuis

@nt1m nt1m added css-contain-2 Current Work Agenda+ labels Oct 16, 2023
@Loirooriol
Copy link
Contributor

Some past discussion in #6850 (comment)

@nt1m
Copy link
Member Author

nt1m commented Oct 16, 2023

Perhaps it should just be an extension to checkVisibility to check for c-v: auto skipped content

@vmpstr
Copy link
Member

vmpstr commented Oct 17, 2023

I think this is a pretty good idea. We currently have

  • The Contentvisibilityautostatechange event which is nice to checking when content-visibility: auto switches state
  • The checkVisibility function that Oriol mentioned.

I don't know if we want checkVisibility to specifically check for c-v auto skipped content, since that content is still visible to a lot of features like find-in-page and accessibility. So saying it's not visible in checkVisibility is a bit misleading.

I wouldn't be opposed to a new function though.

/cc @josepharhar

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-contain-2] API to expose skipped content state, and agreed to the following:

  • RESOLVED: Extend checkVisibility to expose content-visibility: auto state
The full IRC log of that discussion <emilio> ntim: in Safari we have some reader detection code that does some visual analysis and performs layout in different subtrees
<emilio> ... we want an API to be able to skip hidden content for visual analysis
<emilio> ... because otherwise it cancels out optimizations
<vmpstr> q+
<emilio> ... ideally it'd cover content-visibility: auto and hidden
<emilio> q+
<astearns> ack khush
<emilio> ack khush
<astearns> ack vmpstr
<emilio> vmpstr: checkVisibility was close to cover this use case
<emilio> ... it deals with c-v: hidden
<emilio> ... I'd suggest extending checkVisibility
<emilio> ... content-visibility: auto content remains accessible to find-in-page / tab-navigation / etc so it's not "hidden hidden"
<emilio> ... so not sure if it'd be consistent with the model
<emilio> ntim: you can kinda query it with the statechange event
<emilio> ... but it's less convenient and you might miss some events
<astearns> ack emilio
<miriam> scribe+
<miriam> emilio: not opposed
<emilio> ... and I agree with vmpstr on extending checkVisibility
<emilio> ... but curious about the use case because it seems reader-mode should care about content-visibility: auto
<miriam> emilio: to check size you need to layout
<emilio> ntim: so we have a check for the element being small and skipping the content
<emilio> ... and the rest of the code doesn't care about layout
<emilio> ... but actually slows down the website because it lays out the skipped subtrees
<vmpstr> q?
<vmpstr> q+
<emilio> ntim: could be a separate method on checkVisibility even
<miriam> emilio: The element could be large, and you need to lay it out
<miriam> … kinda silly not to expose this
<emilio> ... it's a bit weird that with the statechange event being async you can observe an element being hidden before the state change arrives
<emilio> ... might cause some subtle bugs
<emilio> ... but anyhow it seems fine to expose
<emilio> q?
<astearns> ack vmpstr
<emilio> vmpstr: just to answer emilio, I don't think it's unique to this
<emilio> ... intersectionobserver has the same issue, where you could do your own math via gBCR
<emilio> ... I'd like to suggest not reusing checkVisibility for this
<emilio> q+
<miriam> emilio: why?
<emilio> vmpstr: to me the problematic word there is visibility
<emilio> ... because if you consider things like a11y it's there
<emilio> ... so claiming it's not visible is not entirely accurate
<emilio> ... visibility is a little overloaded
<astearns> ack emilio
<miriam> emilio: but that's true of all the other things CV checks
<miriam> ... things with opacity 0 are still in the visibility tree
<miriam> … as long as you opt in for content visibility auto, seems like you need to check that anyway
<emilio> vmpstr: I guess opacity would be similar to this, yeah, the other's won't
<emilio> astearns: seems there's consensus on exposing this, but not in whether to change checkVisibility or something else
<emilio> ... might be worth resolving on that and discussing the other thing in the issue
<emilio> fantasai: was going to say the same thing as emilio, checkVisibility is already a misnomer and was designed with this extension in mind
<emilio> ntim: I don't feel strongly either way
<vmpstr> +1
<emilio> RESOLVED: Extend checkVisibility to expose content-visibility: auto state

@AutoSponge
Copy link

APA will track this issue. We want to ensure that elements not rendered (e.g., display:none) but that could still end up in the AxTree (e.g., targeted by aria-describedby) are not conflated by developers to be "skipped" for AT.

webkit-commit-queue pushed a commit to nt1m/WebKit that referenced this issue Oct 29, 2023
…ent.checkVisibility()

https://bugs.webkit.org/show_bug.cgi?id=263352
rdar://117177342

Reviewed by Simon Fraser.

As resolved in w3c/csswg-drafts#9474 (comment)

Add a checkContentVisibilityAuto option to element.checkVisibility().

* LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/checkVisibility.html:
* Source/WebCore/dom/CheckVisibilityOptions.h:
* Source/WebCore/dom/CheckVisibilityOptions.idl:
* Source/WebCore/dom/Element.cpp:
(WebCore::Element::checkVisibility):

Canonical link: https://commits.webkit.org/269903@main
@nt1m nt1m changed the title [css-contain-2] API to expose skipped content state [cssom-view] API to expose skipped content state Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css-contain-2 Current Work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants