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
WebKit export of https://bugs.webkit.org/show_bug.cgi?id=240109 #33999
Conversation
This patch has been exported from WebKit; it will be approved automatically once the downstream patch is r+. |
There was a problem hiding this 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 WebKit project.
const computedStyle = window.getComputedStyle(dialog, null); | ||
assert_equals(computedStyle.display, 'none'); | ||
assert_equals(computedStyle.display, "none"); | ||
assert_false(dialog.matches(":modal")); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has this (the :modal
pseudo class) landed in a spec yet? I know it was resolved to be added, but I hadn't seen it land. And this change causes all browsers to fail this test now. (Also, this test is part of Interop2022, so it kind of sucks to change it like this now.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found the spec: https://drafts.csswg.org/selectors/#modal-state
Maybe ok to keep this test, I suppose. Still feels odd to add things to Interop2022 mid-year that were actually added to specs mid-year. But ok, I suppose this is a simple-enough addition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mfreed7 can you weigh in on web-platform-tests/interop#79?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that we generally want the tests to stay the same throughout the year, but we don't have anything in place to prevent test updates. Instead we rely on review after the fact if it happens.
No description provided.