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

With the modal option set to true, elements in the background are still available in the AOM #2246

Open
rpkoller opened this issue Apr 28, 2024 · 5 comments

Comments

@rpkoller
Copy link

If you take a look for example at https://jqueryui.com/dialog/#modal-message and you test the page with voiceover on macos and you are using the rotor functionality you will notice that elements in the background of the dialog modal are still available in the accessibility object model (aom). the number of available elements is sort of limited due to the fact that the example is wrapped in an iframe:

rutrum_convallis.mov

but for example in the context of a drupal sites which are also using jquery ui for building dialog modals they are not wrapped into an iframe, and you have all the elements in the background available in the aom as well. and there is no way for a screenreader user to distinguish what is an element within a dialog modal and what is an element outside of the dialog modal in its background.

modal_false.mp4

An easy fix would be if the modal option is set to true adding the aria-modal="true" attribute to the div of the dialog modal. i've manually added the aria-modal attribute via devtools and rerecorded the same example from before to illustrate the difference

modal_true.mp4
@mgol
Copy link
Member

mgol commented Apr 30, 2024

Thanks for the report. Does the issue you describe exist when jQuery UI 1.12.1 is used or only with jQuery UI 1.13.0 or newer?

@rpkoller
Copy link
Author

I've just rechecked, in Drupal 10.2.5. and 11.x-dev jQuery UI Dialog 1.13.2 is used. Took me a while to go back in versions to finally fin, but turns out drupal 9.0.0 was using jQuery UI Dialog 1.12.1. And for all three versions of Drupal 11.x-dev, 10.2.5, and 9.0.0 the problem described in the issue summary exists.

@mgol
Copy link
Member

mgol commented Apr 30, 2024

Thanks for the info. Since the issue is already in 1.12, given limited team resources it's not likely to be fixed by the UI team; see the project status at https://blog.jqueryui.com/2021/10/jquery-maintainers-update-and-transition-jquery-ui-as-part-of-overall-modernization-efforts/. PRs are welcome if they're not too complex and contain tests.

On a personal note, I definitely appreciate how comprehensive this bug report is. I got interested why jQuery UI hasn't used your proposed solution so far considering its strong focus on accessibility back when it had a much larger team but it seems this API is quite new and it arrived in browsers first in 2019-2020: https://caniuse.com/mdn-api_element_ariamodal. Most of the regular development of jQuery UI ended in 2016.

I'd definitely appreciate a PR with a fix for this issue. Remember to add tests if you submit one but please feel free to ask questions if you get stuck on a PR.

@rpkoller
Copy link
Author

Sorry for the late reply, last two weeks were a bit busy on my end. And an really interesting detail about the chronology you've dug up, thank you for that! And yeah the aria-modal support got better just in recent years. Back then it was not available yet. And I've tried my luck and managed to get the jquery ui dialog script adding the aria-modal attribute on a local drupal instance. i've simply added 'aria-modal': 'true', after line 357 https://github.com/jquery/jquery-ui/blob/main/ui/widgets/dialog.js . just to see if there is an improvement to the current state and if the attribute is getting added. and yes the aria-modal="true" is getting added. but the odd thing. when i test opening a dialog modal for that drupal install the elements in the background are excluded from the AOM in edge (as an example for chrome based browsers) while in safari and firefox they are not. but puzzling detail if i take a look at the a11y tab in firefox the state modal is set with the dialog modal open. so in the AOM the dialog modal is marked up as a modal correctly .
Screenshot 2024-05-14 at 13 47 16
i've just asked now over in the a11y slack if anyone more familiar with the technical details knows if that is some sort of regression in recent versions of voiceover or if some additional measures might be necessary.

@rpkoller
Copy link
Author

rpkoller commented May 24, 2024

I've investigated some more and had a followup discussions with darren lee on that a11y slack. still think the least invasive fix, keeping backwards compatibility in mind, might be by just adding the aria-modal="true" attribute. another option one step further might be changing the div element to a dialog element in line 344 https://github.com/jquery/jquery-ui/blob/9180a8180b17c38f6c3f27ba46d4546d800d3508/ui/widgets/dialog.js#L344C24-L344C27, or you might even do some sniffing according to darren lee if the current UA supports <dialog> and then create <dialog> else create <div>. The <dialog> would make even the addition of the tabindex and the role="dialog"as well as the attribute-modal="true" obsolete. But from the backward compatibility aspect, you dont know if someone relies onto the div element for whatever reason. Therefore, as already stated, adding the aria-modal="true"would be the least invasive approach, providing a good portion of screenreader users still an improved experience ( i have to add in macOS voiceover there is a recent bug breaking both the dialog element as well as aria-modal on macOS, that means no matter if the element and or attribute are in place the dialog modal background is still available in the aom in safari, edge and firefox. i'Ve already reported it and it looks like there are several similar reports).

will try to create an initial draft for a pull request including a test. currently reading up on the requirements and best practices in the context of the jquery ui queue. but please bare with me, i am not a developer as i've already mentioned. ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants