Nested modals don't work: opening a modal from a modal closes the original

This is really about nesting of modal dialogs. I'd swear there is an issue for this, but I can't find it.

hmm....It seems we can improve it by changing CKEditor dialog to a unique ID. Any thoughts @WimLeers / maintainers?
https://github.com/drupal/drupal/blob/8.2.x/core/misc/dialog/dialog.ajax...

This is a generic problem. What if something triggers a modal while already within a modal? That should work. This is not at all CKEditor-specific.

@vollepeer: can you explain that a bit?

Oh, now I found that issue I was looking for in #2 — turns out @vollepeer was the one who opened it! It's #2737319: Opening a modal from inside another modal (nested modals), I marked it as a duplicate of this issue.

Hah, no worries :) Can be confusing/overwhelming to find your way through the Drupal core issue queue :) Hopefully my triaging helps.

I have ran into this issue with the YAML Form module's UI where I am relying on modals.

Below is an animated GIF documenting my issue.

It is worth noting that the Token module's dialog does not replace the YAML Form module's dialog because the Token module is opening a dialog and not a modal.

Considered the BC, I thought it should be opt-in (to open another Modal) than opt-out (close old instance).

Instance A:
In general, by design. If I make a module and expected anything working inside a Modal. I will set a unique ID for the dialog (It's already supported in CORE, like the Token does)

Instance B:
In another case, take the CKEditor as example, I'd thought:

B1. If we designed the CKEditor buttons Modal always pop up to TOP layer and hidden all other elements. Then, other modules should listen to its callback and restore the old windows.

B2. If we designed the CKEditor buttons worked inside Modal BY DEFAULT, then we should create a unique ID for the CKEditor buttons dialog.

Personally, with CKEditor example, I preferred B2. Just like what the Token does in above screenshot.

Instance A & B have no conflicts, it can be done at same time.

Let me know if I'm missing some important points. :)

This issue is also related to this one: #2836043 (https://www.drupal.org/node/2836043)

Tagging 2 tags to see what their thoughts on these usages. (also on Comment #12)

"Single Modal all-time" vs "Nested Modal" vs "BOTH"

In the event that you need to go against the drupal defined method of a single modal window - which in turn prevents the use of nested modal window - you could use the OpenDialogCommand rather than OpenModalDialogCommand with the modal option set to true. This would allow you to use nested modals in the context of a built module.

This is really more of a feature request than anything else. Using nested modals is a bad practice.

If there would be modal form in the core that would use any of these it would be a major bug. Unfortunately it is not :(

Do dialogs in the Views UI count?

See #2900594: CKEditor not available for Views "text" areas

Hi All!

I had same issue.

For calling a modal window we should add a "data-dialog-type" attribute. This attribute accepts the values "modal" or "dialog".

I had the "modal" value.
After changing the value to "dialog" the modals from CKEditor it seems work

Hello!

@tibezh nice idea! :)
I had the same issue. I open popup (modal dialog) and when I click by "link" button, I see just one popup with link settings.
I updated ckeditor module and it works for me - see the screenshot. It shows popup over popup.

I created the patch to fix it. Verify it, please.
Thank you!

Hmm... Sorry, there are errors already. When I click by "Save" on link popup, it closes parent modal.

On my local project I fixed it: I set "dialog" type for my modal and "modal" type for ckeditor, it works correctly.
But it is a custom solution.

I needed to solve this problem as this is blocking having any configurable modal form with fields as any configurable modal form can contain wysiwyg (with ckeditor and thus nested modals in form of image/link modal).

The working nested modal windows idea for my workaround:

1. Change the main #drupal-modal id on the first modal window just before the second level modal is opened.
2. Disable the main #drupal-modal window to be disabled while the second level modal is active/opened.
3. Return back the main #drupal-modal id to the first modal window just after the second level modal is closed.

Even though each dialog window has its own specific id (using hash to be certain about it), the #drupal-modal id is being used as the main identifier (like a shortcut to make the logic/implementation easier). Opening and closing modal dialog is done through jQuery dialog plugin and the id specific to each dialog window is being used.

To support nested modal dialogs we just need to create dialog ids stack and change the #drupal-modal id to something else when getting one level higher and returning it back when moving down. Disabling the lower level dialogs is also required so that user can’t interact with them while working with the active one (the top one).

Additionally I had to create "BeforeOpenDialogCommand" (an AJAX command) that and alter AJAX responses to send it before "OpenDialogCommand". This way I was able to modify modal ids and z-index (to disable it) before new modal dialog was open. Including creating a new placeholder for modal window by:

<?php
$('<div id="drupal-modal" class="ui-front"/>').hide().appendTo('body');
?>

Later on using $(window).on('dialog:afterclose', function (e, dialog, $element) {...} to remove that placeholder and set the previous modal id and z-index back.

I would vote for having somethng like that in core. It might be not the best practise to use nested modals, mostly going more then two levels, but forbidding it by not implementing generic solution for modals, while there's no technical obsticle, seems bad practise to me.

stumbled over this issue because I run into the same problem. bump this issue :)

The Layout Builder Modal module had a similar issue, and what we did was that we use the dialog type dialog instead of modal, and then having the option modal as true for the dialog rendering the CKEditor.

I noticed that the suggestion has already been commented earlier, would that option work for you @dercheffe?

Hi @Johndevman,

thanks for your reply.

I tried the patch provided in #26 and the proposed solution (I guess you mean this solution?) in general it's working fine. but I experienced the same errors/behaviours mentioned in #27.

Hey... the same issue appears in Media Library when Media entities have a text field with CKEditor enabled:

- media entity "image" has a text field "copyright", where a CKEditor is used with a link button
- a node entity has a media field referencing media entities of type image, the form widget is Media Library
- when I upload a new image in the Media Library I can edit the image fields within the modal of the Media Library. Here I see my copyright field. When I click on it the whole modal is filled by the fields for link plugin. When I fill in the fields and click "Save" nothing happens

Just to add another finding: when I use the Table plugin of CKEditor within a textfield the problem does not appear. The Table plugin seems to use the "cke_dialog". So it opens another modal ("cke_dialog") over the modal where CKEditor field is rendered in ("ui-dialog").

I am not able to go in deeper by now but I want to share my findings so far:

• Both Drupal CKEditor plugins causing the problems (links and images) call Drupal.ckeditor.openDialog to open the forms for the plugins in a modal loading some data with Ajax. The Drupal method is defined here: httpdocs/core/modules/ckeditor/js/ckeditor.js
• The CKEditor table plugin uses CKEDITOR.dialog.add.

Maybe someone else with more JavaScript skills may have a look on this. It seems to me that best solution is to refactor Drupal.ckeditor.openDialog in a way that might be similar to CKEDITOR.dialog.add?

For anyone stumbling across this thread wanting a terrible hack that work right now, you can use this:

(function ($) {
    Drupal.behaviors.mymodule = {
    	attach: function(context, settings) {
            if(!Drupal.ckeditor) { return; }

            Drupal.ckeditor.openDialog = function openDialog(editor, url, existingValues, saveCallback, dialogSettings) {
                var $existingDialog = $('#drupal-modal').attr('id', 'drupal-modal-background');
                
                var $target = $(editor.container.$);

                if (editor.elementMode === CKEDITOR.ELEMENT_MODE_REPLACE) {
                    $target = $target.find('.cke_contents');
                }

                $target.css('position', 'relative').find('.ckeditor-dialog-loading').remove();

                var classes = dialogSettings.dialogClass ? dialogSettings.dialogClass.split(' ') : [];
                classes.push('ui-dialog--narrow');
                dialogSettings.dialogClass = classes.join(' ');
                dialogSettings.autoResize = window.matchMedia('(min-width: 600px)').matches;
                dialogSettings.width = 'auto';

                var $content = $('<div class="ckeditor-dialog-loading"><span style="top: -40px;" class="ckeditor-dialog-loading-link">' + Drupal.t('Loading...') + '</span></div>');
                $content.appendTo($target);

                var ckeditorAjaxDialog = Drupal.ajax({
                    dialog: dialogSettings,
                    dialogType: 'dialog',
                    selector: '.ckeditor-dialog-loading-link',
                    url: url,
                    progress: { type: 'throbber' },
                    submit: {
                        editor_object: existingValues
                    }
                });

                ckeditorAjaxDialog.execute();

                window.setTimeout(function () {
                    $content.find('span').animate({ top: '0px' });
                }, 1000);

                Drupal.ckeditor.saveCallback = function(returnValues) {
                    saveCallback(returnValues);

                    var $dialog = $('div[id^="drupal-dialog-editorlink-dialog"]').parent();
                
                    if($dialog.hasClass('ui-dialog')) { $dialog.dialog().dialog('close'); }

                    $dialog = $('div[id^="drupal-dialog-media-libraryui"]').parent();
                    
                    if($dialog.hasClass('ui-dialog')) { $dialog.dialog().dialog('close') }

                    window.setTimeout(function() {
                        $('#drupal-modal-background').attr('id', 'drupal-modal');
                    }, 1000);
                };
            };
     	}
	};
}(jQuery));

I think we agreed earlier in this issue that the issue isnt specific to CKeditor

Current situation

1. Every OpenModalDialogCommand puts the modal in a wrapper with id #drupal-modal.
   Every CloseModalDialogCommandmakes all HTML elements with #drupal-modal disappear. So if two subsequent modals have been opened, both having #drupal-modal (which is not correct HTML), then both will get closed upon CloseModalDialogCommand.
2. I think the reason that this one and only ID is required is to avoid having two modals of the same type being opened at the same time and causing heavoc. However currently it means that two modals of any type cannot be opened at the same type. It should be possible to have one modal of one type and the other of another type interacting with each other.
3. In OpenModalDialogCommand::34 parent::__construct('#drupal-modal', $title, $content, $dialog_options, $settings); the ID is hardcoded.

My proposal

1. Allow the Open and Close Modal commands to receive an ID. If there is no ID, then base the ID on the the path of the request that's being opened inside the modal, and if there is no request, just use #drupal-modal. That way where possible the modal is given a more context aware id.
2. I've tried this with: ckeditor modals in modals work, and also media library field widgets within a modal.
3. Obviously this needs tests, but I would love to hear any feedback about this approach. Also running the test bot to see what comes back.

Afterthoughts

#20: This is really more of a feature request than anything else. Using nested modals is a bad practice.

Whether or not this is bad practice can be debated (I think that depends entirely on the use case.)

Independently, I would still qualify this issue as a bug since Drupal Core allows any url to be opened in a dialog by adding class="use-ajax" data-dialog-type="modal" to any link, including, say, links to node forms. On the other hand core opens modals from ckeditor (image) and in widgets (media library). So a modal in a modal is very much core functionality IMHO.

I'm marking this as a bug again, but feel free to put it back to 'task'.

This is - perhaps not unexpectedly - a lot more complex than it looks.

If there is no ID, then base the ID on the the path of the request that's being opened inside the modal, and if there is no request, just use #drupal-modal.

My idea was to add some sort of 'context' to the ID of the modal, but this context is not consistently available to be able to use it.

I'm uploading a new patch that uses the State System to assign and check for a random ID string to each modal. This is probably far from ideal, but I hope it's the start of a solution. I also have not looked into the JS side of things.

New patch includes tests for:

1. Opening the Media library plugin in CKEDITOR from a modal
2. Opening the Media library widget from a modal.

I'm adding a test-only patch to prove that things are currently not working and are working with the proposed solution (I hope).

Tests that were previously breaking are now working. Curious to see if the testbot has any other complaints.

OK, so:

1. By design, subsequent modals should close the previously opened modals. An example can be found in the Views UI module, where subsequent modals for example for adding fields, close the previously opened modals.
2. All modals share a single 'namespace', so to speak, in the form of the '#drupal-modal' HTLM ID. As a consequence, all modals are closed upon firing a new modal, including modals that shouldn't be closed.
   E.g. If the views UI were itself to be shown inside a parent modal, then all interaction would break because subsequent modals would delete the parent modal.
3. This patch allows the OpenModalDialogCommand and the CloseModalDialogCommand to 'scope' the modals, by adding a selector.
4. Sometimes modals are rendered by the ModalRenderer, which just receives a render array. This is the case for the modals opened by CKEditor buttons.

   It's well beyond my level of understanding how we should make ModalRenderer aware of the scope of the modal: For now I've added a render array property called '#modal_selector', which can be used to pass the scope of modal, which should probably be declared somewhere. Maybe even required with a @trigger_error.

5. In this patch I've only 'scoped' the modals in the Media Library module, but also the modals in Views UI could be scoped by adding corresponding identifier to ViewsFormBase.php. That way you would be able configure views within a modal.
6. Currently, if no scope is defined, the modal gets the original '#drupal-modal' global scope, meaning that it cannot be used inside another modal.

Questions?

1. Should the 'scope' be compulsory for all modal interactions?
2. If there is no scope present, and #drupal-modal is used, should there be a smarter error reporting or something showing a message that the modal is not allowed in another modal, instead of just breaking?
3. How should the ModalRenderer know of the scope?

Looking forward to hear any feedback

With a bug, all tests should pass now. 2741877-46-48.patch is the interdiff.

Thanks for working on this nuez!

+++ b/core/lib/Drupal/Core/Render/MainContent/ModalRenderer.php
@@ -33,7 +33,8 @@ public function renderResponse(array $main_content, Request $request, RouteMatch
+    $modal_selector = $main_content['#modal_selector'] ?? NULL;

+++ b/core/modules/media_library/src/Form/AddFormBase.php
--- a/core/modules/media_library/src/MediaLibraryUiBuilder.php
+++ b/core/modules/media_library/src/MediaLibraryUiBuilder.php

+++ b/core/modules/media_library/src/MediaLibraryUiBuilder.php
@@ -122,6 +122,7 @@ public function buildUi(MediaLibraryState $state = NULL) {
+        '#modal_selector' => '#modal-media-library',

Is this a new default template variable? This would need some documentation.

How would I set this scope selector when using data- attributes? i.e

        'class' => ['use-ajax', 'button', 'button--small'],
        'data-dialog-type' => 'modal',
        'data-dialog-options' => Json::encode(['width' => 400]),

Is this a new default template variable? This would need some documentation.

This definitely needs documentation. I see #modal_selector as a render array property like #cache, which would possibly be required the Modal Renderer. However, even if 'scoping' the modals is the right approach, there are probably still dozens of different solutions to this. I think the maintainers of the different subsystems involved need to say something about this from here.

How would I set this scope selector when using data- attributes? i.e

Maybe the scope doesn't have to be set in data attributes, modals created with the data attributes might get their own scope internally.

Maybe the scope doesn't have to be set in data attributes, modals created with the data attributes might get their own scope internally.

As long as it's predictable I think that could work as well!

Amazing work. I just came across this issue today when using Linkit and Media Library together, and this patch solves that issue + tests pass. Really nice!

I just left a couple of tiny comments. Neither of them are blockers or anything that should break, just clarity changes. Happy for them both to be ignored if people disagree.

This will also need a change record I assume.

I'll leave this at needs review for now. But functionality wise this is RTBC from me.

1. +++ b/core/lib/Drupal/Core/Ajax/OpenModalDialogCommand.php
   @@ -29,10 +29,13 @@ class OpenModalDialogCommand extends OpenDialogCommand {
   -    parent::__construct('#drupal-modal', $title, $content, $dialog_options, $settings);
   +    parent::__construct($selector ?: '#drupal-modal', $title, $content, $dialog_options, $settings);
   

   Tiny nitpick - Can we take the ternery out of the function call?

   Happy to be overridden on that. I'm just not a fan of logic in function calls personally.

2. +++ b/core/lib/Drupal/Core/Render/MainContent/ModalRenderer.php
   @@ -33,7 +33,8 @@ public function renderResponse(array $main_content, Request $request, RouteMatch
   +    $modal_selector = $main_content['#modal_selector'] ?? NULL;
   +    $response->addCommand(new OpenModalDialogCommand($title, $content, $options, NULL, $modal_selector));
   

   Even though we can see the doc comment I think it could be valuable to add an inline comment here.

tested @DanielVeza patch with Linkit and Media Library. Working well! +1 Ta!

@nuez ... Thank you for the patch! @DanielVeza thank you for testing it!

I have been following this issue for a while now and trying to code something myself without any real luck.

I am experiencing a problem with the patch though. I am running Drupal 8 v8.8.4 with your patch.

Here are the steps to reproduce my problem:

• Create new basic page.
• In CKEditor (WYSIWYG) I select the "Insert from Media Library" icon any my first Modal/Dialog opens.
• If I choose to "Add file"
• Then choose my image, add details, and then open addition modal/dialog by adding a link in the Caption CKEditor.
• This all works great, because the multiple modal/dialog issue is resolved by the patch and the background/parent dialogs are not getting closed on me.
• Then I click Save on Add Link dialog and then Save again... I see the new media I just added and it's checkbox is preselected.
• When I click "Insert Selected" it does not insert the added media into the original CKEditor.

I am able to re-open the "Insert from Media Library" and re-selected the added media... then if I click "Insert Selected" it puts it into the CKEditor properly.

Somehow it seems that the first modal/dialog is losing its callback ability. Maybe something with scope... not sure. If we're able to get this functionality back with this patch it will solve everything we're trying to do with our team!

Thank you in advance for any help or advice you can provide.

Tiny thing. @nuez made the patch and deserves all the kudos. I just reviewed it :D.

I'll have a look at that issue when I get some time. I don't think I was seeing that happen but I'll double check.

Patch #48 works, but I get the following errors when using Drupal core v. 8.8.4 Media Library and Focal Point v. 8.x-1.2:

Warning: unlink(/Users/*/Desktop/sites/*/sites/default/files/styles/*/public/2020-04/*.jpg): No such file or directory in Drupal\Core\File\FileSystem->unlink() (line 124 of /Users/*/Desktop/sites/*/core/lib/Drupal/Core/File/FileSystem.php)

Failed to unlink file 'public://styles/*/public/2020-04/*.jpg'.

Using Focal Point 1.4 with the latest Media Library patch (https://www.drupal.org/project/focal_point/issues/3094478), this works almost perfectly. The only oddity is that the AJAX generates additional '0 of 1 item selected' messages when previewing. Screenshot attached.

I'm also getting the weird message in Media Library as @R_H-L.

@DanielVeza and @nuez Obviously a lot has been happening in our lives and around the world as a whole.

I just wanted to see if either of you has been able to review the issues that have come forth and any ideas of how to resolve them?

My specific issue is outlined here: https://www.drupal.org/project/drupal/issues/2741877#comment-13577182

Thank you in advance and I totally understand if your attention has been needed elsewhere.

@BinaryBlock Thanks for your feedback. I can actually reproduce your error and it definitely needs to be addressed in this issue:

Maybe you can write a test to prove that this use case hasn't been solved in the patch of #48?

I also think that, even though we need to address this particular case described in #54, it's equally important that we get some reviews/guidance from a core maintainer or contributor to see if this is the right approach in the first place.

Another thing this patch currently doesn't fix is Models with the same scope. This is an Edge case where we have a site that has a Thumbnail (Media library) field on a Remote Video Media Entity.

So:

1. Inserting Media into the WYSWYG opens the first Media Library model.
2. Uploading a new Remote Video works and opens the edit form correctly
3. Trying to upload a new image to the Thumbnail field opens a new Media Library
4. This replaces the original Media library model, which closes when you've finished, without uploading the video.

Thank you
patch #48 is working

Tested #48 on 9.1, its working fine
This can be moved to RTBC

Rerolled #48 for 9.2.x

Sorry - corrected without the offsets from originally applying #48

Has #62 been tested with this patch?

what patch to use with D9.0.8 with webform 6 alpha? #70 is not helping.

Thanks for the work @nuez.

Without bike shedding too much and as posted in #40, there are two issues at hand:
1) Core allowing nested modals
2) Places where modals break (ckeditor, media, etc)

Looking at the patch to allow the selector to specified, from the Close/Open Dialog this should be a quick patch that removes the hard coded reference. This is strictly copy and paste from @nuez' patch.

Once that patch is in, we could track the more complex interactions and bugs (media) have more comprehensive tests and issues that track other usability issues, where it needs to be enhanced, etc.

NOTE: the patch will not fix the media issues and will not solve the issues, it will just allow those issues to be solved future patches on separate items.

@wim @xjm, thoughts? This feels like an issue that will suffer a death by paper cuts if not segmented and focused.

+1 for #73 approach, as this will also allow to resolve issues in custom code.

Adding a Media Initiative tag.

I got the same issue as described in #62

I have a media library field attached to a media entity (Media Type "File" has a field for a "Preview Image"). So if I want to set the file with media library, it opens the modal and I can upload a file. If I want to set the preview image, I am getting an ajax error:

Drupal\Core\Form\FormAjaxException: in Drupal\Core\Form\FormBuilder->buildForm() (Line 340 in /web/core/lib/Drupal/Core/Form/FormBuilder.php).

Hear a reroll patch.

#77 applied fine for me to the current 9.2.x-dev but there is still a problem when using with CKEditor and adding links. These were my test steps, which are similar to #54:

1. Set up embedding media with CKEditor.
2. Enable a 'Text (formatted)' field on the Media library form mode of Image Media type.
3. Create Basic page and switch to a Text format that has the "Insert from Media Library" on the CKEditor toolbar.
4. Click 'Insert from media library' button.
5. Browse...
6. Select image.
7. Add required Alternative text.
8. 'Save and select'. Image appears in media library, pre-selected. (unlike the problem in #54)
9. 'Insert selected'. Image appears embedded in CKEditor.
10. Repeat for another new image but this time insert a link in the 'text (formatted)' field using the CKEditor link button (I tried 'https://www.drupal.org' and a couple of other links).
11. When I click "Save" nothing happens. The dialog remains on screen.
12. When I dismiss the dialog by clicking "X" it returns directly to the 'Create Basic page' page and the media item did not get added.

Adding links still works correctly when adding image media directly at /media/add/image.

This is a very new test site with little added. No Editor Advanced Link or Linkit installed.

While I'm here I'll link this issue with the Entity Reference Tree Widget that I'm hoping might get resolved by a fix for this issue. #77 unfortunately does not help that case either.

Patch #70 does not allow new media to be inserted by either "Save and select" > "Insert selected" or by "Save and insert". But it does accept a link before that failure, and that link is remembered.

@Nick Hope , looks like the problem is with the plugins refocusing to the editor if text or entity is left unselected. I have added a solution for the link plugin, it is working for the scenario you mentioned.

Please review.

Re-rolled patch #82, Please review.

Thanks. I tried patches #81 and #82 in both Drupal 9.1.8 and 9.3.0-dev. Unfortunately in all cases the problem I described in #78 still remains. They also don't help the case of an Entity Reference Tree widget (modal) launched from a Hierarchy Manager modal (which is the issue that brought me here).

Tested #77 and #82 with Drupal 9.2, with a modal node form using class ajax and data-dialog-type="modal". In this node i have a media field with media library widget.

It doesn't work, just after i select the media it closes the current modal (the media modal) and nothing more happens.

Tested latest patch on Drupal 9.2.1, It doesn't work. If you add a ckeditor field in a custom modal form, and open this form, when you clic on url button or image button, the form is fully closed. I have tested the other patchs, but none works.

I'd like to try and reiterate my point in #73. There are complexities tied with the approaches in testing vs the simple ability to add an id for the open action.

Is it okay to separate the tasks and the areas of concerns?

Bumping this issue again.

Is there any further development on this? I still cannot launch a link modal from a ckeditor wysiwyg inside a modal, my use case:

- List of media references
- Click the 'pencil'-icon, this opens up the media edit form in a modal
- Attempt to add a link in the custom description field (with basic html or something on)
- The initial modal closes, and the other modal is unable to add the link to anything since the ckeditor field is gone.

I've tried the latest patches, and these sadly do not change this as @Nick Hope mentioned.

Could this be in priority? Because I see this as a major bug!

@AJV009 can you expand on why you feel this is major

Per the issue priorities

Major bugs include those that:

• Interfere with normal site visitors' use of the site (for example, content in the wrong language, or validation errors for regular form submissions), even if there is a workaround.
• Trigger a PHP error through the user interface, but only under rare circumstances or affecting only a small percentage of all users, even if there is a workaround.
• Render one feature unusable with no workaround.
• Block contributed projects with no workaround.
• Cause a significant admin- or developer-facing bug with no workaround.
• Cause user input to be lost, but do not delete or corrupt existing data.
• Cause test failures in environments not supported by the automated testing platform.

Does this fall into one of those categories? If so can you update the issue summary to expand on the impact.

Thanks!

Re-rolled "2741877-48.patch" patch from https://www.drupal.org/project/drupal/issues/2741877#comment-13563283 #48 from this thread.

Tested and worked on Drupal 9.3.9

I clearly understand we have shifted our focus to a more reliable solution, but until that happens we need to have something.

My patch might look a bit fuzzy, because I re-rolled it in just few seconds I was SUPER late for something when I was re-rolling the patch on a DrupalPod instance!

Thanks @nuez

And yes @larowlan, I might be able to assume that the problem falls under one of these
- Render one feature unusable with no workaround.
- Cause user input to be lost, but do not delete or corrupt existing data.

We have a pretty simple code that adds ['use-ajax'] and 'data-dialog-type' => ['modal'] to certain contextual links.

Everything works fine until I open another modal such as a media entity edit dialog box, when it opens the previous modal closes automatically weird right? therefore when I click on insert in the media library dialog box it closes as expected BUT the data which I inserted just vanishes, I couldn't see the previous window through which we opened this media library dialog box. MAGIC!LOST!

This causes the use-ajax feature unusable when used with media library modal.
Not just use-ajax being unusable but the data which I uploaded goes missing as well.

I made my last comment before I thought of reapplying neuz #48 patch.
So sorry about changing that the priority without thinking much.
Reverting the change since we have a workaround for now.

Since we have a workaround now maybe it can go to Normal.
I am really sorry about causing the mess here.

Hey @AJV009 no mess at all. If there's user input lost, then it is definitely major

Updated the summary.

#91 patch not working for me when editing a media image, which opens modal. Then trying to add link (via ckeditor) to the image caption field, which opens the ckeditor's url modal. This closes the media modal and clicking save on the url doesn't do anything. Same issue #88 said.

I want to bump this again. Same issue as described in the last comments (and described by me three years ago)

• In a node with a Reference field to a media entity (image) I want to add a new image
• Clicking on the "add new media" button opens the media library in a modal
• Here I create a new media -> in the modal the media entity form is shown. Our media entity has a textfield "copyright" with CKEditor where we want to link the source of the copyright licence
• Clicking on the link button of the CKEditor opens the link modal / seems to replace the modal of the media library / media entity form
• When click "save" in the now open link modal nothing happens in the browser just an error in browser console: Uncaught TypeError: Cannot read properties of null (reading 'getRanges')

I really have no clue if this is an issue for the Media or CKEditor / Text Editor module.

I'm still seeing an issue here regarding modals and ckeditor5. I've already mentioned this here but I figured it wouldn't hurt to add it here as well.

In ckeditor4 I have a custom plugin that inserts some html onto the page. When clicking the icon on the ckeditor toolbar, a modal pops up with a form. The values of those form fields end up being inserted into the html as attributes. One of those fields is a media field that opens up another modal in an entity browser. I am able to make a selection from that modal, and then submit the original form in the first modal with no issues. The html gets inserted right into the editor.

However, in ckeditor5, this functionality breaks. If I don't click the 'select media' button to open up that 2nd modal, the html gets inserted correctly when I submit the original form. However I can't get any html to appear in the editor once I click the 'select media' button and that 2nd modal opens. Even if I close the entity browser modal without making a selection, it still won't insert any html into the editor when I submit the original form. I see no errors in the console either. I've applied the patches above but no luck.

I can confirm patch 91 doesn't work.

This is on Drupal 9.5.5, but... hoping the patch carries backwards.

I have a media entity which takes vimeo URLs.

But the project needs to upload their own custom placeholder image, but we want to use media for that.

So media library opens a modal for the VIDEO entity type, then clicking the add media button for the place holder image, opens the 2nd modal.

Selecting the image sends you back to the node edit screen ... and, you lose the other work you did to build the video entity.

Updating attributions

Hello,
I noticed a bug like this, so I don't know if it's related to this issue or not, but when I have a field media in a Node that has a media field itself, when I try to add a media in the modale I have a 500 error in the logs.

To reproduce:
- Have activated the modules: Node, Media and Media Library (No Contrib module is involved, I reproduced this Bug on an instance without, I even deactivated CKeditor.)
- Create Media with a media field (and put media library in the formatter form)
- Create a Node which is linked to the media field created (and put media library in the formatter form and also put the field in the formatter of media library)
- Add a content of this Node then add a media on the media field, which allows to open the modal
- Once the modal is open, add media via the upload form, which then allows you to arrive on the modification page
- Click on add media
- Nothing is happening

I have warnings in logs :

Warning: count(): Parameter must be an array or an object that implements Countable in Drupal\media_library\Form\FileUploadForm->validateUploadElement() (line 211 of /var/lib/tugboat/stm/web/core/modules/media_library/src/Form/FileUploadForm.php)

Notice: Undefined index: fids in Drupal\media_library\Form\FileUploadForm->validateUploadElement() (line 211 of /var/lib/tugboat/stm/web/core/modules/media_library/src/Form/FileUploadForm.php)

And i have an error in the console (which I also have in the logs) :

Ran into this today and tested the workaround patch in #91. My test case is a media library widget, where the user uploads a new image media entity. The image entity bundle has a CKEditor-enabled text field. Before the patch, clicking the Link button in the CKE dialog closes the Media Library widget prematurely. After the patch, the link dialog opens on top of it as it should. However, it seems to cause other issues. After closing the link dialog and saving the entity, the "Save and Select" button doesn't close the Media Library dialog anymore.

With the widespread adoption of the HTML dialog element, this should become more a matter of modernizing modals. This element supports chaining dialogs, styling, form awareness, good accessibility, and a host of other benefits.

Simple example:

  <dialog id="first-dialog">
    <p>Greetings, one and all!</p>
    <button data-role="open-dialog" data-target="#second-dialog" onclick="document.querySelector(this.getAttribute('data-target')).showModal()">Show second dialog</button>
    <button data-role="close-dialog" onclick="closest('dialog').close()">Close</button>
  </dialog>
  <dialog id="second-dialog">
    <p>Surprise!</p>
    <button data-role="close-dialog" onclick="closest('dialog').close()">Close</button>
  </dialog>
  <button data-role="open-dialog" data-target="#first-dialog" onclick="document.querySelector(this.getAttribute('data-target')).showModal()">Show dialog</button>

Edit, looks lik ethere is already an issue for this: Issue #2158943

#91 can't be apply on 10.1.

Here's the new version

Reroll of #91 as reroll at #106 was reporting as a corrupt patch file using git apply

D10.0 version for those who need it

More work here to support multiple editor dialogs, the saveCallback was previously only a single function but needs to be a Map keyed by the dialog selector

Fixed linting issues and regression with the 'Save and insert' button in media library (argument order mismatch)

CC Failure.

Also can the proposed solution be updated to include what path was taken.

Rerolled the patch for 11.x as the patch was not getting applied.
Please review.

Uploading patch for 10.1.2 in case someone needs to use the fix for stable version. The change from 11.x patch for this file core/misc/dialog/dialog.js didn't work, because it has already more changes.

The previous patch had a typo core/modules/media_library/src/MediaLibraryEditorOpener.php:

--- a/core/modules/media_library/src/MediaLibraryEditorOpener.php
+++ b/core/modules/media_library/src/MediaLibraryEditorOpener.php
@@ -70,7 +70,7 @@ public function getSelectionResponse(MediaLibraryState $state, array $selected_i
         'data-entity-uuid' => $selected_media->uuid(),
       ],
     ];
-    $response->addCommand(new EditorDialogSave($values));
+   $$response->addCommand(new EditorDialogSave($values, '#modal-media-library'));
 
     return $response;
   }

double $$. This is fixed in the new one. This patch is only to use in the project, there is no need to review it or test it.

2741877-10.1.5-115.patch not working with D10.1.16

Re-rolling the patch for 10.1.6

Tried to fix failures in #118.

I think the re-rolls from #113 onward are not really working, here are a Drupal 10.1 patch and a Drupal 11 patch that are functional.

Fixed failures in #119.

Still needs an issue summary update before reviews.

Reroll for 10.2

Hiding patch, it's the same as in #2741877-121: Nested modals don't work: opening a modal from a modal closes the original.

I should have checked first. 😭

Fixed a bug that was caused by dialogSettings.options property. It led to unexpected behavior on modals. Sometimes they missed actions, not opened at all.

The reroll from #123 seems to be missing a few things. Embedding media doesn't seem to work with this reroll.

I've literally rerolled the d10 patch from #119 and there are a few differences.

Resolved the Linting issue appearing on MR.

I'm facing the issue only when trying to update the content that has two nested modals to work.
When I update the media the parent modal gets closed. And this happens only when I try to update the media there is no issue when I create new content or media.
Facing this error after applying the patch at #126, I have tried almost every patch mentioned here.
I tried on ckeditor4 and as well as on ckeditor5. I'm using Drupal 10.2.6, I'm using entity_embed to upload the media.

Noting here that I’m seeing a JS console error in D10.2.6 (with either patch #126 or the merge request since they are virtually the same) when attempting to embed media in ckeditor5 using a newly uploaded image and clicking the "Save and insert" button with the Media Library advanced UI enabled:

ajax.js?v=10.2.6:1143
An error occurred during the execution of the Ajax response: TypeError: Cannot read properties of undefined (reading 'options')

Attached a static patch file from the Drupal Core 2024-06-20 MR 8105
witch applies to both Drupal 10.3.x and 11.0.x branches
To be used with composer patches

Patch #134 worked perfectly on Drupal 10.3.0. Thanks for the re-roll Rajab.

Attaching a patch file from the MR 8779 which applies to Drupal 10.2.x. (tested on 10.2.7) , to be used with composer patches.
It fixes an Ajax error from happening with the Layout builder modal and Drupal-off-canvas that wasn't fixed by the #126.
The Ajax error was preventing the LB modal from closing.

Patch #137 for Drupal 10.3.x

Patch #137 for Drupal 10.3.x with:

• 025fb5d2 - Ensure dialogSettings variable is a object for dialog:afterclose event listener

Sorry, #140 patch was bad.

Patch #137 for Drupal 10.3.x with:

025fb5d2 - Ensure dialogSettings variable is a object for dialog:afterclose event listener

The patch works in some cases, but I have the same issues as described in #132

None of the patches works for me:

What works:
Without this or any patch, When editing a node which is in modal and then opening a nother modal over it ( media entity browser for uploading media) works so that a modal opens over another modal and it can be closed without closing the parent.

So nested modals are working with media entity browser and node.

What does not work:
When editing a node which is in modal, then another modal opened from ckeditor5 (AI ckeditor5 integration) causes a problem = closing that modal will close all modals. (Uncaught TypeError: dialogSettings is undefined)
Also tried with patch #142 and #134 and they do not fix this issue

Re-roll patch 142

We have another important case that needs to be addressed in this issue. The case involves using a media library widget within a media library widget. When we open a media library from another media library, it replaces the previous one and closes both upon submitting the child media library. For example, in the remote video media form, we added a cover image field with a media library widget. When we click to add media for the cover image, the new media library will replace the old one and will now submit or save or add the remote video or the cover image.

Facing an issue with AI CKEditor 5 integration as well when using this MR/Patch.

Attached a static drupal-core--2025-02-06--2741877--mr-11137.patch file from this point of MR11137
witch applies to Drupal 10.4.x
To be used with Composer Patches

The patch in #149 applies and works as expected for me in drupal 10.4
thanks!

Attached a static drupal-core--2025-02-16--2741877--mr-11137.patch file
from this point of MR11137
witch applies to Drupal 10.4.x
To be used with Composer Patches

Had a better fallback check on Store the save callback to be executed when this dialog is closed

The issue from #146 is resolved in this patch, which applies to Drupal 10.2.x.

If you are using media_library_extend, apply this patch: https://www.drupal.org/project/media_library_extend/issues/3507967.
If you are using media_library_form_element, apply this patch: https://www.drupal.org/project/media_library_form_element/issues/3507968.

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.

let's worry about 11.x first and we'll handle the backports once it's RTBC/committed

Attaching a static drupal-core--2025-06-24--2741877--mr-8105.patch file to this point of MR8105.
To be used with Composer Patches and Drupal 11.2.0

Shouldn't we better get rid of jQuery dialog first? Also see #105. I don't think that we should continue fixing jQuery dialog?

A patch file from 2741877-156-d10.2.patch to Drupal 10.2.12.

It fixes a black screen appears when using the Insert Media option for the second time in the WYSIWYG editor.

Steps to reproduce

1. Create an article
2. On the WYSIWYG, click the Insert Media then the modal will appear.
3. Insert any media or close the modal
4. Click again the Insert Media then no modal will appear and the screen turns black.

I forgot to include the MediaLibraryIds, so I’ve created another file to include them here.

We've ran into the following issue with this patch:

"An error occurred during the execution of the Ajax response: TypeError: Drupal.ckeditor5.saveCallback is not a function"

We've seen this error with contrib modules that provide their own saveCallback, but we didn't do that.

In our case both if-conditions evaluted to FALSE.

$(window).on('editor:dialogsave', (e, values, selector) => {
    if (Drupal.ckeditor5.saveCallback) {
      if (Object.hasOwn(Drupal.ckeditor5.saveCallback, 'has') && Drupal.ckeditor5.saveCallback.has(selector)) {
        Drupal.ckeditor5.saveCallback.get(selector)(values);
      }
      else {
        Drupal.ckeditor5.saveCallback(values);
      }
    }
  });

We figured out the issue was that we had an overridden CKEditor plugin editor config, that did not set the selector (which is provided in this patch), thus generating the fallback "#drupal-modal", which resulted in a failed lookup in the map when inserting the media (leading to the error).

This was fixed by copying over the selector into the overridden configuration.
While this fixes the selector check condition, `Object.hasOwn(Drupal.ckeditor5.saveCallback, 'has')` still is FALSE.

We are unsure if this is only relevant to our custom code or this is an issue with the patch, but we thought this might be worth mentioning.

Are there any reasons this patch checks for `hasOwn` instead of something more generic like `Drupal.ckeditor5.saveCallback instanceof Map` (which worked for us) ?
Maybe `hasOwn` has some implications with inherited objects?
"If the property is inherited, or does not exist, the method returns false"

Re-roll of patch #167 to 10.5.

Note: If the Media Library Edit module is enabled, apply patch #5 from https://www.drupal.org/project/media_library_edit/issues/3305908 to fix an Ajax issue that occurs when opening the modal via the edit button.

We hit this bug in the SVG Image Field module when users try to set a default Media (Vector Image) value via the Field UI "Add Field" (and "Edit Field") modals. Before, these screens loaded as a separate page but now in D11.3 (maybe 11.2 too?). these screens are loaded in a modal.

Steps to reproduce:

1. Add a new Media (Vector Image) field via Structure > Content types > [type] > Manage fields
2. Click "Create a new field" (opens in modal)
3. Configure the field and attempt to set a default value
4. Click "Select media" to choose a default image from the Media Library (opens nested modal)
5. Select an item and click "Insert"
6. Result: The Media Library modal closes, but the parent field configuration modal also closes instead of being restored. All changes are lost.

Workaround: Save the field first without a default value, then edit the field directly at /admin/structure/types/manage/{type}/fields/{field} (not via modal) to configure the default media value.

We've documented this as a known issue in our module project page and in the README

This likely affects any media-related module (including core) that allows setting default Media values via Field UI, not just SVG Image Field.

I can confirm that applying the patch to core from MR 8105 fixes the problem for the "Edit field" modal.

HOWEVER, there is a different error that still prevents setting the default value (fallback media entity) directly when creating the field on the "Add Field" modal. I see ajax error message in JS console: Field field_test_2741877 is unknown. (The field has not been saved yet, so doesn't exist). This might be a separate but related issue.

Full stack trace:

An AJAX HTTP error occurred.
HTTP Result Code: 500
Debugging information follows.
Path: /media-library?js=true&_drupal_ajax=1&ajax_page_state%5Btheme%5D=claro&ajax_page_state%5Btheme_token%5D=_5csUEeAZ0ni-Y_H7sn1eKiGsB1L5o8PVk9P2XTOj28&ajax_page_state%5Blibraries%5D=big_pipe/big_pipe%2Cclaro/drupal.nav-tabs%2Cclaro/global-styling%2Ccontextual/drupal.contextual-toolbar%2Ccore/drupal.active-link%2Ccore/drupal.collapse%2Ccore/drupal.dropbutton%2Ccore/drupal.machine-name%2Ccore/drupal.reset-appearance%2Ccore/drupal.states%2Ccore/drupal.tableresponsive%2Ccore/drupal.textarea-resize%2Ccore/internal.jquery.form%2Ccore/normalize%2Cfield_ui/drupal.field_ui%2Cfield_ui/drupal.field_ui.manage_fields%2Cmedia_library/widget%2Cshortcut/drupal.shortcut%2Ctoolbar/toolbar%2Ctoolbar/toolbar.escapeAdmin%2Cuser/drupal.user.icons%2Ccore/drupal.dialog.ajax&_wrapper_format=drupal_ajax&ajax_form=1&media_library_opener_id=media_library.opener.field_widget&media_library_allowed_types%5Bvector_image%5D=vector_image&media_library_selected_type=vector_image&media_library_remaining=1&media_library_opener_parameters%5Bfield_widget_id%5D=field_test_2741877%3A-default_value_input&media_library_opener_parameters%5Bentity_type_id%5D=node&media_library_opener_parameters%5Bbundle%5D=article&media_library_opener_parameters%5Bfield_name%5D=field_test_2741877&hash=NkU-pbcvEijdE1Ax6MH6TawVrJgfoMyKSeLx4_GQC3A&views_display_id=widget
StatusText: error
ResponseText: The website encountered an unexpected error. Try again later.InvalidArgumentException: Field field_test_2741877 is unknown. in Drupal\Core\Entity\ContentEntityBase->getTranslatedField() (line 628 of core/lib/Drupal/Core/Entity/ContentEntityBase.php). Drupal\Core\Entity\ContentEntityBase->get() (Line: 97)
Drupal\media_library\MediaLibraryFieldWidgetOpener->checkAccess() (Line: 210)
Drupal\media_library\MediaLibraryUiBuilder->checkAccess()
call_user_func_array() (Line: 66)
Drupal\Core\Access\CustomAccessCheck->access()
call_user_func_array() (Line: 160)
Drupal\Core\Access\AccessManager->performCheck() (Line: 136)
Drupal\Core\Access\AccessManager->check() (Line: 113)
Drupal\Core\Access\AccessManager->checkRequest() (Line: 106)
Drupal\Core\Routing\AccessAwareRouter->checkAccess() (Line: 91)
Drupal\Core\Routing\AccessAwareRouter->matchRequest() (Line: 101)
Symfony\Component\HttpKernel\EventListener\RouterListener->onKernelRequest() (Line: 246)
Symfony\Component\EventDispatcher\EventDispatcher::Symfony\Component\EventDispatcher\{closure}() (Line: 206)
Symfony\Component\EventDispatcher\EventDispatcher->callListeners() (Line: 56)
Symfony\Component\EventDispatcher\EventDispatcher->dispatch() (Line: 159)
Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 53)
Drupal\Core\StackMiddleware\Session->handle() (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle() (Line: 32)
Drupal\big_pipe\StackMiddleware\ContentLength->handle() (Line: 118)
Drupal\page_cache\StackMiddleware\PageCache->pass() (Line: 92)
Drupal\page_cache\StackMiddleware\PageCache->handle() (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 53)
Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 54)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 745)
Drupal\Core\DrupalKernel->handle() (Line: 19)

I searched but couldn't find anything referencing the Field field_X is unknown error when setting default media value via modal, so I opened up a separate issue.

I have attached the patch for Drupal 11.3 in case you need that as well 🤓

Same problem here as in #169.

`hasOwn` is not working, `Drupal.ckeditor5.saveCallback instanceof Map` works.

I've added a new commit to solve the issue described at #177 and #169. There's an additional problem solved in this commit. When the first dialog is opened from ckeditor, and that dialog opens a second dialog, the saveCallback is set to null by the dialog:afterclose event handler. I've changed a little bit the logic to keep the backward compatibility but ensuring the map is not set to null when the dialog has been opened from a different selector (therefore, is a nested dialog).

thanks @akalam, for what it's worth your latest commit fixes up my quite unusual edge-case requirements (embedded_content and media within embedded_content)

Having multiple layered modals is NOT an edge-case or untypical. If you're simply using Layout Paragraphs with Media you already have two modals. If using entity reference with entity browser in paragraphs, it may even be three layers.

Long-term I'm very much hoping for a browser-native dialog solution, which supports stacking. So IMHO #2158943: Add a native dialog element to deprecate the jQuery UI dialog should be the way to go. Maybe until then quickfixes here are good enough.

Patch #176 with latest fix (#179)

I am seeing the same thing with a node with a media library that when you edit it - with the media library edit module - opens another media library modal and selecting the media gives an ajax error and don't do anything.

Likely the same as #146, not sure if #156 was added to the latest MR.

Pushed quite a few follow-up commits to the branch. Testing the MR against a self-referencing media model turned up a gap, and closing it surfaced a few test issues in the MR that were keeping the pipeline red.

Note that I am now heavily relying on this MR as well as Media Library Edit and #3561003: Nested modals.

The MR scopes modals by selector but opens every media library level with the static #modal-media-library. That's fine for a library nested inside a different modal (the case testWidgetInModalContext covers), but not a library opened inside another media library, e.g. a media type whose own form has a media reference field on the widget (create/edit an item in the library → its form opens a second library). Both get the same id, so querySelector('#modal-media-library') in closeDialog() matches the outer node and inserting in the inner library tears down both.

I fix it staying within the MR's mechanism: add MediaLibraryState::getModalSelector(), unique per opener for the field_widget opener and the shared id otherwise, and use it at the four field-widget sites that hardcoded the id (MediaLibraryWidget::openMediaLibrary(), buildUi()'s #modal_selector, and the closes in AddFormBase and MediaLibrarySelectForm). The editor opener keeps #modal-media-library.

One subtlety worth flagging: the selector is derived from the opener id + opener parameters, not ::getHash(). The hash also covers the selected type and remaining slots, which change as the user navigates inside the library; since the modal element is created once on open, hashing the full state makes the close selector diverge from the open selector and the modal won't close. A kernel test asserts navigation doesn't change the selector.

Test changes needed to get green (all on the branch):
- MediaLibraryStateTest::testGetModalSelector — new, covers uniqueness + navigation-stability.
- testNestedFieldWidgetModals — new functional coverage for the library-in-library case.
- testWidgetInModalContext — this test shipped without its fixture: it visits /media-library-test-modal and clicks .modal-basic_page, but that route/page didn't exist, so it couldn't run. Added a small controller + route, and an assertion that the outer modal survives the insert.
- ckeditor5 MediaLibraryTest (testButton/testAllowedMediaTypes/testAlt) — updated the assertions from #drupal-modal to #modal-media-library to match the editor library modal id this issue moves.
- demo_umami AssetAggregationAcrossPagesTest — bumped the ScriptBytes baseline for the added editor/dialog JS.

The only remaining red jobs are unrelated to this issue and also fail on main: UnpackRecipeTest (composer now emits an extra "No security vulnerability advisories found." line) and the next-major-PHP run (phpspec/prophecy "Returning a value from a destructor is deprecated" on PHP 8.6-dev, allowed-failure).

Oh, an I also added https://git.drupalcode.org/issue/drupal-2741877/-/tree/2741877-nested-mo... as a not so formal backport but including all relevant fixes so that it can be applied there.

Hi,

I'm still on D10.6, and I also add some issues with the patch #170, where the media modal was opening, but once you've submitted the "add form", it wouldn't close.
The new media was created, but I had to close the modal and reopen it to select the created media.

With the following patch, everything seems to be working fine.

@seb_r would you consider trying the MR so we concentrate all efforts on a a single place? Thanks!

Hello,

For the ones impacted by the media library won't opening, perharps this patch will help you (cf https://www.drupal.org/project/metatag/issues/3074777#comment-16692826). I just switcher the default display_id of the view media_library from grid to table when opening via AJAX.