Problem/Motivation

https://blog.jqueryui.com/2024/06/jquery-ui-1-14-0-beta-2-released/

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3454960

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

larowlan created an issue. See original summary.

quietone made their first commit to this issue’s fork.

quietone opened merge request !8425

quietone’s picture

quietone commented
Status: Active » Needs review
rkoller’s picture

rkoller
he/him
Primary language German
Location Nürnberg, Germany
commented

I've successfully applied the patch. I was curious because I've worked (with the help of @rocketeerbkw) on adding the aria-modal attribute to dialog modals upstream to jquery ui (https://github.com/jquery/jquery-ui/pull/2257), as a short term fix for a serious issue for screen reader users in drupal #3395355: With an open dialog modal also elements in the background are added to the accessibility object model until https://www.drupal.org/project/dialog_native lands. I've assumed it should/would work out of the box with drupal but when i've tested with one of the dialog modals on admin/structure/views/view/content the dialog-min.js for the beta2 of jquery ui dialog is loaded but aria-modal="true" is not being added to the open dialog modal? Is the dialog initialized somehow else in drupal and not with the modal option set to true? because with the latter the aria-modal attribute should be added in beta2? and sorry for the basic question but creating the patch was already way out of my comfort zone. :/

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

@rkoller believe the dialog is initialized from misc/dialog/dialog.js but confirmed not seeing the aria-modal so probably something we could fix on our end in the issue you linked.

Reading https://jqueryui.com/changelog/1.14.0-beta.2/ that seems to be the main fix in this release with some smaller tests for jquery.

Applied locally and not noticing anything broken.

nod_’s picture

nod_
Primary language French
Location Lille
commented

Can you open a follow-up for the aria modal issue?

nod_’s picture

nod_
Primary language French
Location Lille
commented

No need for follow-up, we override the _createWrapper method where this is set initially. Ported the relevant line in core and updated the MR

  • nod_ committed 7be520e6 on 11.0.x 
    Issue #3454960 by quietone, nod_, larowlan, rkoller, smustgrave: Update...

  • nod_ committed 1b5c7db4 on 11.x 
    Issue #3454960 by quietone, nod_, larowlan, rkoller, smustgrave: Update...

nod_ closed merge request !8425

nod_’s picture

nod_
Primary language French
Location Lille
commented
Component: base system » javascript
Status: Reviewed & tested by the community » Fixed

Committed and pushed 1b5c7db4ee to 11.x and 7be520e6e5 to 11.0.x. Thanks!

rkoller’s picture

rkoller
he/him
Primary language German
Location Nürnberg, Germany
commented

ah thanks i wasn't aware of that detail in regards of the override of the method. but there is one detail i wonder about. would it make sense to also add a test that makes sure that this override adding the aria-modal attribute when the modal option is set to true is still properly working and not silently breaking in one of the future upgrades of core?

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +11.0.0 release notes