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

Convert :open to :popover-open for popovers #39222

Merged
merged 1 commit into from
Mar 31, 2023

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Mar 27, 2023

See [1]/[2] for more context, but there are cases where :open is
ambiguous for popovers. If multiple elements support :open/:closed,
and [popover] can be applied to any of them, there are situations
where an element is both open and closed. For example,
\<details popover> can be closed as a details element and open as
a popover, which makes it match both :open and :closed. It seems
that really :open and :closed should match elements that can
open and close, and not things that can be made to open or close
via an attribute or other mechanism such as JS.

This CL adds :popover-open which only applies to popovers in the open
state, and it removes :open and :closed support for popovers. It also converts all of the popover WPTs to use :popover-open instead
of either :open or :closed.

[1] w3c/csswg-drafts#8637
[2] whatwg/html#9077

Bug: 1307772
Fixed: 1429670
Change-Id: I8d840512166ccdb5d5c8abbb7192bbce7177ee88
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4373888
Reviewed-by: Joey Arhar <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1124869}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

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

The review process for this patch is being conducted in the Chromium project.

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-4373888 branch 3 times, most recently from 80d0037 to f0c8874 Compare March 29, 2023 20:06
@chromium-wpt-export-bot chromium-wpt-export-bot changed the title Convert :open to :popover for popovers Convert :open to :popover-open for popovers Mar 31, 2023
@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-4373888 branch 2 times, most recently from e6a34df to f7dc063 Compare March 31, 2023 19:21
See [1]/[2] for more context, but there are cases where `:open` is
ambiguous for popovers. If multiple elements support `:open/:closed`,
and [popover] can be applied to any of them, there are situations
where an element is both open and closed. For example,
`<details popover>` can be closed as a details element and open as
a popover, which makes it match both `:open` and `:closed`. It seems
that really `:open` and `:closed` should match *elements* that can
open and close, and not things that can be made to open or close
via an attribute or other mechanism such as JS.

This CL adds `:popover-open` which only applies to popovers in the open
state, and it removes `:open` and `:closed` support for popovers. It also converts all of the popover WPTs to use `:popover-open` instead
of either `:open` or `:closed`.

[1] w3c/csswg-drafts#8637
[2] whatwg/html#9077

Bug: 1307772
Fixed: 1429670
Change-Id: I8d840512166ccdb5d5c8abbb7192bbce7177ee88
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4373888
Reviewed-by: Joey Arhar <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1124869}
@chromium-wpt-export-bot chromium-wpt-export-bot merged commit e68cb91 into master Mar 31, 2023
@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-cl-4373888 branch March 31, 2023 21:26
annevk pushed a commit to whatwg/html that referenced this pull request Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants