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] Incomplete event definition for ContentVisibilityAutoStateChanged #7603

Open
tidoust opened this issue Aug 14, 2022 · 7 comments
Labels
css-contain-2 Current Work

Comments

@tidoust
Copy link
Member

tidoust commented Aug 14, 2022

Spec says that a ContentVisibilityAutoStateChanged event gets fired at an element. But ContentVisibilityAutoStateChanged is the name of the interface used by the event, not the name of the event itself. What is the name of the event that fires at the element?

Ideally, the spec would use the fire an event concept defined in DOM with wording similar to:

When the rendering state of an element with content-visibility: auto style changes and the element either becomes or stops being relevant to the user, the user agent must fire an event named contentvisibilityautostatechange at the Element using ContentVisibilityAutoStateChanged.

Also, if you're firing a new event on the Element interface, that interface should be extended to add an onevent event handler IDL attribute to meet the Always add event handlers design principle.

@tidoust tidoust changed the title [css-contain-2] Incomplete event definition for `ContentVisibilityAutoStateChanged [css-contain-2] Incomplete event definition for ContentVisibilityAutoStateChanged Aug 14, 2022
tidoust added a commit to w3c/webref that referenced this issue Aug 14, 2022
The `ContentVisibilityAutoStateChangedEvent` event interface is defined without
any corresponding event type or event handler IDL attribute. This adds the
interface to the list of expected failures pending resolution of
w3c/csswg-drafts#7603
tidoust added a commit to w3c/webref that referenced this issue Aug 16, 2022
The `ContentVisibilityAutoStateChangedEvent` event interface is defined without
any corresponding event type or event handler IDL attribute. This adds the
interface to the list of expected failures pending resolution of
w3c/csswg-drafts#7603
tidoust added a commit to tidoust/csswg-drafts that referenced this issue Sep 14, 2022
A [recent commit](w3c@c3d88b2)
improved the definition of the event somehow but the link between the
`contentvisibilityautostatechanged` event and the
`ContentVisibilityAutoStateChanged` is not specified anywhere. This update
switches to the usual phrasing used across specs for firing events to make the
link between the event type and the interface explicit.

Also, Apart from a few exceptions in old specs, events are always named with
verbs in the present tense. Accordingly, this update switches the event type to
`contentvisibilityautostatechange` instead of
`contentvisibilityautostatechanged` and also renames the IDL interfaces. For
what it's worth, this guidance does not yet appear in the TAG's API design
principles but is considered for inclusion in:
  w3ctag/design-principles#280

This partially addresses concerns raised in w3c#7603. The spec would still need to
to define an `oncontentvisibilityautostatechange` event handler IDL attribute,
which could perhaps be done by extending the `GlobalEventHandlers` mixin (to
target `HTMLElement`, `MathMLElement` and `SVGElement` all at once) or by
working with the WHATWG to integrate the change in HTML directly.
@Loirooriol Loirooriol added the css-contain-2 Current Work label Nov 3, 2022
@Loirooriol
Copy link
Contributor

Blink already shipped and Gecko is implementing, so if it has to be renamed, better do it soon.

@Loirooriol
Copy link
Contributor

I mean, this is not clear from the top comment, but as stated in #7740

Apart from a few exceptions in old specs, events are always named with verbs in the present tense. Accordingly, this update switches the event type to contentvisibilityautostatechange instead of contentvisibilityautostatechanged and also renames the IDL interfaces. For what it's worth, this guidance does not yet appear in the TAG's API design principles but is considered for inclusion in w3ctag/design-principles#280

In particular, there is the change event which is not called changed.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-contain-2] Incomplete event definition for ContentVisibilityAutoStateChanged.

The full IRC log of that discussion <fantasai> Topic: [css-contain-2] Incomplete event definition for ContentVisibilityAutoStateChanged
<fantasai> github: https://github.com//issues/7603
<fantasai> oriol: This issue covers different things, but only want to discuss name of the event
<fantasai> oriol: contain introduced ContentVisiblityAutoStateChanged event
<fantasai> oriol: this is inconsistent naming, since other events they use the present tense and this is using past tense
<fantasai> oriol: there's even a proposed design principle in TAG about naming events about avoiding past tense
<fantasai> oriol: just some legacy events are using past tense
<fantasai> oriol: So should we align with this design principle?
<fantasai> oriol: This feature has already shipped in Blink in July or such
<fantasai> oriol: but maybe it could still be compatible to change or maybe Blink could support both for awhile or something like that
<fantasai> oriol: Firefox is implementing now, and WebKit has not implemented this property yet
<fantasai> florian: Do you want to change the name of the event and the object type or just the event?
<vmpstr> q+
<fantasai> oriol: It should be consistent
<flackr> I *think* this is the use counter for registrations: https://chromestatus.com/metrics/feature/timeline/popularity/4328
<emilio> ack emilio
<vmpstr> https://chromestatus.com/metrics/feature/popularity#ContentVisibilityAutoStateChangedHandlerRegistered
<Rossen_> ack vmpstr
<fantasai> vmpstr: On Chrome status there is 0.048% that register a handle for this event
<fantasai> vmpstr: it's a low usage but it's non-zero
<fantasai> Rossen_: is that above the threshold? Wasn't it 0.03%
<fantasai> vmpstr: Justification is that the state change has already happened, different from click event where you can preventDefault
<fantasai> vmpstr: but that's why it was named in the past tense
<flackr> Also animationstart, animationend, transitionstart, transitionend
<fantasai> oriol: form controls also have change event for afterwards, can't prevent change, and this event is in present
<emilio> Yeah, lots of precedents there, `visibilitychange` / `pageshow` / etc
<emilio> +1 for changing
<fantasai> Rossen_: Can we resolve to change, and if we get feedback strongly that this will be a problem, we can revisit?
<fantasai> Rossen_: from spec point of view, right thing to resolve is in favor of change
<fantasai> +1
<vmpstr> +1
<Rossen_> q?
<fantasai> RESOVLED: Change to present tense for ContentVisiblityAutoStateChanged (event and object)

myakura added a commit that referenced this issue Feb 7, 2023
…tVisibilityAutoStateChange`

Per #7603 (comment) the group resolved to use present tense for the event and inteface.
Since then Mozilla and Chrome both implemented the interface under the new name.
https://hg.mozilla.org/mozilla-central/rev/6095dabf5574
https://chromium.googlesource.com/chromium/src/+/9851571ecb54c47a205b2321006577b9799c0ea3

This updates both the event name and interface to reflect the resolution and to match implementation.
myakura added a commit to myakura/csswg-drafts that referenced this issue Feb 7, 2023
…tVisibilityAutoStateChange`

Per w3c#7603 (comment) the group resolved to use present tense for the event and interface.

Since then, Mozilla and Chrome both implemented the interface under the new name.
https://hg.mozilla.org/mozilla-central/rev/6095dabf5574
https://chromium.googlesource.com/chromium/src/+/9851571ecb54c47a205b2321006577b9799c0ea3

This updates both the event name and interface to reflect the resolution and to match implementation.
myakura added a commit to myakura/mdn-content that referenced this issue Feb 7, 2023
…ilityautostatechange

Per w3c/csswg-drafts#7603 (comment) CSSWG has resolved to change the event and interface name to use present tense "contentvisibilityautostatechange".
This patch just move files to the new directionary.
myakura added a commit to myakura/mdn-content that referenced this issue Feb 8, 2023
…oStateChangeEvent

The CSSWG has resolved to change the event and interface name to use present tense "contentvisibilityautostatechange".
w3c/csswg-drafts#7603 (comment)
@svgeesus
Copy link
Contributor

Due to a typo, this is not marked as resolved. From the minutes:

RESOVLED: Change to present tense for ContentVisiblityAutoStateChanged (event and object)

should be

RESOLVED: Change to present tense for ContentVisiblityAutoStateChanged (event and object)

@svgeesus
Copy link
Contributor

Also due to the merged PRs, is this still really Needs Edits?

Pushing on this because RESOLVED: Accept the PR and republishing WD of containment 2

@vmpstr
Copy link
Member

vmpstr commented May 23, 2024

I believe this can be closed

@vmpstr vmpstr closed this as completed May 23, 2024
@Loirooriol
Copy link
Contributor

I guess this wasn't addressed:

if you're firing a new event on the Element interface, that interface should be extended to add an onevent event handler IDL attribute to meet the Always add event handlers design principle

Blink supports the IDL attribute but not the content attribute, Gecko doesn't support either.

But I'm not sure if this should be done in HTML instead?

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

No branches or pull requests

6 participants