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] What does element.checkVisibility() return for display: contents elements? #9478

Closed
nt1m opened this issue Oct 17, 2023 · 3 comments
Labels
Closed as Question Answered Used when the issue is more of a question than a problem, and it's been answered.

Comments

@nt1m
Copy link
Member

nt1m commented Oct 17, 2023

The checkVisibility definition includes:

If this does not have an associated box, return false.

That means display: contents elements are considered as invisible. This seems like what Chrome implements, but the related WPT (https://github.com/web-platform-tests/wpt/blob/master/css/cssom-view/checkVisibility.html) does not explicitly test that. Just want to make sure it is an intentional choice.

cc @josepharhar @vmpstr

@nt1m
Copy link
Member Author

nt1m commented Oct 17, 2023

Looks like @Loirooriol raised this question originally but no one answered it: #6850 (comment)

@nt1m nt1m added the Agenda+ label Oct 18, 2023
@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [cssom-view] What does element.checkVisibility() return for display: contents elements?, and agreed to the following:

  • RESOLVED: Close no change, reopen if use cases are presented on the issue
The full IRC log of that discussion <emilio> ntim: right now checkVisibility will return that display: contents elements are not visible because they don't have a box
<emilio> ... which I find not very intuitive
<emilio> ... I don't know if that was intentional or not
<emilio> ... WPTs don't test for this
<emilio> q+
<astearns> ack emilio
<vmpstr> emilio: FF also returns this, not sure if it was intentional. I can see usecases. The spec is not ambiguous
<vmpstr> q+
<emilio> ntim: it's weird because display: contents is technically visible
<astearns> ack vmpstr
<emilio> astearns: it's children are anyways
<emilio> vmpstr: I don't think this was intentional
<dbaron> (leaving the meeting now)
<emilio> ... idea was to catch display: none and so
<emilio> ntim: my suggestion would be to check for the element or any flat tree ancestor having display: none
<vmpstr> emilio: seems ok-ish to me, but i also think there are use cases for making display:contents or considering it invisible. So maybe this should be opt in. I don't know
<noamr> q+
<vmpstr> ntim: you can see display contents elements
<vmpstr> emilio: you can see their children
<vmpstr> astearns: you don't see the box, it's background, just its children
<bradk> Bay Area just had a small earthquake
<astearns> ack noamr
<emilio> noamr: What is the behavior for an element with visibility: hidden that has children with visibility: visible? I'd expect this to be consistent
<emilio> emilio: It's consistent with that
<emilio> noamr: this is an invisible element with visible children, so it's consistent with that
<emilio> astearns: I'd suggest taking it back to the issue
<emilio> ... if there are use cases we can come back to this
<SebastianZ> +1 on Noam's points.
<emilio> ... but I don't see consensus
<emilio> vmpstr: use cases for making it visible?
<emilio> ntim: not necessarily, just a bit unintuitive
<emilio> ... don't care either way
<emilio> ... I guess for visibility we only check the visibility if checkVisibilityCSS option is
<emilio> ... specified
<emilio> astearns: suggested to close no change and add WPTs
<emilio> fantasai: if there are use cases for both behaviors can we make that flip?
<emilio> astearns: we didn't hear use cases
<emilio> fantasai: so point is to go back to the issue to request use cases?
<emilio> astearns: I was suggesting close no change and reopening if use cases are presented
<emilio> ... if you'd like to keep it open fine with that
<emilio> ntim: don't feel strongly, I could see web devs wanting to check if contents are visible
<emilio> ... but if we don't respect that for visibility then...
<emilio> RESOLVE: Close no change, reopen if use cases are presented on the issue
<emilio> RESOLVED: Close no change, reopen if use cases are presented on the issue
<emilio> ntim: should we add a note to the spec?
<emilio> astearns: We should def. have a WPT
<emilio> ... having it in the draft is an editorial decision
<ntim> my zoom is freezing, but I can add one
<emilio> vmpstr: I can add one
<fantasai> scribenick: fantasai

@nt1m
Copy link
Member Author

nt1m commented Oct 18, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closed as Question Answered Used when the issue is more of a question than a problem, and it's been answered.
Projects
None yet
Development

No branches or pull requests

2 participants