[regression] Class is not added to MediaLibrary dialog

I've tested it on D11.0.1 using the mentioned steps.
verified that the class "media-library-widget-modal" is attached.

Sharing SS for reference:

Hmm I wonder what is different then? I ran into this with a fresh install of Drupal 11 using the recommended composer template. I installed the umami profile. I also switched to Olivero in case there was something theme specific going on there.

On my actual production site which is on 10.3.x where the change had already taken place I don't experience this bug.

Did you test this on a new install or an existing site?

I tested it on new D11 instance.

@rhovland have you retested the issue?

I just tested it again. This time using the standard profile. Enabled the media module, added the embed media button to the basic text format.

Created a new basic page. Clicked the add media button. Class media-library-widget-modal is missing.

I had found another issue talking about this. I had noticed on my Drupal 11 site that the media library modal was narrower than in Drupal 10. I then found the missing class.

Changing the line in ckeditor5.ckeditor5.yml certainly fixes the problem.

I also tried adding the class in MediaLibrary.php. This worked too.

// Making the title for editor drupal media embed translatable.
    $static_plugin_config['drupalMedia']['dialogSettings'] = [
      'title' => $this->t('Add or select media'),
      'classes' => ['media-library-widget-modal'],
    ];

Looks like this affects other users as well but only some of them.

This is a patch applying the change suggested in the original ticket. It restores the line

dialogClass: media-library-widget-modal

to ckeditor5.ckeditor5.yml.

Thanks for the patch, it works for me on Drupal 11.1.1.

Also works for me on Drupal 11.1.1. Thanks!

Yes, MR changes are working correctly. Moving to review..

Will need test coverage as a bug.

Patch is working for me as well. but not sure why class - ui-dialog-narrow is added here when used in ckeditor
while using media field out of ckeditor does not have this class ? do we also want to remove this class ? if no then may i know what is the use of this ?

We don't have visual regression testing in core so I'm not sure how we'd add test coverage for this, tagging for manual testing though.

We can't add a test to verify the existence of a class?

Seems to be consistently having a test failure though.

I can confirm the issue as I stumbled on this after upgrading to D11.
It's very easy to reproduce with demo_umami profile and opening the add media modal from ckeditor.
I just did a git bisect on the 11.x branch and
66c3914d39bf24d828ec8516389d7cadad52294e is the first bad commit and indeed comes from 📌 Removal :tabbable usage in dialog.js Fixed

herved → changed the visibility of the branch 3474018-alt to hidden.

herved → changed the visibility of the branch 3474018-alt to active.

Created new MR, the actual fix is in OpenDialogCommand::__construct
- 📌 Removal :tabbable usage in dialog.js Fixed was committed to 10.3.x and 11.x but their commit differs, mainly in OpenDialogCommand::__construct where the issue lies in 11.x, because if $dialog_options['dialogClass'] is set (it is via ckeditor5.js), then $dialog_options['classes']['ui-dialog'] is not taken into account.
- this MR contains improvements in order to ensure both $dialog_options['classes']['ui-dialog'] and $dialog_options['dialogClass'] are taken into account/preserved.
- 10.3.x is not affected by this bug per-se because it does preserve $dialog_options['classes']['ui-dialog'], however it could (should IMO) be ported as well, for consistency.
- I took the opportunity to align the way this is handled in OpenDialogCommand and OpenOffCanvasDialogCommand
- Added some regression tests for those + test to ensure the media-library-widget-modal class is present on the ckeditor media library modal.

Current MR snapshot, for composer.

IS appears to need some love

Thanks @herved for grabbing the latest MR snapshot. Confirmed that this applies correctly in Core 11.1.8

Noting that this fixed and applied cleanly to 11.2.3. Thank you very much

herved → changed the visibility of the branch 3474018-regression-class-is to hidden.

Left some small comments

Cleaning up tags

Test-only feature was ran here https://git.drupalcode.org/issue/drupal-3474018/-/jobs/5763875 so removing that tag.
Summary is updated so removing that tag as well.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

MR rebased