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

dialog & ::backdrop #12

Closed
jensimmons opened this issue Nov 10, 2021 · 37 comments
Closed

dialog & ::backdrop #12

jensimmons opened this issue Nov 10, 2021 · 37 comments
Labels
accepted An accepted proposal focus-area-proposal Focus Area Proposal
Milestone

Comments

@jensimmons
Copy link
Contributor

jensimmons commented Nov 10, 2021

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, and dialog 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.

@bkardell
Copy link
Contributor

Really high interop on a newly landing in several browsers feature like this, very quickly, seems kind of good to me.

@foolip foolip mentioned this issue Nov 12, 2021
36 tasks
@josepharhar
Copy link

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
The new tests are already included in the same directory as the WPT link in the issue description.

@foolip
Copy link
Member

foolip commented Nov 22, 2021

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.

While <dialog> wasn't among the top categories in #7, there were a few mentions of it in State of CSS 2021. Twice in the "browser compatibilities" question, once for "other missing features", but I think most interestingly dialogs (native or custom) were brought up in the accessibility section:

keyboard trap für Dialoge

Appropriate focus handling (programmatically moving focus, retaining focus inside custom modal dialogs, etc)

Focus management (such as when a dialogue is open).

WCAG compliant components via JS (i.e. dialogs, etc).

There were also mentions of <dialog> in the MDN browser compat survey, most without much detail, but https://github.com/willmcpo/body-scroll-lock came up in an interview. It's sometimes used to prevent scrolling behind modals.

So even though the HTML element <dialog> isn't top of mind like Container Queries or Subgrid, problems that have to do with the use case do seem fairly prominent, and making <dialog> solid cross-browser should help with that, even if it takes a few years for the effect to be visible.

@foolip foolip closed this as completed Nov 22, 2021
@foolip foolip reopened this Nov 22, 2021
@foolip
Copy link
Member

foolip commented Nov 26, 2021

I just found an old email from @nt1m, who pointed out that we have some tests for <dialog> in Chromium:
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/html/dialog/

Looking through the history, I see that some were already converted and upstreamed to WPT by @shanmuga-m:
web-platform-tests/wpt#7338
web-platform-tests/wpt#7859
web-platform-tests/wpt#8174

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.

@josepharhar
Copy link

@foolip here is a related crbug: https://bugs.chromium.org/p/chromium/issues/detail?id=1240798

@jensimmons
Copy link
Contributor Author

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.

@jensimmons
Copy link
Contributor Author

jensimmons commented Nov 29, 2021

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, dialog is highly requested. But hey, looking for new information to double check long-held beliefs is always a good idea. So I wrote a tweet.

Screen Shot 2021-11-29 at 5 23 10 PM

63% of responders said, yes, dialog is important to them. They will use it. It should be a priority.

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 dialog. Fifty-two people replied as well, with why & how they'll use it. Read the replies to get a sense of how people feel.

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.

@foolip
Copy link
Member

foolip commented Dec 1, 2021

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.

Thanks for pointing this out, I agree that's a strong signal in itself.

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.

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.

@nt1m
Copy link
Member

nt1m commented Dec 1, 2021

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)

@foolip
Copy link
Member

foolip commented Dec 2, 2021

@nt1m thanks, I've asked internally about that PR and if there are any other <dialog> issues related to focus or accessibility that need attention. Do you know of others, or is some resolution of whatwg/html#4184 the only outstanding issue here?

@josepharhar
Copy link

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.

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:

  • dialog-focus-shadow.html
  • dialog-cancel-with-select.html
  • dialog-showModal.html
  • inert-node-is-unfocusable.html
  • shadowdom-in-dialog.html

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.

@nt1m
Copy link
Member

nt1m commented Dec 3, 2021

I think we can probably just use autofocus on tests unrelated which rely on focusing the first item.

@josepharhar
Copy link

I have opened PRs to migrate the rest of the chrome-only dialog tests to WPT:

@josepharhar
Copy link

All of the dialog tests from my last comment have been merged!
If there any issues with any of them I'm happy to address them, but we can at least collect the test names now.
@foolip

@foolip foolip added the accepted An accepted proposal label Dec 14, 2021
@foolip
Copy link
Member

foolip commented Dec 15, 2021

@nt1m
Copy link
Member

nt1m commented Dec 15, 2021

dialog-focusing-steps-inert.html and dialog-inert.tentative.html should be unlabeled, they test the inert attribute which isn't in the HTML spec yet.

@foolip
Copy link
Member

foolip commented Dec 16, 2021

@nt1m done in web-platform-tests/wpt-metadata#2303

github-actions bot pushed a commit to web-platform-tests/wpt-metadata that referenced this issue Dec 16, 2021
@nt1m
Copy link
Member

nt1m commented Dec 17, 2021

Can these 2 tests be labeled? web-platform-tests/wpt@b3958a2

They test ::backdrop + animation

@foolip
Copy link
Member

foolip commented Dec 22, 2021

@nt1m I've labeled those in web-platform-tests/wpt-metadata#2327

@emilio
Copy link

emilio commented Jan 10, 2022

I'm technically on PTO, but from a quick look the tests that Firefox fail are about backdrop-animation (which is sort - of unrelated to dialog, it's about ~every non-element backed pseudo in Gecko, and affects :fullscreen too).

Well given this is <dialog> and ::backdrop these are legit I suppose :-)

@josepharhar
Copy link

I'll contact the author of modal-dialog-scroll-height.html and see if we can test the same behavior without the exact height expectation thats causing the failure

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 10, 2022
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
foolip added a commit to web-platform-tests/wpt-metadata that referenced this issue Jan 11, 2022
@foolip
Copy link
Member

foolip commented Jan 11, 2022

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.

@josepharhar
Copy link

I believe I found a way to fix modal-dialog-scroll-height.html by replacing the hard coded height with window.innerHeight:
https://chromium-review.googlesource.com/c/chromium/src/+/3379191
web-platform-tests/wpt#32319

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 12, 2022
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
aarongable pushed a commit to chromium/chromium that referenced this issue Jan 12, 2022
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 <jarhar@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#958207}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 12, 2022
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 <jarhar@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#958207}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 12, 2022
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 <jarhar@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#958207}
@josepharhar
Copy link

@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

@nt1m
Copy link
Member

nt1m commented Jan 18, 2022

@foolip
Copy link
Member

foolip commented Jan 20, 2022

One test came up in my analysis in #48. It looks like a solved problem, but quoting for visibility:

/html/semantics/interactive-elements/the-dialog-element/backdrop-receives-element-events.html is TIMEOUT in Safari with the single subtest being NOTRUN. It's linked to https://webkit.org/b/233072 which was recently fixed, confirmed with Safari + WebKitGTK.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Feb 5, 2022
…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 <jarhar@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#958207}

--

wpt-commits: 7915bc605347950da0a0af3707a788048a08ff3c
wpt-pr: 32319
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Feb 7, 2022
…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 <jarhar@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#958207}

--

wpt-commits: 7915bc605347950da0a0af3707a788048a08ff3c
wpt-pr: 32319
@kbrilla
Copy link

kbrilla commented Jul 5, 2022

@foolip
Copy link
Member

foolip commented Jul 5, 2022

@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 :modal this year.

@jgraham
Copy link
Contributor

jgraham commented Jul 5, 2022

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.

@nt1m
Copy link
Member

nt1m commented Jul 5, 2022

WebKit and Gecko have both implemented :modal and I think only Chromium is not shipping it, but has a patch up.

I don't have strong opinions on including it in interop 2022 though.

@kbrilla
Copy link

kbrilla commented Jul 5, 2022

Turns out there already is test that checks for :modal introduced by this PR https://github.com/web-platform-tests/wpt/pull/34107/files

@kbrilla
Copy link

kbrilla commented Jul 13, 2022

update - :modal is shipping in Chrome106 Chromestatus

@foolip foolip closed this as completed Aug 18, 2022
@gsnedders gsnedders added focus-area-proposal Focus Area Proposal and removed proposal labels Sep 16, 2022
@gsnedders gsnedders added this to the Interop 2022 milestone Sep 16, 2022
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
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 <jarhar@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#958207}
NOKEYCHECK=True
GitOrigin-RevId: 86245eb07b2d6a8b90eefb53de3e812f85eab609
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted An accepted proposal focus-area-proposal Focus Area Proposal
Projects
None yet
Development

No branches or pull requests

9 participants