Problem/Motivation
When using CKeditor in a jQuery UI dialog with the modal option set as TRUE
, it is not possible to CKeditor native dialogs. The text fields within the modal dialog are not clickable.
Proposed resolution
Extend the dialog by using the jQuery UI native widget factory feature. By the way, jQuery UI itself uses exactly the same example for demonstration purposes of extending _allowInteraction( event )
method, see https://api.jqueryui.com/dialog/#method-_allowInteraction:
Modal dialogs do not allow users to interact with elements behind the dialog. This can be problematic for elements that are not children of the dialog, but are absolutely positioned to appear as though they are. The _allowInteraction() method determines whether the user should be allowed to interact with a given target element; therefore, it can be used to whitelist elements that are not children of the dialog but you want users to be able to use.
Remaining tasks
Review a patch
User interface changes
None
API changes
None
Data model changes
None
Comment | File | Size | Author |
---|---|---|---|
#71 | 3065095-71-1.0.x.patch | 7.19 KB | SerkanB |
#56 | 3065095-56.patch | 7.34 KB | Taran2L |
| |||
Screenshot 2019-07-01_10-13-23-064.png | 75.26 KB | PascalMortier |
Issue fork drupal-3065095
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:
Comments
Comment #2
idebr CreditAttribution: idebr at iO commentedThis seems to match the 'official' behavior on https://ckeditor.com/ckeditor-4/demo/#article where some cells are disabled. However, Seven does not display the 'disabled' styling.
Comment #3
PascalMortier CreditAttribution: PascalMortier commentedIt has nothing to do with disabled cells. If I use it in a normal form everything is working. As soon as I use the editor in a modal form, the input fields for cell properties are not working.
Comment #4
cilefen CreditAttribution: cilefen commentedIs there anything in the browser console?
Comment #5
PascalMortier CreditAttribution: PascalMortier commentedNo I can't see any error.
Select lists are working, but text fields doesn't.
Comment #6
olmanslm CreditAttribution: olmanslm commentedI also could reproduce this issue, There is no console log errors. Also when we click the table icon in the CKeditor the auto focus is also missing, it should focus the first input in the modal.
Also I noticed that the CKeditor modals works correctly outside the layout builder.
Comment #7
olmanslm CreditAttribution: olmanslm commentedHello,
I was able to reproduce the error and come with a solution. The Error es caused by how the CKeditor modal element is added to the page. In order to make the two modals work together, one solution could be to append the CKeditor modal and its overlay inside the #layout-builder-modal
I extend the module functionality with the following fix:
Also, we can and a Drupal behavior by adding the following:
Hope this fix and guide you to implement the proper fix.
Comment #8
PascalMortier CreditAttribution: PascalMortier commentedHi olmanslm,
Thank you a lot for your feedback.
However I was not working with the layout builder, I stripped your code and placed it in a js file of my custom module.
Now everything is working, thank you !
I only used this part in my module :
In this example you have to replace '#my-div' with the div that is used as the parent modal.
Comment #9
olmanslm CreditAttribution: olmanslm commentedHello PascalMortier,
Glad that it helped, I also noticed some styling issue and this is a update to that code:
Comment #10
Wim LeersWe now can write tests that can reproduce even complex JS or element reachability problems like this one. So let's write a failing test!
Comment #11
johnwebdev CreditAttribution: johnwebdev commentedTo reproduce without Layout Builder Modal module you could do something like:
One thing more; it is the jQuery UI dialog modal option that makes the table not clickable.
Comment #12
johnwebdev CreditAttribution: johnwebdev commentedComment #13
johnwebdev CreditAttribution: johnwebdev commentedI intend to work on the failing test today :)
Comment #14
johnwebdev CreditAttribution: johnwebdev commentedFailing test
Comment #16
SoulReceiver CreditAttribution: SoulReceiver commentedThe fix provided in #9 works great for when you add a new table, but if you want to edit an existing table and get the table properties by right-clicking then this is still broken.
Comment #17
knaffles CreditAttribution: knaffles commentedI tried working with #9 but wasn't able to adapt it to work when there are multiple toolbar buttons that launch a modal.
I added the snippet below into a library in a custom module, with a dependency on core/jquery.ui.dialog. This took care of the issue for me, and also works for other buttons that launch a modal, such as the anchor button, which had the same issue. That said, I'm not sure where in core this would belong as a patch.
This snippet comes from this comment on stack overflow: https://stackoverflow.com/a/28086465
Comment #19
pcorbett CreditAttribution: pcorbett at Redfin Solutions, LLC commented#17 works well by implementing a custom module and library with the referenced JS.
Comment #20
WebbehComment #21
WebbehOne of my colleagues was able to mediate this issue using #17, thank you for sharing! They noted that:
I wanted to highlight a consideration of how to take this fix snippet JS and create a patch out of it as our next steps?
Comment #22
komlenic CreditAttribution: komlenic commentedI encountered another instance of this issue when using CKEditor Media Embed with Layout Builder Modal.
The solution in #17 resolves this problem for me.
Comment #24
AaronChristian CreditAttribution: AaronChristian at ImageX commentedConfirming that #17 works for me as well.
Make sure you don't forget to add the dependency for
core/jquery.ui.dialog
like mentioned above the original snippet.Thanks!
Comment #25
Luke.LeberConfirmed that #17 works as advertised on Drupal 8.9.9!
Comment #26
malmeida287 CreditAttribution: malmeida287 commentedConfirming that #17 works for me as well.
Thanks @knaffes !
Comment #27
YesCT CreditAttribution: YesCT at Lullabot for Iowa State University commentedThanks so much for this issue, and the snippet in #17.
I wasn't sure what todo with it... so I'll share the diff I ended up with, in case it helps others see a bigger picture too.
Caution, this is not a diff that can be "applied" to anyone's specific code base, but should indicate one example of how to get the snippet into code.
---
also, I'm making this related to https://www.drupal.org/project/drupal/issues/3113649 since that issue is re-writing the js core uses. So depending on how that works out, the "fix" here might not be needed eventually.... maybe.
I made a comment on that issue too, so they are aware of this test case if they want to include this in their testing while working on re-writing core.
Comment #28
alexharries CreditAttribution: alexharries commentedThank you, @YesCT - your diff was really helpful.
Edit: unfortunately, using the libraries-extend option doesn't work with Entity Reference with Layout, which we're using for Paragraphs.
Instead, we had to add the function either into an existing Js file in one of our existing custom modules, or as a new Js file in a new or existing module.
We went with the first option, which gave us this:
redactived8platform_dragdrop/js/redactived8platform-dragdrop-layout-builder-fix.js:
redactived8platform_dragdrop/redactived8platform_dragdrop.libraries.yml
... just in case this detail helps the next person dealing with this shiny-new DrupalWTF... ¯\_(ツ)_/¯
Thanks - Al :)
Comment #29
ndf CreditAttribution: ndf as a volunteer and commentedComment #31
ndf CreditAttribution: ndf as a volunteer and commentedThis merge request has:
- the existing automated test from johnwebdev comment #14
- the existing snippet from #17 that copies the interaction settings to the modal
- added code the snippet runs when the dialog opens. My goal was that a custom new library is not needed.
Comment #32
ndf CreditAttribution: ndf as a volunteer and commentedNow many tests fail due to the "core/jquery.ui.dialog" dependency. Not sure how to proceed with this at the moment. Maybe move this issue to https://www.drupal.org/project/jquery_ui_dialog? And then let affected contrib modules set a dependency to that module?
Still putting this to needs review because I would like to know if this patch fixes the issue in various scenario's.
Comment #33
saso.sotlar CreditAttribution: saso.sotlar commentedUploaded (locked) version of a patch from latest MR (comment #31).
Patch applies successfully on 9.2.6.
Comment #35
Grayle CreditAttribution: Grayle at Dropsolid commentedLatest patch causes a lot of console errors when just clicking anything in any layout builder modal.
Comment #36
Poindexterous CreditAttribution: Poindexterous commentedI'm getting the same errors as @grayle with patch #31 running drupal core 9.2.7, I also ran across a "permission denied" error when attempting to insert a media item into a field with the layout builder modal. Removing the patch in this issue restored access to insert the media image into the field. The error I received was
Path: /admin/content/media-widget/image?directory=401&name=&sort_by=created. Drupal\Core\Http\Exception\CacheableAccessDeniedHttpException: The opener ID parameter is required and must be a string. in Drupal\Core\Routing\AccessAwareRouter->checkAccess() (line 117 of /app/docroot/core/lib/Drupal/Core/Routing/AccessAwareRouter.php).
Comment #37
seeduardo CreditAttribution: seeduardo at Cyber-Duck commentedJust confirming successful application of the patch in #33 on Drupal 9.2.8.
Comment #39
chrisgross CreditAttribution: chrisgross commentedThis patch applies to Drupal 9.3.4, but does not appear to do anything. Both before and after applying the patch, I can change table row and column properties if I select the table and use the Table button in the editor. However, if I access this via the Table Properties right-click menu, those properties are not editable.
Edit: Okay, so it looks like this patch does fix the issue where modal dialogs cannot be focused at all. So is this unrelated to being not able to change the number of rows and columns after the table is created? I guess we couldn't do this in 7 either, so if that's the case then I can confirm that this patch works.
Comment #40
Grayle CreditAttribution: Grayle at Dropsolid commentedLocked patch based on last MR status. All I did was revert the linter removing "arguments", that's what was causing all the console errors.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functi...
Also used plain diff instead of the email formatted diff the previous patch was based on. You can get the plain diff version using the download button on the MR's status page.
Comment #41
immaculatexavier CreditAttribution: immaculatexavier as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented#40 Custom Commands Failed. Changing the status to Need Work
Comment #42
Wim LeersHuh, I wonder if #3274937: Get CKEditor 5 to work in (modal) dialogs would need to be applied to CKEditor 4 too 🤔
Comment #45
Taran2LComment #49
Taran2LSo, I've created a new MR against 9.5.x with contains the same code as the previous one.
Now CI fails due to #3278415: Remove usages of the JavaScript ES6 build step, the build step itself, and associated dev dependencies
Comment #50
Taran2LComment #51
vlad.dancerThanks @Taran@L. Tested MR863, works as expected.
Comment #52
Taran2L@vlad.dancer, thanks for the RTBC, but the last MR does not pass the tests.
Discussed the matter with @nod_ in Slack, thus the new patch that mimics the CKE5 fix from #3274937: Get CKEditor 5 to work in (modal) dialogs and #3301631: Regression with CKEditor 35.0.1 and modal dialogs
Comment #53
Taran2LComment #54
Taran2LComment #55
Taran2LComment #56
Taran2LComment #57
nod_probably should be a 9.x patch since ckeditor will be removed in 10.x.
Comment #58
Taran2L@nod_, it won't be removed in 10.0.x, CKE4 will be supported till the end of 2023 (afaik), so it will be (probably) removed in 10.1 or even in 10.2 (or even 10.3)
Comment #59
nod_It'll live in contrib but the plan is to remove it from core in 10.0.0, see #3304326: Deprecate CKEditor 4 module in 9.5
Comment #60
Taran2LComment #61
nod_(that's because if it makes it in D10, we can't remove it until D11 because of our support/BC rules. And because CKE4 will be out of support before the end of the D10 support it would implicitly means that we'll need to support CKE4 over the life of D10, which no one wants to do. Major version are the only time we can remove existing modules from core)
Comment #62
Taran2LComment #64
Taran2LComment #66
Taran2LComment #67
quietone CreditAttribution: quietone at PreviousNext commentedCKEditor has been removed from core, CKEditor 4 is removed from Drupal Core in 10.0.0
Comment #68
carolpettirossi CreditAttribution: carolpettirossi at ImageX commentedI tested patch #56 and it worked for me on Drupal 9.4
Comment #69
Rajeshreeputrapatch #56 works with Drupal 9.4
Comment #70
ari.saves CreditAttribution: ari.saves commentedThe patch in #56 appears to have broken for 9.4.7; it still applies cleanly, but doesn't appear to work. I ended up using the one in #40 after upgrading.
Comment #71
SerkanB CreditAttribution: SerkanB commentedI'm using Drupal 9.4.8, CKEditor 4 (contrib) 1.0.1 and bunch of paragraphs and layout-builder stuff, where the modal-overlay-stuff comes from.
I've taken the patch in #64 and changed it a bit until it applied and worked correctly for me.
I didn't touch the tests though. For me the changes seem to work fine.
I based the patch on the branch 1.0.x, but it works just fine for the released 1.0.1.
Comment #72
mark_fullmerFor sites using this patch to fix this issue with the jQuery UI dialog, an alternative to consider is switching to the very similar contributed module, https://drupal.org/project/layout_builder_iframe_modal.
I verified that this issue is not present when using that module.
Comment #73
TerraCoders CreditAttribution: TerraCoders at CivicActions for Centers for Medicare and Medicaid Services commentedPatch in #71 looks to be working for Drupal 9.5.5 and CKE4 1.0.1 -- resolving this issue in Layout Builder when using CKE Anchor Link plugin.
Comment #74
alexortega_98 CreditAttribution: alexortega_98 at 1xINTERNET commented#71 works, tested in Drupal 9.5.8 and Drupal 10.0.8.
Comment #75
cbanman CreditAttribution: cbanman at Acro Commerce commented#71 works with Drupal 10.2.3