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

Created on 3 June 2016, almost 9 years ago
Updated 19 January 2023, about 2 years ago

Issue Summary

  • #46 Describes the problem as to why the issue is happening which generally narrows down to hard coding of html ids for modal opening and closing command.
  • #48 Provides a solution so that each open and close command can work with separate modal ids, It also provides a solution for media library form.
  • #73 Extracts the open and close command modifications from #48 so that it can be merged with core first and then for each individual case a separate issue and patch can be added.

Proposed resolution

  • Review and merge #73 or #48 in core.
  • Create individual issues for each possible use case like media library, Ckeditor etc.
  • Add the change records.

Original Post

I am currently looking into running CKEditor in a modal dialog using the Drupal Modal API.

At this moment it is working OK, but there is a problem with the link and image modals which can be triggered from the CKEditor interface. Those also use the Modal API and when triggering one of these functions, the modal containing CKEditor is fully closed.

Is there any way to open de link/image modal without closing the CKEditor modal? It seems like it is possible to define the HTML element to which the modal should append when triggering. When overriding this setting ('appendTo' => '#foo'), even the link/image modals are appended to #foo (instead of the default '#drupal-modal').

Any ideas on how to get this to work?
Thanks for your support.

🐛 Bug report
Status

Needs work

Version

10.1

Component
Ajax 

Last updated about 11 hours ago

Created by

🇧🇪Belgium vollepeer

Live updates comments and jobs are added and updated live.
  • Needs usability review

    Used to alert the usability topic maintainer(s) that an issue significantly affects (or has the potential to affect) the usability of Drupal, and their signoff is needed. When adding this tag, make it easy to review the issue. Make sure the issue summary describes the problem and the proposed solution. Screenshots usually help a lot! To get sign-off on issues with the "Needs usability review" tag, post about them in the #ux channel on Drupal Slack, and/or attend a UX meeting to demo the patch and get direct feedback from designers/UX folks/product management on next steps. If an issue represents a significant new feature, UI change, or change to the general "user experience" of Drupal, use Needs product manager review instead. See the scope of responsibilities for product managers.

  • Needs accessibility review

    Used to alert the accessibility topic maintainer(s) that an issue significantly affects (or has the potential to affect) the accessibility of Drupal, and their signoff is needed (see the governance policy draft for more information). Useful links: Drupal's accessibility standards, the Drupal Core accessibility gate.

  • Needs change record

    A change record needs to be drafted before an issue is committed. Note: Change records used to be called change notifications.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇺🇸United States alphex Atlanta, GA USA

    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.

  • 🇮🇳India gauravvvv Delhi, India

    Updating attributions

  • 🇫🇷France Fabsgugu

    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) :

  • 🇺🇸United States bkosborne New Jersey, USA

    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.

  • 🇺🇸United States R_H-L

    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="this.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>
    
  • 🇫🇷France arnaud-brugnon

    #91 can't be apply on 10.1.

    Here's the new version

  • Status changed to Needs review over 1 year ago
  • last update over 1 year ago
    Patch Failed to Apply
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

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

  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    D10.0 version for those who need it

  • last update over 1 year ago
    Custom Commands Failed
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    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

  • 🇪🇸Spain nuez Madrid, Spain
  • Open on Drupal.org →
    Environment: PHP 8.2 & MySQL 8
    last update over 1 year ago
    Waiting for branch to pass
  • Open on Drupal.org →
    Environment: PHP 8.2 & MySQL 8
    last update over 1 year ago
    Waiting for branch to pass
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

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

  • last update over 1 year ago
    Custom Commands Failed
  • Status changed to Needs work over 1 year ago
  • 🇺🇸United States smustgrave

    CC Failure.

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

  • last update over 1 year ago
    Patch Failed to Apply
  • 🇮🇳India _utsavsharma

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

  • last update over 1 year ago
    29,872 pass, 3 fail
  • 🇩🇪Germany a.dmitriiev

    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.

  • 🇩🇪Germany a.dmitriiev

    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.

  • last update over 1 year ago
    Patch Failed to Apply
  • last update over 1 year ago
    Patch Failed to Apply
  • 🇩🇪Germany Rar9

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

  • 🇩🇪Germany a.dmitriiev

    Re-rolling the patch for 10.1.6

  • last update over 1 year ago
    Custom Commands Failed
  • 🇮🇳India _utsavsharma

    Tried to fix failures in #118.

  • last update over 1 year ago
    30,483 pass, 5 fail
  • 🇪🇸Spain pcambra Asturies

    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.

  • Status changed to Needs review over 1 year ago
  • last update over 1 year ago
    Custom Commands Failed
  • 🇮🇳India _utsavsharma

    Fixed failures in #119.

  • last update over 1 year ago
    30,600 pass, 3 fail
  • Status changed to Needs work over 1 year ago
  • 🇺🇸United States smustgrave

    Still needs an issue summary update before reviews.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update over 1 year ago
    Custom Commands Failed
  • 🇺🇦Ukraine dinazaur

    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.

  • last update about 1 year ago
    Patch Failed to Apply
  • 🇪🇸Spain nuez Madrid, Spain

    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.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 5.3 & MySQL 5.5
    last update about 1 year ago
    Composer error. Unable to continue.
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update about 1 year ago
    Custom Commands Failed
  • 🇺🇸United States darren oh Lakeland, Florida

    Darren Oh made their first commit to this issue’s fork.

  • Pipeline finished with Failed
    11 months ago
    Total: 183s
    #174732
  • Pipeline finished with Failed
    11 months ago
    Total: 172s
    #174768
  • 🇮🇳India mithun s Bangalore

    Mithun S made their first commit to this issue’s fork.

  • 🇮🇳India mithun s Bangalore

    Resolved the Linting issue appearing on MR.

  • Pipeline finished with Failed
    11 months ago
    Total: 568s
    #175092
  • 🇺🇸United States darren oh Lakeland, Florida
  • 🇮🇳India deepaksingh05

    I'm facing the issue only when trying to update the content that has two nested modals to work.
    Facing this error after applying the patch at #126 🐛 Nested modals don't work: opening a modal from a modal closes the original Needs work , 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.

  • 🇺🇸United States daniel korte Brooklyn, NY

    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')
  • 🇯🇴Jordan Rajab Natshah Jordan

    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

  • 🇧🇷Brazil carolpettirossi Campinas - SP

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

  • Pipeline finished with Failed
    9 months ago
    Total: 474s
    #225047
  • Pipeline finished with Failed
    9 months ago
    Total: 510s
    #225062
  • 🇧🇷Brazil andre.bonon

    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.

  • 🇫🇷France xavier.masson Haute-Normandie

    xavier.masson made their first commit to this issue’s fork.

  • Pipeline finished with Failed
    8 months ago
    Total: 473s
    #239150
  • 🇧🇷Brazil renanmfd

    Patch #137 for Drupal 10.3.x

  • 🇧🇷Brazil renanmfd

    Patch #137 for Drupal 10.3.x with:

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

    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

  • 🇨🇭Switzerland luksak

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

  • 🇫🇮Finland anaconda777

    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

  • 🇯🇴Jordan oways23

    Re-roll patch 142

  • 🇯🇴Jordan Mutasim Al-Shoura

    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.

  • 🇯🇴Jordan Rajab Natshah Jordan

    Facing issue with AI ckeditor5 integration too.

  • Pipeline finished with Failed
    about 2 months ago
    Total: 156s
    #417169
  • Pipeline finished with Failed
    about 2 months ago
    Total: 198s
    #417170
  • Pipeline finished with Failed
    about 2 months ago
    Total: 175s
    #417175
  • Pipeline finished with Failed
    about 2 months ago
    Total: 189s
    #417192
  • 🇯🇴Jordan Rajab Natshah Jordan

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

  • 🇺🇸United States loze Los Angeles

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

  • First commit to issue fork.
  • Pipeline finished with Failed
    about 2 months ago
    Total: 150s
    #419196
  • Pipeline finished with Failed
    about 2 months ago
    Total: 104s
    #419200
  • Pipeline finished with Failed
    about 2 months ago
    Total: 106s
    #419201
  • Pipeline finished with Failed
    about 2 months ago
    Total: 164s
    #419202
  • Pipeline finished with Failed
    about 2 months ago
    Total: 171s
    #419205
  • Pipeline finished with Failed
    about 2 months ago
    Total: 650s
    #419208
  • Pipeline finished with Failed
    about 2 months ago
    Total: 174s
    #419318
  • Pipeline finished with Failed
    about 2 months ago
    Total: 124s
    #425615
  • 🇯🇴Jordan Rajab Natshah Jordan

    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

  • 🇯🇴Jordan Mutasim Al-Shoura

    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 Nested media library Active .
    If you are using media_library_form_element, apply this patch: https://www.drupal.org/project/media_library_form_element/issues/3507968 Nested media library Active .

  • 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.

  • First commit to issue fork.
  • 🇫🇷France nod_ Lille

    nod_ changed the visibility of the branch 2741877-10-4-x to hidden.

  • Pipeline finished with Failed
    27 days ago
    Total: 109s
    #441799
  • 🇫🇷France nod_ Lille

    nod_ changed the visibility of the branch 2741877-nested-modals-10.2 to hidden.

  • 🇫🇷France nod_ Lille

    nod_ changed the visibility of the branch 2741877-nested-modals-10.3 to hidden.

  • 🇫🇷France nod_ Lille

    nod_ changed the visibility of the branch 2741877-nested-modals-10.1 to hidden.

  • 🇫🇷France nod_ Lille

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

Production build 0.71.5 2024