Update CKEditor 5 to 39.0.1

Based on the test results at #3379104-7: Add a "CKEditor 5 nightly" GitLab CI job, I expect that

html-support: A link attached to an image should not be lost when loading content with the LinkImage plugin and full General HTML Support enabled. Closes #12831. (commit)

will cause some tests to fail — but likely this means a simplification on our end, since we won't need to work around it anymore 🤞

Hm, looks like this is causing a genuine regression:

    // When unrestricted, additional attributes on links should be retained.
    $xpath = new \DOMXPath($this->getEditorDataAsDom());
    $this->assertCount($unrestricted ? 1 : 0, $xpath->query('//a[@href="http://www.drupal.org/association" and @class="trusted"]'));

👆 when $unrestricted === TRUE, we expect that that class="trusted" is retained. That is no longer true once we update to 39.0.1. 🐛

When $unrestricted === TRUE, Drupal will have:

ckeditor5_arbitraryHtmlSupport:
  ckeditor5:
    plugins: [htmlSupport.GeneralHtmlSupport]
    config:
      htmlSupport:
        allow:
          -
            name:
              regexp:
                pattern: /.*/
            attributes: true
            classes: true
            styles: true
  drupal:
    label: Arbitrary HTML support
    elements: false
    library: core/ckeditor5.htmlSupport
    # @see \Drupal\ckeditor5\Plugin\CKEditor5PluginManagerInterface::getEnabledDefinitions()
    conditions: []

i.e. it will enable the GHS that allows EVERYTHING.

It looks like https://github.com/ckeditor/ckeditor5/commit/3d155169b2cad4b40330059382b... introduced a genuine regression against this 🤔 Or maybe we need to change our logic in core/modules/ckeditor5/js/ckeditor5_plugins/drupalImage/src/drupalimageediting.js

Either way, this is causing a net regression. 🙈

On our side, only @lauriii can figure this out efficiently. He’ll be back from vacation next week. Assigning to him. Welcome back, Lauri 😜

@lauriii, could you add some comments explaining why this change is needed? Why is https://github.com/ckeditor/ckeditor5/commit/3d155169b2cad4b40330059382b... not handling this for us? I hoped/expected that we'd be able to delete code rather than add more.

Especially because the conditionality in that upstream commit:

			if ( editor.plugins.has( 'LinkImage' ) ) {
				conversion.for( 'upcast' ).add( viewToModelLinkImageAttributeConverter( dataFilter, editor ) );
			}

… indicates that the necessary upcasting should be added only if LinkImage is loaded, and \Drupal\Tests\ckeditor5\FunctionalJavascript\ImageTest::setUp() does place the link toolbar item, which means that the CKEditor 5 LinkImage plugin should be loaded according to:

ckeditor5_linkImage:
  ckeditor5:
    plugins:
      - link.LinkImage
    config:
      # Append the "Link" button to the image balloon toolbar.
      image:
        toolbar:
          - '|'
          - linkImage
  drupal:
    label: Linked Image
    elements: false
    conditions:
      plugins:
        - ckeditor5_link
        - ckeditor5_image

And hence I thought we'd be able to delete the "image link GHS" integration code on our end:

    // Modified alternative implementation of GHS' addBlockImageLinkAttributeConversion().
    // This is happening here as well to avoid a race condition with the link
    // element not yet existing.
    …

which we had introduced in #3246168: Images are not linkable through UI; already linked images are unlinked (data loss!) (before it was added to core!).

I think just adding #9 as a comment to the modified file would indeed be sufficient 😊👍

No more concerns — thanks! 😊

🚢

Lovely, thank you! 😊 🚀

@Eduardo Morales Alberti No, not possible. Other contrib modules will break. If you are using no CKEditor 5 contrib modules, then you could.

But it'll have to be you doing it for your site, an official release Drupal 9.5 release with this would break countless sites.

Wish there was more I could do — but our hands are tied! Good luck 😊 Personally, I'd look into getting those contrib modules on D10!