CKEditor 5 + Drupal 10 Compatibility

Created on 16 May 2022, over 2 years ago
Updated 22 July 2023, over 1 year ago

Problem/Motivation

Have enabled 'Shortcodes' and 'Shortcodes - html corrector' filters and CKEditor 5 produces error 'CKEditor 5 only works with HTML-based text formats. The "Shortcodes" (shortcode) filter implies this text format is not HTML anymore.'

Steps to reproduce

Enable Shortcode 2.0 and CKEditor 5 on Drupal 9. Set Text Format to use CKEditor 5 and try enabling 'Shortcodes' or 'Shortcodes - html corrector' filters.

Proposed resolution

Make Shortcode 2.0 and CKEditor 5 compatible in Drupal 9.

Remaining tasks

User interface changes

API changes

Data model changes

🐛 Bug report
Status

Fixed

Version

2.0

Component

Miscellaneous

Created by

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

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

  • 🇺🇸United States jastraat

    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 📌 Remove all references to "self-closing" void elements in core Needs work
    • 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 📌 Automated Drupal 10 compatibility fixes Fixed into this patch.

  • Status changed to Needs work over 1 year ago
  • 🇮🇹Italy apaderno Brescia, 🇮🇹
  • Status changed to Needs review over 1 year ago
  • 🇺🇸United States jastraat

    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.

  • 🇺🇸United States jastraat

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

  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    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.

  • 🇺🇸United States jastraat

    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.

  • Status changed to Needs work over 1 year ago
  • 🇺🇸United States DamienMcKenna NH, USA

    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.

  • 🇺🇸United States jastraat

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

  • 🇺🇸United States jastraat

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

  • Status changed to Needs review over 1 year ago
  • Status changed to Needs work over 1 year ago
  • 🇺🇸United States DamienMcKenna NH, USA

    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.

  • 🇺🇸United States jastraat

    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?

  • Status changed to Needs review over 1 year ago
  • 🇺🇸United States jastraat

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

  • 🇺🇸United States DamienMcKenna NH, USA

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

  • Open in Jenkins → Open on Drupal.org →
    Core: 10.0.7 + Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    Composer require failure
  • Status changed to Needs work over 1 year ago
  • 🇺🇸United States mile23 Seattle, WA

    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
    
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    Composer require failure
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.0.7 + Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    Composer require failure
  • Status changed to RTBC over 1 year ago
  • 🇺🇸United States mile23 Seattle, WA

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

  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    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.

  • Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update over 1 year ago
    Waiting for branch to pass
  • 🇮🇹Italy apaderno Brescia, 🇮🇹

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

  • 🇺🇸United States jastraat

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

  • 🇺🇸United States jastraat

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

  • Status changed to Fixed over 1 year ago
  • 🇭🇺Hungary denes.szabo Hungary

    Thank you for your contribution, the 2.0.2 just released.

  • Status changed to Fixed over 1 year ago
Production build 0.71.5 2024