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 oncontentvisibilityautostatechange #10364

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vmpstr
Copy link
Member

@vmpstr vmpstr commented May 23, 2024

The contentvisibilityautostatechange event was added in CSSWG:
https://www.w3.org/TR/css-contain-2/#content-visibility-auto-state-changed

An issue was raised that rightly points out that oncontentvisibilityautostatechange should also be added in line with design principles

This PR does this.

(See WHATWG Working Mode: Changes for more details.)


/dom.html ( diff )
/indices.html ( diff )
/webappapis.html ( diff )

@vmpstr
Copy link
Member Author

vmpstr commented May 23, 2024

/cc @Loirooriol

@Loirooriol
Copy link
Contributor

This feature is implemented in all major browsers

Blink only seems to implement the IDL attribute, not the content attribute. Gecko doesn't seem to implement either.

Tests are written

These tests don't seem to cover oncontentvisibilityautostatechange as an IDL or content attribute.

@vmpstr
Copy link
Member Author

vmpstr commented May 27, 2024

Tests are written

These tests don't seem to cover oncontentvisibilityautostatechange as an IDL or content attribute.

I landed some changes to https://wpt.fyi/results/css/css-contain/content-visibility/content-visibility-auto-state-changed.html?label=master&label=experimental&aligned&q=content-visibility-auto-state to use the IDL and content attribute

This feature is implemented in all major browsers

Blink only seems to implement the IDL attribute, not the content attribute. Gecko doesn't seem to implement either.

I meant the ContentVisibilityAutoState change event, but I can see how the intent of the question is specific to the spec change, not to the feature that has these attributes.

/cc @emilio @nt1m to see if there is support

Copy link
Contributor

@emilio emilio left a comment

Choose a reason for hiding this comment

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

Seems fine if we want to keep doing on* attributes? I always found them a bit of an antipattern. Have we done this for other recent events?

@vmpstr
Copy link
Member Author

vmpstr commented Jun 20, 2024

It's part of the design principles right now: https://w3ctag.github.io/design-principles/#always-add-event-handlers so I figured it's still a pattern that we encourage. If that's not the case, then we should probably change the design principles and figure out if anything needs to be done with the existing on* attributes. I don't have a strong opinion either way

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

Yep, we don't want to remove this useful functionality for new events :)

@@ -142156,6 +142165,14 @@ INSERT INTERFACES HERE
<td> <code>SharedWorkerGlobalScope</code>
<td> Fired at a shared worker's global scope when a new client connects

<tr> <!-- contentvisibilityautostatechange -->
<td> <dfn event for="HTMLElement"><code data-x="event-contentvisibilityautostatechange">contentvisibilityautostatechange</code></dfn>
Copy link
Member

Choose a reason for hiding this comment

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

So actually we should remove this:

This means we'll add an xref to the definitions section pointing to https://drafts.csswg.org/css-contain-2/#eventdef-element-contentvisibilityautostatechange .

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

Successfully merging this pull request may close these issues.

None yet

4 participants