-
Notifications
You must be signed in to change notification settings - Fork 27
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
dialog & ::backdrop #12
Comments
Really high interop on a newly landing in several browsers feature like this, very quickly, seems kind of good to me. |
There was a recent change to the spec and tests here: And here is a crbug for changing that behavior: https://bugs.chromium.org/p/chromium/issues/detail?id=383230 |
While
There were also mentions of So even though the HTML element |
I just found an old email from @nt1m, who pointed out that we have some tests for Looking through the history, I see that some were already converted and upstreamed to WPT by @shanmuga-m: However, a bunch of the tests that remain probably still make sense to upstream. I don't know who might be able to work on that (if this proposal is accepted) but I will ask around. |
@foolip here is a related crbug: https://bugs.chromium.org/p/chromium/issues/detail?id=1240798 |
We should make sure we use modern tests that match the HTML spec version that Mozilla & Apple are endorsing, and not the older version from the past that isn't accessible. IOW, I'm not sure that older tests are testing the right version of the spec. |
I wanted to get another data point for a sense of whether or not this is important to web developers. After 6 years of doing this research non-stop, my instincts say yes, 63% of responders said, yes, More than that, however, the number of people who answered the poll tells me a lot. Over 2,000 people answered in 24 hours — which is far more than my quick informal twitter polls usually get. There was excitement about and clarity in expressing an opinion about People want this element. They will use it. They've been waiting a really long time. And they are going to be really frustrated if we don't have interop. |
Thanks for pointing this out, I agree that's a strong signal in itself.
Do you know which spec changes were made around this? The WHATWG working mode involves adding tests, so PRs like web-platform-tests/wpt#31523 might have made the necessary changes. |
We're waiting to hear back from Google on whatwg/html#4184 (comment) |
@nt1m thanks, I've asked internally about that PR and if there are any other |
The only WPT which is explicitly concerned with the behavior in the proposed spec change is show-modal-focusing-steps.html. I made a patch for the proposed change in chromium based on the firefox one in order to find out which WPTs would also fail, and I found that these tests also fail:
These tests aren't necessarily concerned with the proposed focus changes, and perhaps could even be changed to pass regardless of which focus behavior is implemented. Some have many subtests where only one fails with the proposed changes. |
I think we can probably just use |
I have opened PRs to migrate the rest of the chrome-only dialog tests to WPT: |
All of the dialog tests from my last comment have been merged! |
Thanks @josepharhar! I've labeled all the tests (I think!) and you can see them here: |
dialog-focusing-steps-inert.html and dialog-inert.tentative.html should be unlabeled, they test the |
Can these 2 tests be labeled? web-platform-tests/wpt@b3958a2 They test |
@nt1m I've labeled those in web-platform-tests/wpt-metadata#2327 |
Well given this is |
I'll contact the author of |
This test is failing in all browsers on wpt.fyi: https://wpt.fyi/results/html/semantics/interactive-elements/the-dialog-element/modal-dialog-scroll-height.html I'm not sure why it passes in the chromium test infrastructure, but people want to remove it: web-platform-tests/interop#12 (comment) Steve, I see you added this test here: https://codereview.chromium.org/574143002 Do you have any ideas for how else we could write this test which doesn't check the explicit height? Maybe with a reftest? I couldn't easily understand the bug and the change. Change-Id: If27a4b07ca235190d19cbe2f52bbfe6dd2594225
I've sent web-platform-tests/wpt-metadata#2352 to drop modal-dialog-scroll-height.html from Interop 2022. If another test can be added to test the same thing in a better way soon, maybe we can include that. |
I believe I found a way to fix modal-dialog-scroll-height.html by replacing the hard coded height with window.innerHeight: |
This test is failing in all browsers on wpt.fyi due to the hard coded height: https://wpt.fyi/results/html/semantics/interactive-elements/the-dialog-element/modal-dialog-scroll-height.html I'm not sure why it passes in the chromium test infrastructure, but people want to remove it: web-platform-tests/interop#12 (comment) This patch replaces the hard coded height with window.innerHeight. Change-Id: If27a4b07ca235190d19cbe2f52bbfe6dd2594225
This test is failing in all browsers on wpt.fyi due to the hard coded height: https://wpt.fyi/results/html/semantics/interactive-elements/the-dialog-element/modal-dialog-scroll-height.html I'm not sure why it passes in the chromium test infrastructure, but people want to remove it: web-platform-tests/interop#12 (comment) This patch replaces the hard coded height with window.innerHeight. Change-Id: If27a4b07ca235190d19cbe2f52bbfe6dd2594225 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3379191 Auto-Submit: Joey Arhar <[email protected]> Reviewed-by: Steve Kobes <[email protected]> Commit-Queue: Steve Kobes <[email protected]> Cr-Commit-Position: refs/heads/main@{#958207}
This test is failing in all browsers on wpt.fyi due to the hard coded height: https://wpt.fyi/results/html/semantics/interactive-elements/the-dialog-element/modal-dialog-scroll-height.html I'm not sure why it passes in the chromium test infrastructure, but people want to remove it: web-platform-tests/interop#12 (comment) This patch replaces the hard coded height with window.innerHeight. Change-Id: If27a4b07ca235190d19cbe2f52bbfe6dd2594225 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3379191 Auto-Submit: Joey Arhar <[email protected]> Reviewed-by: Steve Kobes <[email protected]> Commit-Queue: Steve Kobes <[email protected]> Cr-Commit-Position: refs/heads/main@{#958207}
This test is failing in all browsers on wpt.fyi due to the hard coded height: https://wpt.fyi/results/html/semantics/interactive-elements/the-dialog-element/modal-dialog-scroll-height.html I'm not sure why it passes in the chromium test infrastructure, but people want to remove it: web-platform-tests/interop#12 (comment) This patch replaces the hard coded height with window.innerHeight. Change-Id: If27a4b07ca235190d19cbe2f52bbfe6dd2594225 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3379191 Auto-Submit: Joey Arhar <[email protected]> Reviewed-by: Steve Kobes <[email protected]> Commit-Queue: Steve Kobes <[email protected]> Cr-Commit-Position: refs/heads/main@{#958207}
@foolip Can you put modal-dialog-scroll-height.html back on the list? The new version is passing in all browsers: https://wpt.fyi/results/html/semantics/interactive-elements/the-dialog-element/modal-dialog-scroll-height.html |
One test came up in my analysis in #48. It looks like a solved problem, but quoting for visibility:
|
…estonly Automatic update from web-platform-tests Fix modal-dialog-scroll-height.html This test is failing in all browsers on wpt.fyi due to the hard coded height: https://wpt.fyi/results/html/semantics/interactive-elements/the-dialog-element/modal-dialog-scroll-height.html I'm not sure why it passes in the chromium test infrastructure, but people want to remove it: web-platform-tests/interop#12 (comment) This patch replaces the hard coded height with window.innerHeight. Change-Id: If27a4b07ca235190d19cbe2f52bbfe6dd2594225 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3379191 Auto-Submit: Joey Arhar <[email protected]> Reviewed-by: Steve Kobes <[email protected]> Commit-Queue: Steve Kobes <[email protected]> Cr-Commit-Position: refs/heads/main@{#958207} -- wpt-commits: 7915bc605347950da0a0af3707a788048a08ff3c wpt-pr: 32319
…estonly Automatic update from web-platform-tests Fix modal-dialog-scroll-height.html This test is failing in all browsers on wpt.fyi due to the hard coded height: https://wpt.fyi/results/html/semantics/interactive-elements/the-dialog-element/modal-dialog-scroll-height.html I'm not sure why it passes in the chromium test infrastructure, but people want to remove it: web-platform-tests/interop#12 (comment) This patch replaces the hard coded height with window.innerHeight. Change-Id: If27a4b07ca235190d19cbe2f52bbfe6dd2594225 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3379191 Auto-Submit: Joey Arhar <[email protected]> Reviewed-by: Steve Kobes <[email protected]> Commit-Queue: Steve Kobes <[email protected]> Cr-Commit-Position: refs/heads/main@{#958207} -- wpt-commits: 7915bc605347950da0a0af3707a788048a08ff3c wpt-pr: 32319
What about |
@criskrzysiu that's newer than Interop 2022, and changes to the tests we include for scoring need to be reviewed, so that the target doesn't change much throughout the year. But we have both added and removed tests, for various reasons. If you want to suggest that, you can file a new issue in this repo. Whether everyone is on board with it probably depends mostly on whether they already intend to ship |
Without knowing anything about Gecko's plans here, I don't like the idea of making major changes to the scope of a focus area during the year. I think test additions / removals should be scope-neutral and only used to address issues of correctness or completeness within the agreed boundaries of the focus area. |
WebKit and Gecko have both implemented I don't have strong opinions on including it in interop 2022 though. |
Turns out there already is test that checks for |
update - |
This test is failing in all browsers on wpt.fyi due to the hard coded height: https://wpt.fyi/results/html/semantics/interactive-elements/the-dialog-element/modal-dialog-scroll-height.html I'm not sure why it passes in the chromium test infrastructure, but people want to remove it: web-platform-tests/interop#12 (comment) This patch replaces the hard coded height with window.innerHeight. Change-Id: If27a4b07ca235190d19cbe2f52bbfe6dd2594225 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3379191 Auto-Submit: Joey Arhar <[email protected]> Reviewed-by: Steve Kobes <[email protected]> Commit-Queue: Steve Kobes <[email protected]> Cr-Commit-Position: refs/heads/main@{#958207} NOKEYCHECK=True GitOrigin-RevId: 86245eb07b2d6a8b90eefb53de3e812f85eab609
Description
Implement / update implementations of the dialog element & ::backdrop pseudo-element.
Specifications
Tests
dialog tests
tests needed for tabindex + .showModal() and focus of dialog element
Rationale
The
<dialog>
element has been highly requested for years. If it's not appearing on recent surveys, it may just be because web developers have given up hope. The need to create overlays has not disappeared, anddialog
gives web developers an accessible built-into-the-web technology to do so well.The
::backdrop
pseudo-element gives developers a way to style the modal box.According to Can I Use and MDB BDC, dialog & ::backdrop are supported by Chromium browsers, and need to be implemented in Gecko & WebKit. It seems progress was blocked for years by disagreement about accessibility details, especially regarding how to handle focus. There are now two implementers agreeing on what the HTML spec should be.
The text was updated successfully, but these errors were encountered: