CKEditor 5 + Drupal 10 Compatibility

adiro created an issue. See original summary.

Changed filter type to use FilterInterface::TYPE_TRANSFORM_IRREVERSIBLE as per https://www.drupal.org/docs/8/api/filter-api/overview. Validated this type is compatible with CKEditor5.

The Shortcode HTML corrector filter is also affected by the same issue. This patch also changes that filter to use the TYPE_TRANSFORM_IRREVERSIBLE FilterInterface so it too can be enabled.

This updated patch fixes the functional tests for shortcode.

Since the filter transforms on render, we needed a more robust setup for the functional tests that includes having a content type (using the standard install profile), creating a custom filter format, creating nodes with the test content, and then displaying those and comparing the expected result to the actual result.

A few subtle additional changes:

• Drupal has dropped closing tags in self-closing void elements (e.g. img) since they aren't part of HTML5. In order to get the test for the img tag to pass the test needed to take that change into account. See #1388926: Remove all references to "self-closing" void elements in core
• Drupal has some oddities with how it sometimes adds classes to elements, and under some circumstances when there is a single class, a preceding space is added. The tests take that into account.
• Since we're comparing rendered content, special characters like ampersands render as multiple length text strings instead of single character strings leading to unpredictable results when running the random test. I've limited the character set generation to only alphanumeric characters now.

I also rolled the D10 deprecation fixes from #3289595: Automated Drupal 10 compatibility fixes into this patch.

Trying this once again with #3203585: Patch didn't apply, but it seemed like it was only the info file that it failed on in mind.

Edit: I believe these will continue to fail until the 2.0.x branch is available to test against.

The patches provided in the previous comments failed to apply.

The 2.0.x branch is the current one, so that is the branch this patch was created against which is why the patch does not apply. The 2.0.x branch is not available to test against right now. I've created another issue to make it available so that automated tests are useful. #3350953: Enable tests for 2.0.x branch

Removing the core: key from the info files again. Waiting to run tests until 2.0.x branch is available.

The previous patches were tested for the wrong branch (8.x-1.x instead of 2.0.x). Probably that is why the patches failed to apply.

Could someone else please review this so we can get a CKEditor 5 compatible version out the door? We've been using the patch above in production for a few weeks now.

Did you test it with ckeditor5, or just make it compatible with CKEditor 4 on Drupal 10? This doesn't include any changes for CKEditor 5 itself, everything seems to be for API compatibility with D10, along with some minor edits that should be reverted, e.g.:

@@ -55,7 +124,7 @@ class ShortcodeTest extends BrowserTestBase {
 $sets = [
 [
 'input' => '[button]Label[/button]',
- 'output' => '<a href="' . $this->siteUrl . '" class="button" title="Label"><span>Label</span></a>',
+ 'output' => '<a href="' . $this->siteUrl . '" class=" button" title="Label"><span>Label</span></a>',
 'message' => 'Button shortcode output matches.',
 ],
 [

and

@@ -84,22 +155,22 @@ class ShortcodeTest extends BrowserTestBase {
 $sets = [
 [
 'input' => '[clear]<div>Other elements</div>[/clear]',
- 'output' => '<div class="clearfix"><div>Other elements</div></div>',
+ 'output' => '<div class=" clearfix"><div>Other elements</div></div>',
 'message' => 'Clear shortcode output matches.',
 ],

etc

I recommend reviewing reverting these changes and starting over with CKEditor 5 on Drupal 9, so that you can avoid spending time on unnecessary changes.

@DamienMcKenna, if you revert those edits, the tests won't pass. When the HTML is generated, it creates spaces ahead of class names.

To clarify further why that is, look at the addClass() function in ShortcodeBase https://git.drupalcode.org/project/shortcode/-/blob/2.0.x/src/Plugin/Sho...

If a shortcode does not already have a class attribute, the first parameter will be an empty string. However that empty string becomes an empty first array value which gets imploded with spaces resulting in a prepended space in any instance where there was not a class attribute.

We aren't using shortcode with the WYSIWYG with any of the built-in submodules (tags) in our project. So my changes were intended primarily for the existing tests to pass and the module to be compatible with Drupal 10.

We have tested with this patch and Drupal 9.5.x + CKEditor 5. We are using our own custom Shortcode plugins that extend ShortcodeBase and they work with CKEditor 5 and this patch.

We don't use the shortcode_basic_tags submodule typically, but I just enabled it in a Drupal 9.5 site using CKEditor5 along with the patch above and [highlight] for example works as expected replacing the shortcode syntax with the span tag.

In order for this module to be compatible with CKEditor 5 it will need a new plugin; you can't say that it's compatible with CKEditor 5 based upon your site because your use case uses a custom plugin and not the code provided by these patches.

Again, the changes in the patches here amount to Drupal 10 compatibility fixes and a minor number of other changes, there's nothing specific to CKEditor 5 in the patches.

Damien, I tested the shortcode basic tags plugin with CKEditor 5. There are pieces to this that make the module compatible with CKEditor 5 - namely changing the filter type from TYPE_MARKUP_LANGUAGE to TYPE_TRANSFORM_IRREVERSIBLE

Have you tested it with CKEditor5 and it did not work?

Just to clarify, the 2.x version of this module doesn't have a CKEditor plugin per se; it has text filters. I don't believe the 2.x includes buttons even for CKEditor 4.

This is a patch to make the existing Shortcode text filters compatible with CKEditor 5, the module as a whole compatible with Drupal 10, and fix the existing shortcode tests so that they pass.

The existing Shortcode 2.x code does not include a CKEditor 4 plugin (e.g. a class that extends CKEditorPluginBase) so while there may be a desire to add that for CKEditor 5, it would be new functionality. Marking back to needs review.

If someone has tested this patch with CKEditor 5 and are getting errors, please post those errors so we can get them fixed! Thanks :)

My apologies, I mistakenly thought the module did have a CKEditor plugin.

Verifying that the patch in #11 still applies and doesn't seem to break anything. :-)

Using a path repo setup in a Drupal core dev environment, I was able to verify that I can install and enable the patched module with core 9.5, 10.0, and 10.1. Also was able to edit text and add shortcodes.

It would be super-nice if the dev branch were passing tests, and we could see changes to tests here.... I don't see another issue to fix those so let's do it here, unless someone wants to file it.

The test changes in the patch do (mostly) fix the tests so they'll run again.

I had passing tests for Drupal 9.5x and 10.0.x.

Using Drupal 10.1.x I only saw one fail:

$ ./vendor/bin/phpunit --config ./core/phpunit.xml --group shortcode
PHPUnit 9.6.8 by Sebastian Bergmann and contributors.
Testing 
........F 9 / 9 (100%)
Time: 02:31.670, Memory: 158.00 MB
There was 1 failure:
1) Drupal\Tests\shortcode\Functional\ShortcodeTest::testRandomShortcode
Random shortcode self-closed, output length is correct.
Failed asserting that 6 matches expected 8.
/var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/Equality/IsEqual.php:96
/var/www/html/shortcode/tests/src/Functional/ShortcodeTest.php:400
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

@jastraat mentioned that she couldn't reproduce the fail, so I ran it again and it passed.

It would be *very* much better if a maintainer could turn on automatic CI for this project.... :-)

Automatic testing was enabled, but it used PHP 7.3 with Drupal 9.5, which requires PHP 7.4. I corrected that; tests for the 2.0.x branch now use PHP 7.4 and Drupal 9.5.

With the currently committed code, one of the module tests fails. If that happens for this patch too, the culprit is not the patch.

I believe this code may correct the failing test in the committed code -

@Denes.Szabo pinging to see if you could look at / commit this to add compatibility for shortcode. This is our last module hold out for upgrading to Drupal 10.

Thank you for your contribution, the 2.0.2 just released.