ckeditor5.dialog.fix.js throws "Uncaught TypeError: event.target.classList is undefined" in Firefox in Drupal 10 with the editor in a modal

Found a similar ticket here: https://www.drupal.org/project/admin_toolbar/issues/3349992 🐛 Uncaught TypeError: event.target.classList is undefined in Drupal 10 node edit forms Closed: works as designed

Created a patch for the issue mentioned, please review.
Thank You.

Fixed CCF, please review.

Fixed CCF, please review.
Attaching Interdiff.

andrew.farquharson → made their first commit to this issue’s fork.

I have tested patch #7. I forked the issue and created a new branch to apply the patch to and test it on. I installed the add_content_modal contrib module, used devel_generate to create some articles, configured the add_content_modal so i can edit the article content type in a modal.

See screenshot for the error message in the console and the ckeditor field loaded in a modal window. The error is 'Uncaught TypeError: event.data is undefined'.

The issue in #2 was created by me, so I can confirm I was running into this issue twice now.

Any ideas how it can come to this situation?

Hi @Anybody I think that if this is fixed the fix should work whether or not the admin_menu is enabled.

MR !3951 works for me repeating the test I describe at #11. No error messages in browser console when I switch focus between the ckeditor fields and another browser window.

@Anybody perhaps you could test MR !3951 with the admin menu enabled..

Should only be marked fixed by a committer when the fix is merged.

But this was previously tagged for tests which still need to happen.

@smustgrave Well, that may be correct. I understood that if i mark fixed it gets merged if there is no intervention for 2 weeks. This is not really a major issue. It generates an error in console but does not affect any functionality. CKeditor5 is 'experimental' in any case, is it not? Anyway let's hope others will test it.

In which browser is this happening?

For me it was Firefox

@Wim Leers, I reproduced the error in Firefox 112.0.2. Also I tested my MR !3951 in the same and the error disappears. I switched focus between all of the fields in the node (inside a modal with ckeditor5 being used on the body field) and no errors triggered. The code change required is JQuery. I studied the JQuery documentation regarding the methods involved but the fix I got by googling how to fix 'Uncaught TypError undefined' error.

Firefox twice.

Can you also try in another browser? That'd help to rule out a browser-specific bug as the cause.

Here is the error triggered in Safari Version 16.2
Error triggered in Safari →

#22 Merge request !3951 tested in Chrome

Merge request !3951 tested in Safari 16.2

Merge request !3951 tested in Firefox 113.0

If you can use DDEV-LOCAL, here are the command line steps if once you have cloned the repository, forked the issue and checked out the branch:

ddev config --project-type=drupal10
ddev start
composer install
composer require drush/drush
ddev drush site:install --account-name=admin --account-pass=admin -y
ddev launch
ddev composer require 'drupal/add_content_modal:^1.0'
ddev drush en add_content_modal
ddev composer require 'drupal/devel:^5.0'
ddev drush generate-content 20
ddev drush cr

You will have to go to disable aggregation of css and js, clear all caches
Configure the add_content_modal: enable it for at least one content type

Did not see tests in the MR

@smustgrave

This is one example core test that seems to cover everything I could possibly create a test for:

core/modules/ckeditor5/tests/src/FunctionalJavascript/CKEditor5Test.php

It not only creates a text field for a node and activates ckeditor5 for that field, it inserts text into the text field using every possible button in the toolbar of the ckeditor and saves the result..

And it is one of 20 'FunctionalJavascript' tests. I am assuming that all these tests are run when anyone runs automated testing? Are you expecting in addition to all such tests me to write a custom one? If so, what do i need to test that is not being tested already? Or are these core tests not triggered when choosing 'Add test / re-test'?

If they are triggered then for overall functionality of the ckeditor5 module then these tests seem comprehensive. The error I am trying to fix though triggering an error does not seem to break any functionality so by in addition, manually viewing the browser's console, it seems to cover the bases?

Probably would leave up to submaintainer and committer if they would merge without test coverage but it is possible to test for console errors. There may be a FunctionalJavascript function for this or it could be nightwatch. Would have to double check that.

@smustgrave Yes I need to re-read the original description of this issue. I can install the add_content_modal:^1.0 module, make it a test dependency. I will also have a look at nightwatch- see if it can supply any testing needs.

Also posted in the slack channel for wimleers to take a look.

@smustgrave, thank you for posting in Slack. Although i have been testing with a contributed module to make ckeditor5 to load in a modal, it may not be best practice in core tests to use somewhat random contrib modules as test dependencies?. I am thinking to test this modal functionality in core, it requires a more 'native' way of loading a ckeditor5 field(s) in a modal? So I am wondering if there is some Nightwatch technique that can load a specific form in a modal?

Could you also try to reproduce this error in Chrome please? 🙏

Drupal's automated JavaScript tests use Chrome. If we can reproduce it in Chrome, an automated test is possible. Otherwise, it's impossible.

Great work here so far BTW! 👏😊

@Wim Leers The error does not appear in Chrome. I see that the nightwatch.conf.js file in core only sets up a chrome environment. But not currently Firefox or Safari. To test the error using Nightwatch I would have to add an environment for these other browsers.

@smustgrave Is that something that is permissible? Or is the core nightwatch config restricted to Chrome only? If Chrome only then it seems as @Wim Leers says creatoin of an automated test for this issue is not possible.

Alright, it's not possible to test then 👍

The fix seems reasonable.

Very much looking forward to the fix in 10.1.x. Will this be part of the next minor release (possibly 10.1.1?) Current broken state of WYSIWYG editor functionality in 10.1.0 is a real issue for editors.

As the MR patch doesn't apply against 10.1.0 I guess this will needs rerolls against 10.1.x and 11.x?
Should we do that in separate MR's or how to proceed here best @Wim Leers?

Or will this be first committed against 10.0.x now?

The MR diff applies against 11.x for me, however I think this needs an inline comment.

Hi @catch I have added comments.

 // Fixes a console error message generated in Firefox only
 // @see https://www.drupal.org/project/drupal/issues/3351600

I can edit them if they are not clear enough.

I think we need the MR to be against 11.x, but I can't change that. Comment was added. @catch for help? ;)

Uploading the static patch from MR!3951, if anyone needs it for composer and confirming it still applies and works with Drupal 10.1.1 and Drupal 11.

So re-confirming RTBC from #39 once this is rebased / retargeted.

Running into the same issue with 9.5

#45 still works in Drupal 10.1.4.
I will not change the Status, as I don't know if it has been rebased / retargeted.

New merge request against 11.x, back to RTBC :)

I don't think this needs a frontend framework manager review. This is a straight forward bug fix targeting a specific browser that we don't have as part of our testing infrastructure.

Committed 8b0a91f and pushed to 11.x. Also cherry-picked to 10.2.x and 10.1.x. Thanks!

lauriii → closed merge request !4977

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

It seems that the patch #46 does not writes to the proper place. This one should work well.

The above patch did not apply to 9.5.11 installation.

Attached a new version without code changes