Drupal 10 & CKEditor 5 readiness

Also wondering about this

Media entities

Are you using https://www.drupal.org/project/entity_embed to embed media entities? If so, you can make the switch to Drupal core's media_embed text format filter (all media entities embedded with Entity Embed will continue to work just fine!).

You can then also enable the Media Library functionality in CKEditor for a far better UX!

You can do that today!

Other entities

If you're using it for other types of entities, then you'll indeed need to wait for the CKEditor 5 integration to be developed.

Getting started on CKEditor 5 support

I think significant parts of core/modules/ckeditor5/js/ckeditor5_plugins/drupalMedia can be reused. Especially the drupalelementstyle part there — that was specifically designed for future reuse. Even if it's not (yet) a public API, it can absolutely be copy/pasted to reuse today.

Looks like the original maintainer, Dave Reid, has been very active in the past few weeks! 🥳

@Dave Reid, would you be interested in taking the lead on this, with support from those of us who've been working on CKEditor 5 in core?

Thanks Wim! Yes, we are only using it to embed media entities thankfully, so we are working on switching to media embed.

so we are working on switching to media embed.

Great!

That should be as simple as:

1. Enable media_embed filter on relevant text formats
2. Add Insert from Media Library button to those same text formats' CKEditor 4 toolbar
3. Remove entity_embed filter — check and confirm that previously embedded media entities still render correctly
4. Remove the Entity Embed button from the CKEditor 4 toolbars

… but it won't be that simple if you're using advanced Entity Embed functionality.

Step 3a: Update all our existing embed codes to the new format used by media embed :) - but not a huge deal. Someone wrote a module to do just that which we can start from.

Ah, crap, you're right, all the <drupal-entity> tags need to be changed to <drupal-media>! Sorry! 🙈

@Wim Leers

We are looking to embed nodes in Ckeditor5. Is there any workaround for it?

Thanks in advance
Venkat

My team makes heavy use of entity-embed to pull in entities as cards. Where is the best place to track progress and possibly help? Is there a specific ticket, etc? Thanks!

We use media embed for media items and we are glad to hear that will work.

Thank you for all your work! Cheers.

This is the issue to track the progress - of which there is none at the moment. If your team has the resources to help get started that would be great! Sounds like a lot of the progress to get Media Embed working in CKE5 would translate to this module.

I started working on the support for CKEditor5. Currently I'm at a stage where the preview is not implemented yet, so you won't see for example the Full content or the Node ID in the CKEditor5 itself when embedding content, however you will see the actual element in the source, and the embedding works upon saving. The preview needs work and will happen soon.

Another issue I'm facing currently is entity embed used to create individual buttons in CKEditor4 based on how many Embed Buttons are defined in /admin/config/content/embed. This was easy in CKEditor4, but it is not so easy anymore as CKEditor5 uses YamlDiscovery, and whatever you are declaring under the drupal.toolbar_items key in the entity_embed.ckeditor5.yml file, if there is a corresponding plugin exported in the plugin's index.js, the toolbar item will show. Currently the EmbedButtons are loaded and iterated through in \Drupal\entity_embed\Plugin\CKEditor5Plugin\DrupalEntity, then passed to the js/ckeditor5_plugins/drupalentity/src/ui.js file to generate these buttons. (I'm not sure if this is the correct way, but haven't found anything else, as it's a fairly new thing, and no other modules are declaring dynamic toolbar items.) The buttons are generated correctly based on the embed buttons, however, they are not showing unless you manually add them to the drupal.toolbar_items key in the entity_embed.ckeditor5.yml file. (You can see at the end of the attached video, the newly created Embed Button called 'This wont show' does not show up in the toolbar dynamically.)

One solution I could think of is using the hook_ckeditor5_plugin_info_alter() to alter the plugin definition dynamically and set the toolbar_items from there, but this would mean we would need to invalidate the plugin cache whenever a new embed button, or an existing one is saved in order for the plugin discovery to add the buttons correctly and immediately to the toolbar. Also, the toolbar items have their own admin_library which we could use to dynamically set up these buttons, also their CSS which includes the icons of them, however I haven't seen any implementations yet. We could be the first ones.

Also, embed module currently only supports CKEditor4, so this won't work without changing the access check (embed/src/Access/EmbedButtonEditorAccessCheck.php's checkButtonEditorAccess() method) to allow CKEditor5 as well. (If this is not changed, when clicking the toolbar button in CKEditor, nothing would happen and console log would be found with an issue that Ajax can't load that page (because of the HttpException that is thrown there.)
Related issue: https://www.drupal.org/project/embed/issues/3309747

I'm attaching a video of what I have currently, so you don't need to download it and change embed's access check in order to test it.

If the code is a mess, I'm sincerely sorry for it, and I'm happy to take any advice / guidance as CKEditor5 is fairly new for myself as well.

Adding D10 compatibility as a related issue.

Excellent write-up here 👏 — thank you for providing so much detail, that makes this much easier! 😊

I first wanted to provide direction before reading all of #13, i.e. just based on the video, the merge request and knowing that preview is not yet implemented. I wanted to provide an unbiased analysis. Here goes:

1. For preview: see how this is done for embedded media core/modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/drupalmediaediting.js in Drupal core (which uses <drupal-media>).
2. For multiple buttons: CKEditor 4's plugin architecture had \Drupal\ckeditor\CKEditorPluginButtonsInterface::getButtons() (i.e. there's a method), whereas in CKEditor 5's plugin architecture, as much as possible has been made declarative. This means that you'll need to implement hook_ckeditor5_plugin_info_alter() to generate the appropriate annotation metadata (i.e. generate the desired toolbar_items).
3. (Following up on the previous point.) To make sure that your alter implementation gets invoked so you can add the needed toolbar items, the plugin discovery should re-run whenever there is a reason to. That means that changes (creations/updates) to \Drupal\embed\Entity\EmbedButtons should invalidate the cache by calling \Drupal\ckeditor5\Plugin\CKEditor5PluginManager::clearCachedDefinitions(). This is actually something that should have been happening for the CKEditor 4 integration too. Plus, on top of that, there should be logic in the (Entity) Embed module to disallow deleting EmbedButton entities that are actually being used by CKEditor (4 or 5).
4. An alternative to the previous two suggestions is to instead of generating a single CKEditor 5 plugin (entity_embed_drupalentity) with multiple toolbar items, you generate one CKEditor 5 plugin per EmbedButton. So that'd be entity_embed_node, entity_embed_file, et cetera. I think that'd actually be cleaner. Admittedly, it's somewhat confusing that the "alter plugins" hook allows you to add plugins — but that's a precedent set by the Drupal plugin system 🤷 (You have to write custom logic to disallow this, i.e. to only allow actual altering, and not adding nor removing. The only plugin manager that does that to my knowledge is the TypedConfig one — see \Drupal\Core\Config\TypedConfigManager::alterDefinitions(). I think there are some pretty clear benefits to this: one explicit CKEditor 5 plugin per EmbedButton, which means each button can have their own CKEditor 5 plugin settings (i.e. a vertical tab with settings).
5. More fundamentally: for now, I think going with the Drupal-server-generated-dialog is appropriate: it's the path of least friction. But … it also results in the same less-than-stellar UX that we've had for CKEditor 4. For CKEditor 5, the Media Embed functionality has been completely rewritten: you only see a Drupal-server-generated dialog for selecting Media (the Media Library), but once that's done, you can manipulate the way it looks using nothing but a client-side UI! We implemented the Media Embed alignment & view mode selection UX in #3260554: [drupalMedia] Support alignment on <drupal-media> and #3245720: [drupalMedia] Support choosing a view mode for <drupal-media> respectively. The result is something like this:
   
   It took a lot of effort to make this work well. But … we explicitly thought of use cases like Entity Embed when building this, so we specifically built this as a reusable DrupalElementStyle plugin (note the absence of the keyword "media"!) You can and should use this for the "Display as" functionality. Thanks to a bug by @mglaman I know that this is already being used successfully by other contrib modules too! I doubt that specific bug affects Entity Embed's use case: #3300568: DrupalElementStyleUi renders button even if there are no items.

… clearly you were thinking along the same lines for supporting dynamic toolbar_items! I don't see why you'd need multiple of admin_library — just that single one can contain multiple JS and CSS files, which means you can still easily generate as many as you like. But, if you generate one CKEditor 5 plugin per EmbedButton, then this definitely won't be a problem! 🤓

Can't wait to see where you take this! 🤩

P.S.: a known bug you could run into in the current implementation is #3303191: Drupal.ckeditor5.openDialog missing existingValues param.

For now based on the talk with @wimleers, we decided to stick with the current functionality regards to the dialog, and once we are in a state we are confident everything is working we could convert it so the view modes are working the same as in media.

The latest commit adds the initial preview functionality, which misses what happens when the preview is still loading, it's in progress.

Also, the latest commit misses the icons on the admin page toolbar, still under work.

I also attach an updated video of the current progress regarding the preview work.

Wow, massive progress today! Looking forward to review tomorrow 😊

Currently it seems like CKEditor5 only supports SVGs for the icons meaning we should probably open an issue in Embed to reflect this, and have Embed only allow SVGs for the Embed Button icons, or just have the default image load if the image uploaded for the button is not in an SVG format. (And probably alter the form of Embed to add an info message, if CKEditor5 module is enabled that it will only accept SVGs, and the default image will be loaded if you choose to upload an image other than SVG). Any other ideas for the icons are welcome.

I made some changes, which involves dynamically setting the icon on the text format editor page (admin page of the CKEditor5 instance), but since CKEditor5 only supports SVGs as said above, the actual instances (for example a Node creation page) will only have the icons from the Embed Button configuration, if they are uploaded as an SVG. Until the other issue is resolved in Embed (Only allow SVGs for button icons), the Entity Embed module's default icon will be used.

Example:
A user creates a new Embed Button configuration to upload files.
1. They add a custom icon with an extension of .png
2. The default E icon will be shown on the text format where it is configured.

Example 2:
A user creates a new Embed Button configuration to upload files.
1. They add a custom icon with an extension of .svg
2. The configured icon will be shown on the text format.

Possibly we could edit the entity_embed.set_dynamic_icons.js to reflect that, and use the default image on the admin configuration as well, if it's not an SVG.

1. 👍 I've confirmed with the CKEditor team that CKEditor 5 only allows SVG icons.
2. 🐛 In my testing, no preview showed up. This turned out to be because I did not yet enable the entity_embed filter!

   ✅ I pushed commit 3c1ad59 to add the necessary plugin annotation to declare a dependency on that filter. Now you get a The Node toolbar item requires the Display embedded entities filter to be enabled. validation error if you try to do that!

3. 🐛 With that out the way, I wanted to try seeing a preview in CKEditor 5. This too failed. After debugging for a few minutes, I discovered why: only <drupal-entity data-entity-type="node" data-entity-uuid> are allowed, but \Drupal\entity_embed\Plugin\Filter\EntityEmbedFilter::process() requires either data-view-mode or data-entity-embed-display to be set, otherwise it doesn't do anything. Those attributes were set in CKEditor 5 sure enough but … they were being stripped by filter_html.

   So we need the elements part of the annotation to be fixed. I'll lave that to you.

4. 🤓 I think we should look into removing entity_embed_ckeditor5_plugin_info_alter() and using a plugin deriver instead, that would make the code simpler to read. But I haven't used a plugin deriver yet for CKEditor 5 plugins, so this might take some poking around 🤓

This also really should have some test coverage.

I think you can reuse a lot from Drupal core's MediaTest. For example:

• \Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testPreviewUsesDefaultThemeAndIsClientCacheable()
• \Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testEmbedPreviewAccess()
• \Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testAlignment()
• \Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testEditableCaption()

That should save you a lot of time :)

Of course, starting with just porting entity_embed's \Drupal\Tests\entity_embed\FunctionalJavascript\CKEditorIntegrationTest::testIntegration() to test the CKEditor 5 equivalent would be a great start! 😊

This is shaping up really nicely by the way! 👏

I've been working on allowing derivers to be used for CKEditor 5 plugin definitions in #3313473: CKEditor 5 plugin definitions should be derivable, but been having somewhat of a hard time with getting the tests to work — to be continued tomorrow!

#3313473: CKEditor 5 plugin definitions should be derivable is now ready!

#3106808: Array to string conversion notice when embedding a Media file introduced a small conflict that made this MR not mergeable. Did git merge 8.x-1.x and fixed the conflict.

Sending this for another round of review while working on the editing part of the UI raised by @laurii to see if there are any other things left out / needs change.

This looks great now! 🤩 Reviewed everything in detail, except for the CKE5 JS plugin — leaving that to @lauriii to review 😊

#3313473: CKEditor 5 plugin definitions should be derivable is in (in 9.4, 9.5, 10.0 and 10.1), so this is no longer blocked on Drupal core!

nevermind - but thank you :D

FYI tried testing this. I see the Node button appear but it doesn't trigger.

Get this error in the console

"
An AJAX HTTP error occurred.
HTTP Result Code: 500
Debugging information follows.
Path: /entity-embed/dialog/basic_html/node
StatusText: error
ResponseText: The website encountered an unexpected error. Please try again later.Symfony\Component\HttpKernel\Exception\HttpException: Currently, only CKEditor is supported. in Drupal\embed\Access\EmbedButtonEditorAccessCheck->checkButtonEditorAccess() (line 82 of modules/contrib/embed/src/Access/EmbedButtonEditorAccessCheck.php). Drupal\embed\Access\EmbedButtonEditorAccessCheck->access(Object, Object)
call_user_func_array(Array, Array) (Line: 160)
Drupal\Core\Access\AccessManager->performCheck('access_check.embed_button_editor', Object) (Line: 136)
Drupal\Core\Access\AccessManager->check(Object, Object, Object, 1) (Line: 113)
Drupal\Core\Access\AccessManager->checkRequest(Object, Object, 1) (Line: 110)
Drupal\Core\Routing\AccessAwareRouter->checkAccess(Object) (Line: 95)
Drupal\Core\Routing\AccessAwareRouter->matchRequest(Object) (Line: 112)
Symfony\Component\HttpKernel\EventListener\RouterListener->onKernelRequest(Object, 'kernel.request', Object)
call_user_func(Array, Object, 'kernel.request', Object) (Line: 142)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object, 'kernel.request') (Line: 135)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 81)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 707)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
"

In what version for Drupal did you test this?

With D10 beta I needed these pataches:

        "patches": {
            "drupal/core": {
                "#3313473 CKE5 derivable": "https://www.drupal.org/files/issues/2022-10-31/3313473-43.patch"
            },
            "drupal/entity_embed": {
              "#3287235-28: D10": "https://www.drupal.org/files/issues/2022-10-25/3287235-28.patch"
            },
            "drupal/embed": {
                "#3309747 CKE5": "https://git.drupalcode.org/project/embed/-/merge_requests/2.diff"
            }
        },

9.5-dev which has the core patch included

I guess this patch is yet only D10 ready, and still misses the D9 compatibility :( not true - it is!

#33 – #36: You need to apply #3309747: Add CKEditor 5 compatibility and keep supporting CKEditor 4 to the embed module. 😬

Ah once I applied that this does appear to be working then. +1 RTBC.

Not moving as I see the needs tests tag.

I had to spend some time searching why my Entity Embed icons wouldn't appear on the CKE5 toolbar until I found this line on entity_embed.set_dynamic_icons.js that checks if the icon is a SVG and fallbacks to the Entity Embed default icon if not.

   let iconUrl = button.icon.endsWith('svg') ? button.icon : '/' + drupalSettings.modulePath + '/js/ckeditor5_plugins/drupalentity/entity.svg';

It's a very minor thing, but it's confusing for the end user / site builder because no indication about this requirement is to be found anywhere.

For better UX, I would suggest documenting this requirement on the icon upload form, something like this:

I've just realized that an issue to force SVG exists on drupal/embed and re: my last comment, that would solve everything.

I use entity browser (https://www.drupal.org/project/entity_browser) with entity_embed module with different views/views displays for embedding media. In my Drupal site (currently still running on CKEditor 4), I have different content (editor) formats depending on content type and user role. Also I have different entity browser views which are filtered and restricted by role depending on use case. The core media library is a great improvement but it has it's limits related to user permissions. One media view is IMO to less aside for a one-person-site.

In my site config we have "normal" users , "business" users, "sales team" users, "admin users" with different permissions for media management and each has an extra entity browser view (entity browser view "display").

#41: did you realize that Media Library's grid-based UI is … really just a view? 🤓 See core/modules/media_library/config/install/views.view.media_library.yml. It can be customized just like any view! And that was in fact the very intention (if I remember correctly).

Yes, media library is *one* view. Entity browser is an extensible system that allows you to create different browsers for different use cases, exactly as #41 describes. Media Library is by far not as flexible.

#43: 👍 — sounds like all the extra functionality is indeed actively used by @dercheffe then!

Besides #3309747: Add CKEditor 5 compatibility and keep supporting CKEditor 4 being a hard blocker, per #24, the only missing thing here is test coverage:

I think you can reuse a lot from Drupal core's MediaTest. For example:

• \Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testPreviewUsesDefaultThemeAndIsClientCacheable()
• \Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testEmbedPreviewAccess()
• \Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testAlignment()
• \Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testEditableCaption()

That should save you a lot of time :)

Of course, starting with just porting entity_embed's \Drupal\Tests\entity_embed\FunctionalJavascript\CKEditorIntegrationTest::testIntegration() to test the CKEditor 5 equivalent would be a great start! 😊

Any chance you have the bandwidth to push that forward, @balintpekker? 🤞

I still want to mention besides the tests the editing part is missing per https://git.drupalcode.org/project/entity_embed/-/merge_requests/12#note...

I'm not sure how much of a work is needed to implement the editing part, but the testing shouldn't be too much of a problem. If someone has the time to implement the editing part, that would be nice, I'm not sure how much time I'll have in the near future. It would be really nice to have this in as soon as possible considering D10 is out in a bit.

Tested this today and it was pretty good. Couple of observations.

1) The button image doesn't work. All buttons have E even if the button has an image associated with it.
2) The migration didn't work? I don't know if this is a bug or just awkward but migrating shows "The CKEditor 4 button image_embed does not have a known upgrade path. If it allowed editing markup, then you can do so now through the Source Editing functionality." however the buttons are in the toolbar.

@neclimdul
1) The buttons are having an E as a fallback if you are not uploading the embed button in an SVG format. CKEditor5 only supports SVG, and there is an open issue about it in embed. This will likely change, but I hadn't got time to work on it lately. All contributions are welcome.
2) This was not done by the usual process of migrating from CKE4 to CKE5 as the docs say (sadly), and the editing part is not working at the moment, again, lack of time to work on it. If you think that would fix the editing part that would be awesome and we could move on to close this.

Thanks! That makes sense. Updating IS summarizing the comments in #47

Hi all.

How can I help test this?

Is the https://git.drupalcode.org/project/entity_embed/-/merge_requests/12/diff... commit stable against 8.x.1.x

(I'm at a major decision point if I see if i can use CKEDITOR5 on a project, and being able to select entities and place them in CKEDITOR5 is my main blocker)

Thank you.

I was able to apply the patches need to the EMBED module and this module to get it to work in Drupal 9.5.1 with CKEDITOR 5.

I know the title is "Drupal 10 & CKEditor 5 readiness" ... but I figured 9.5.x would act the same ...

... when I selected a node to embed in CKEDITOR It does not render anything in the RTE besides the blue border with the arrows to make breaks

This is a node content type with 3 custom plain text fields.

I was expecting to see the node.html.twig file rendered in this space so I could apply styles to it that would accurately display it in the RTE.

When I hit save, the entity doesn't render on the front end either.

<p><drupal-entity data-entity-type="node" data-entity-uuid="bbdb531d-515d-4110-8ffc-6bfc92a17da5">&nbsp;</drupal-entity></p>

Is all that comes out.

FYI I'm seeing a number of people come by and "made their first commit to this issue’s fork." but didn't commit anything. Credit should be adjusted.

Wrong issue sorry!

@alphex, are you sure you have the checkbox checked to "Display embedded entities" on your text format (Full HTML for example) you enabled the entity embed button on?

hey @alphex,
please check a couple of things below,
1. have you put `Embed media`filter directly after `Align images` and `Caption images` ?
2. have you enabled `Display embedded entities` filter and put it after `Embed media` filter
3. uncheck `Limit allowed HTML tags and correct faulty HTML` if you are not sure which HTML tags need to allow.

Also experiencing the same issue with @alphex, this is because of missing allowed attributes in the <drupal-entity> element.

Pushed fix to MR

I've added a toolbar balloon for edit functionality that brings up a modal dialog. I duplicate core's openDialog function as now it's missing existingValues param as in #3303191: Drupal.ckeditor5.openDialog missing existingValues param

I personally think we can use something like Image contextual toolbar for display, alt, align, and caption.

Thank you for moving this forward. I, too, need to embed nodes and upgrade to ckeditor5. The patch does not apply to version 8.x-1.3. Is it possible for someone to work on this? Thank you for all your work!

Thanks for the patch!

But I am getting

Uncaught EvalError: Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script in the following Content Security Policy directive: "script-src 'self' 'unsafe-inline' svc.webspellchecker.net cdn.jsdelivr.net".

    at ./js/ckeditor5_plugins/drupalentity/src/index.js (<anonymous>:1:14050)

error, and the CKEDITOR5 can't be rendered.
The fix for me is `cd` to the `entity_embed` module , run `npm run build` to regenerate the index.js

Attaching the latest code from MR in patch format for testing and usage in one of my projects.

While testing the patch i observed following error on basic html format of my project:
CKEditorError: plugincollection-soft-required {"missingPlugin":"WidgetToolbarRepository","requiredBy":"EntityEmbedToolbar"}

On some debugging i observed that this error gets resolved if either table or image widget is enabled.

Possibly adding below code inside entity_embed/js/ckeditor5_plugins/drupalentity/src/toolbar.js should resolve the issue:

import { WidgetToolbarRepository } from 'ckeditor5/src/widget';

The current MR/patch doesn't apply properly to me due to conflicts between the .info.yml changes and autogenerated changes from Drupal's packages. Here's a patch that can be applied via composer. The changes should not be brought into the MR.

I also notice that alignment is not displayed correctly in the editor. Alignment is applied on the image, but not the widget, so the widget will still taking full width in the editor.

Just tested #3322523: Migrate to CKEditor 5 for Drupal 9.4+ / Drupal 10 for paragraphs embed with the MR from #3309747: Add CKEditor 5 compatibility and keep supporting CKEditor 4 and MR 12 from here and thinks appeared to be working for me.

Yeah its working for us too. Moving to NR but this might be RTBC pending the Embed module fix.

Confirmed, this is working for me. I used the issue fork from Embed Module with this issue's fork.

This fork is working for me as long as it's paired with the Embed Module fork listed in this issue here (Just like in #68). It successfully solved an issue with a custom embed for the ckeditor 5 interface we were having. Wonderful work here folks.

Also confirming after initial testing the MR here is working well.

A remaining task is listed as adding a CKEditor4To5Upgrade Plugin, but I'm thinking there'll be a bit to that as each site will have a customised list of cke4_buttons that need converting? The plugin would need to load each embed.button and add them as cke4_buttons to be converted.

We could theoretically apply the patch on the embed module via the composer file in the module here :-) but not sure if we want to go that route. Depending on how fast the for that is merged we could pin the min version requirements to the version that contains the fix for embed.

I'd missed the fact that embedded entities cannot be linked. This was possible in CKE4 thanks to #2511404: Image entities/fields embedded using Entity Embed cannot be linked in CKEditor which we'd used to allow linkit links for embedded entities.

Unfortunately this is block upstream in CKEditor 5 itself. This is being tracked. #3349893: [upstream] [GHS] CKEditor 5 removes <a>s that wrap HTML elements not natively supported by CKEditor 5 and is listed as issue 4. in the tracking issue #3340578: [meta] [upstream] Prioritized CKEditor 5 upstream blockers.

Linking embed never worked for me in ckeditor 4 actually. #3131055: Wrapping drupal-media image in a link produces empty P tags after image #3075527: _filter_autop() breaks links around the <drupal-media> tag #2876094: Linking an image creates an empty paragraph tag in CKEditor so I don't think that's a blocker for this fix.

Also you might be able to work around it by wrapping it in a div? I'm pretty sure we tested linking embed content and it worked but our content might have different wrappers.

This is the patch I used, in case anyone else wants to try out this issue but can't get the MR patch to apply

The merge request only applies to the dev release I noticed when I was testing this out.

I did notice broken CSS today when using entity embed with Acquia Site Studio (cohesion) on the embed dialog where you select the embed type. Still need to check what's happening there and where the issue lays exactly.

I think the icons may need to be converted to SVG too.

@fenstrat I've started looking into adding a CKEditor4To5Upgrade Plugin, and run into some issues. I was wondering if anyone else was looking into it, or if there was anyone more experienced with plugins than I am, who can answer some questions for me:

• Is there a way to add values to annotation dynamically? I don't know of one, but the upgrade plugin needs an array of all the names of CKE4 buttons it deals with. Since users create buttons, it can't be hard coded.
• Are any configuration changes needed? I didn't think so from a quick look at the merge request. If there are no config changes between the CKE4 and 5 buttons, things easy on that front

Are any configuration changes needed? I didn't think so from a quick look at the merge request. If there are no config changes between the CKE4 and 5 buttons, things easy on that front

The buttons are handled by the Embed module. I started to write test coverage for #3309747: Add CKEditor 5 compatibility and keep supporting CKEditor 4 and it looks like the button configuration itself is completely unaware of the editor.

I have added the CKEditor4To5Upgrade Plugin.

The annotation issue was overcome by using the hook hook_ckeditor4to5upgrade_plugin_info_alter, which allows you to change the array of available CKE 4 to 5 plugins after it has checked the annotations.

This version of the plugin assumes no configuration changes are needed and that the button names don't need to change.

To test this, select a text editor format and switch it from using CKE4 to 5. Make sure there are no warnings displayed on the page or errors in the log. You should not need to save the changes to the format to see these.

I have included it as a patch too.

Subscribing

@pbone3b FYI: No need to create noise in the issue queue by placing a comment. You can simply click on the star/follow on the right under the issue meta info on the top of the issue. 😇

I have a ckeditor5 button created using the Embed module ("Embed type" = Views with "Filter which Views to be allowed as options" checked).
When I click on the icon in the editor toolbar, I get this ajax error:

The website encountered an unexpected error. Please try again later.

Drupal\Component\Plugin\Exception\PluginNotFoundException: The "" entity type does not exist. in Drupal\Core\Entity\EntityTypeManager->getDefinition() (line 139 of core/lib/Drupal/Core/Entity/EntityTypeManager.php).
Drupal\Core\Entity\EntityTypeManager->getHandler() (Line: 192)
Drupal\Core\Entity\EntityTypeManager->getStorage() (Line: 147)
Drupal\entity_embed\Form\EntityEmbedDialog->loadEntityByAttributes() (Line: 194)
Drupal\entity_embed\Form\EntityEmbedDialog->buildForm()
call_user_func_array() (Line: 534)
Drupal\Core\Form\FormBuilder->retrieveForm() (Line: 281)
Drupal\Core\Form\FormBuilder->buildForm() (Line: 73)
Drupal\Core\Controller\FormController->getContentResult()
call_user_func_array() (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 580)
Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 169)
Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 81)
Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 58)
Drupal\Core\StackMiddleware\Session->handle() (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 270)
Drupal\shield\ShieldMiddleware->bypass() (Line: 137)
Drupal\shield\ShieldMiddleware->handle() (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 49)
Drupal\remove_http_headers\StackMiddleware\RemoveHttpHeadersMiddleware->handle() (Line: 23)
Stack\StackedHttpKernel->handle() (Line: 718)
Drupal\Core\DrupalKernel->handle() (Line: 19)

Drupal version : 9.5.5
PHP version : 8.0.21

@RedNeko thanks for your work on the CKEditor4To5Upgrade Plugin! Confirming the hook_ckeditor4to5upgrade_plugin_info_alter is the correct approach. It looks good to me.

@neclimdul re linking the embedded entity with a wrapping div, hmm I don't think that'll worth either. See the markup that was tried #3349893: [upstream] [GHS] CKEditor 5 removes <a>s that wrap HTML elements not natively supported by CKEditor 5 which seems to use a wrapping div.

@drunir I believe you are using the views_entity_embed module. I don't think it is CKEditor5 ready yet.

Working on:

This also really should have some test coverage.

I think you can reuse a lot from Drupal core's MediaTest. For example:

• \Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testPreviewUsesDefaultThemeAndIsClientCacheable()
• \Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testEmbedPreviewAccess()
• \Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testAlignment()
• \Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testEditableCaption()

That should save you a lot of time :)

Of course, starting with just porting entity_embed's \Drupal\Tests\entity_embed\FunctionalJavascript\CKEditorIntegrationTest::testIntegration() to test the CKEditor 5 equivalent would be a great start! 😊

(I wrote that in #24, in September 2022.)

Just a heads up that I'm working on trying to get an updated Embed module release out today so we can run tests properly here.

First, catching up on what happened since #47 (January 2023):

• Thanks @MrDaleSmith for= updating the version requirements, looks good 👍
• Thanks @el7cosmos for addressing @lauriii's review at https://git.drupalcode.org/project/entity_embed/-/merge_requests/12#note..., looks good! 👍
• Thanks @el7cosmos for adding editing support! I will defer to @lauriii to review that. 😅 I did test it, and it works! 👍
• Thanks @neclimdul for your optimism in saying in #67 this is RTBC pending embed module changes. 😁 I don't think we're quite there yet though 😅 We at the very least need one functional JS test and a kernel test for the upgrade path. We want to make sure this remains maintainable for @Dave Reid, don't we? 😊
• @pmagunia is right in #77 that this should have equivalent SVGs — just confirmed with @Dave Reid in the room at the DrupalCon sprints 👍
• @RedNeko: thanks for adding a CKEditor4To5Upgrade plugin! 👍 I'll write test coverage for that too 😊

I'm adding some base classes and deriver to #3309747: Add CKEditor 5 compatibility and keep supporting CKEditor 4 that should reduce the amount of code necessary here and also help out the rest of the Embed ecosystem, not just this module. So testing this will be challenging until that lands in Embed just as an FYI.

Also we noted during the DrupalCon sprints that we likely need to add a prefix to the button names, and I don't see that added to the MR yet. I'm hesitant to push up a bunch of refactors until that is also committed - I'm not sure what the state of that is @wimleers?

#89: 👍 +1 for moving the deriver there, as discussed.

You're right that not everything we discussed is in this MR yet: functional JS test is still in progress (it was impossible to do uninterrupted work on that at DrupalCon), and indeed the button names are not yet renamed.

Still TODO:

1. @Wim Leers (OR SOMEBODY ELSE!): get functional JS tests working
2. @Wim Leers: rename buttons by prefixing so that conflicts are impossible, which will require an update to the upgrade path test AND the functional JS tests, but those tests will prove that the button rename did not break anything 👍🤓
3. @lauriii: review the JS

I will not have time soon though. The remaining June weeks are swamped. I'll try to get this prioritized by my employer in early July.

Just pushed the final commit that makes the CKEditor 4 → 5 upgrade path test coverage pass tests. That now has comprehensive test coverage and uncovered a few bugs in the process! 👍😊

While working on #3367194: [Entity Browser] Tests are failing on D10 , I ran into the following issue: #3367204: [CKEditor5] Missing dependency on drupal.ajax.

During the tests of entity_browser, the embed button in the CKEditor5 toolbar doesn't respond because the library internal.drupal.ckeditor5 doesn't have a dependency on core/drupal.ajax. In ckeditor5.js there is a method Drupal.ckeditor5.openDialog(), which calls Drupal.ajax and may fail in some cases.

Posting it here since I think the same problem may occur here.

I was trying the apply the entity_embed-3272732-80.patch using cweagans and it failed on two files.

tests/modules/entity_embed_test/entity_embed_test.info.yml
tests/modules/entity_embed_translation_test/entity_embed_translation_test.info.yml

So removed those two files and using a new patch which gets applied and now able to pull the entity into ckeditor5.

Here is my diff of composer.json

+      "drupal/entity_embed": {
+        "Drupal 10 & CKEditor 5 readiness[https://www.drupal.org/project/entity_embed/issues/3272732]": "https://www.drupal.org/files/issues/2023-06-19/entity_embed-3272732-93.patch"
+      },
+      "drupal/embed": {
+        "CKEditor 5 compatibility[https://www.drupal.org/project/embed/issues/3309747]": "https://www.drupal.org/files/issues/2023-03-30/fix_module_embed_compatibility_CKEditor_5.patch"
+      },

#93's changes seem to be working perfectly for me on my local. I'm gonna change this to review and tested so that others can weigh in.

Okay - got it rolling on a lower environment and I get the following error in my php logs

Symfony\Component\HttpKernel\Exception\HttpException: Currently, only CKEditor is supported. in Drupal\embed\Access\EmbedButtonEditorAccessCheck->checkButtonEditorAccess() (line 82 of /var/www/html/docroot/modules/contrib/embed/src/Access/EmbedButtonEditorAccessCheck.php).

First off - thanks for all this!

@laura.gates as noted above, you'll need to patch the embed module as well to get rid of the "only CKEditor is supported" error.

I'm having some trouble with embedded Paragraph entities which contain links - the links interrupt the form's tab order, so once I tab to the CKE field, I can't tab out of it to the next form element. On my screen I can briefly see the first link within a Paragraph flash as if it's selected, but the location of my cursor doesn't change - and every subsequent use of the tab button just flashes that same link. I can Shift+Tab back out of the CKE field, but I can't tab forward past it.

If I add tabindex="-1" to links within the embedded Paragraph, all seems to work as expected, but I'm wondering if there's a better way than this:

// Remove in-Paragraph links from the tab order to prevent breaking the edit form as a whole.
setTimeout(noTabParagraphLinks, 1000);
function noTabParagraphLinks() {
  $('figure.drupal-entity a').each(function() {
    $(this).attr('aria-hidden', 'true').attr('tabindex', -1);
  });
}

I am trying to put right status to this issue. According to @Wim Leers's comment in #91, I am believe this should be in "Needs Review" status.

I tested this MR along with https://www.drupal.org/project/embed/issues/3309747

Environment config:

Drupal 10.1 using DDev
PHP 8.1
Embed & Entity Embed 8.x-1.x branches with patches applied

The MR worked good.

Attached a static patch of the latest MR 12 that can be used in composer files.

Catching up!

• I worked on this during DrupalCon Pittsburgh ~1 month ago. See #90 + #91 for where I left things.
• #93 uploaded a patch of the MR to make it consistently applicable using cweagans/composer-patches 👍
• @navneet0693 added some really solid improvements yesterday! 🤩

Now working on functional JS test coverage as mentioned in #90.1.

Pushed functional JS test!

It tries to port \Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest as directly as possible. This worked out well:

• ✅ ::testConversion()
• ✅ ::testOnlyDrupalMediaTagProcessed() → ::testOnlyDrupalEntityTagProcessed()
• ❌ ::testMediaArbitraryHtml() → ::testEntityArbitraryHtml() ⇒ JS BUG DISCOVERED! 👍
• ❌ 🤷‍♂️ ::testLinkedMediaArbitraryHtml() → ::testLinkedEntityArbitraryHtml() + ::testLinkability() + ::testLinkManualDecorator()⇒ linkability of embedded entities is still missing , but per #74, that didn't quite work in CKEditor 4 either, so it's not a regression, so we shouldn't hold this up on that. per #123, this is a regression after all!
• ✅ ::testErrorMessages()
• ✅ ::testPreviewUsesDefaultThemeAndIsClientCacheable()
• 🗑️ ::testAlignment() + ::testDrupalMediaStyleInDropdown() + ::testDrupalMediaStyleWithClass() have not been ported because the alignment functionality (for now) is present solely in the Drupal dialog — that's also less disruptive because AFAIK many sites are customizing the dialog using hook_form_alter()
• 🗑️ ::testEditableCaption() has not been ported for the same reason
• 🗑️ ::testViewMode() has not been ported for the same reason
• 🗑️ ::testAltDisabled(), ::testAlt() and ::testTranslationAlt() have not been ported because it's irrelevant for Entity Embed — that's special-case logic/UX for image media
• ✅ ::testEmbedPreviewAccess()
• ❌ ::testMediaPointerEvent() → ::testEntityPointerEvent() ⇒ JS BUG DISCOVERED! 👍

Hi wim!

linkability of embedded entities is still missing, but per #74, that didn't quite work in CKEditor 4 either, so it's not a regression, so we shouldn't hold this up on that.

Welp... problem located.

I tried to use the patch from #101, but in "Text formats and editors" config page, when I switch the drop-down to "CKEditor 5" I get the following error in the ajax request:

TypeError: Drupal\entity_embed\Plugin\CKEditor4To5Upgrade\EntityEmbed::__construct(): Argument #3 ($plugin_definition) must be of type Drupal\ckeditor5\Plugin\CKEditor4To5UpgradePluginInterface, array given, called in /app/web/modules/contrib/entity_embed/src/Plugin/CKEditor4To5Upgrade/EntityEmbed.php on line 67 in Drupal\entity_embed\Plugin\CKEditor4To5Upgrade\EntityEmbed->__construct() (line 50 of /app/web/modules/contrib/entity_embed/src/Plugin/CKEditor4To5Upgrade/EntityEmbed.php)

BTW: the patch from #93 works fine.

My environment:
* Drupal 9.5.9
* PHP 8.0
* embed 1.6
* entity_embed 1.4

It couldn't be applied in 1.4, so I made a temporary patch. from #93.

The latest from MR12 didn't apply cleanly. This is the URL I used on the 1.x-dev version of the module:

https://git.drupalcode.org/project/entity_embed/-/merge_requests/12.patch

The entity_embed.info.yml and entity_embed.module files were out of sync.

Even after applying the changes manually the embed icons were missing for me on the text format config page.

#93 worked well for me though.

Thanks everyone for all the hard work on this!

I wonder if it was intentional to set embedded entities to only be allowed at the block level? I'm looking after a site at the moment which embeds entities within text, and upon updating those have all been shunted up the DOM when opened in ckeditor5.

I messed around a bit with the options in _defineSchema():

    schema.register('drupalEntity', {
      isObject: true,
      isContent: true,
      isBlock: false,
      allowWhere: ['$block', '$text'],
      allowAttributes: Object.keys(this.attrs),
    });

to no avail. For reference this is using paragraph_inline_entity_form module to embed a couple of words which open into a tooltip on hover for displaying academic references, they very much need to be in-text.

In Text formats and editors config the new default entity.svg embed button icon is properly being loaded for ckeditor5 editors, but in Embed buttons config the old default entity.png icon is being loaded.

This is also being reported as an error on the status report page

Hmm... the fix is bouncing between MRs and patches which is pretty confusing.

@Wim Leers This is still assigned to you. Would be curious for your assessment on next steps here please :)

p.s. We have people on my team that could probably help move this along if you need more fixes/review/testing/etc.

@Kristen Pol The MR contains the truth. The patches are just extracted from the MR, to have a stable patch to apply (every commit to the MR would change the auto-generated patch from the MR). This is a limitation of GitLab 🤷‍♂️

I should've unassigned this issue from me in #104, sorry. I am not sufficiently proficient in the CKEditor 5 JS APIs to efficiently fix this. I'm hoping @lauriii can do that in the next few weeks. (He, @nod_ and @bnjmnm wrote >90% of the JS for the CKEditor 5 module!)

Basically: as soon as all functional JS tests I added pass, I'd instantly RTBC, commit and ship this 🤓 (Not because I wrote those tests, but because they're ported from Drupal core's similar "media embed" functionality, which is very thoroughly tested after years of deep reviews and edge case testing!)

Thanks for the great summary of status and next steps 🙏

UPDATE: not sure why it shows the tag changed. I didn’t touch those. But I’m on my phone so 🤷‍♀️

Hey, just pushed a commit to address the issue I mentioned in #110

Please let me know if there is any related code that I should update besides this.

(I could also clean up that \Drupal::service() call and properly inject the extension.list.module service while I'm at it)

Got this up and running! Many thanks to everyone working on this.

MR#12 Works (after installing https://www.drupal.org/project/embed and applying a patch: https://www.drupal.org/project/embed/issues/3309747#comment-15166227)

Let's ask Embed module to get issue #3309747 released and release this issue too!

D9 EOL is only 2 months away!

Attaching static patch with HEAD at 4d4809529ccfbe64c7488d4438f92e1bff52b900

Everything's working well, except this functionality #2511404: Image entities/fields embedded using Entity Embed cannot be linked in CKEditor it's not possible to create a link around an embedded entity in CKE5

Yeah that's effectively the same as my issue per #109

So this is a OOTB media embed of CKEditor 5 and it has a link feature:

Maybe entity-embed should have something similar?

To speed up release - maybe this feature should be a separate issue?

#120: that's known. See the tests I added and the summary of what passes and fails in #104. It didn't work in CKEditor 4 either. Apparently I fixed this in 2019 over at #2511404-77: Image entities/fields embedded using Entity Embed cannot be linked in CKEditor and committed it in #81 there?! 😳😅 I have no recollection of this! 👴

That means that in #104, the 4th bullet with the "🤷‍♂️" should really be a "❌" too. That means there are THREE bugs to be fixed in the JS, not TWO.

@jannakha We can't release this as-is, because that could result in data loss (links disappearing) when editing existing content with linked embedded entities using CKEditor 5.

Should entity_embed work with the "Limit allowed HTML tags and correct faulty HTML" filter? The embedded entity is not displayed when I have that filter enabled.

@Pasqualle Yes, it should, and it does. The test coverage proves it does. The filters must be ordered correctly though. entity_embed_filter_format_edit_form_validate() ensures the order is valid. The only ways it cannot work: the FilterFormat on your site was created with that validation logic disabled and/or a new bug. If it's the latter, please create a new issue 😊🙏

Clarified the remaining work.

Fixed the pointer test by porting the (CSS-only) fix from #3326455: [drupalMedia] When media is embedded in a view mode whose display is configured to link elsewhere, that link should not be clickable in CKEditor 👍

2 JS bugs left!

For the two remaining JS bugs surfaced in #104, we need to port:

1. ::testEntityArbitraryHtml() → #3231337: [drupalMedia] Remove manual dataDowncast from DrupalMediaEditing
2. ::testLinkManualDecorator() → #3248228: Unable to change selection after linking inline media when manual decorators have been defined

… and maybe more. But at least the patterns in those issues should help a lot in getting this closer to the finish line.

Anybody interested? 🤞

@Wim Leers,

I am willing to offer help, but it seems that my knowledge is limited, I am highly willing to make point 2 in #127 work.

Observation:-
if you add a piece of media using the entity embed on the formatting toolbar, then close out without making a selection, you are taken to the top of the body section.
Steps to reproduce
* Navigate to the bottom of the body content section
* Place your cursor
* Select the “entity embed icon”
* Click the “x” in the top right corner without making a selection
* See that you are taken to the top of the body section

If the content inside

#93 works great (with the patch to drupal/embed)

Just adding "Drupal 10 compatibility" label.

Attaching latest diff from MR !12 as static patch for composer builds.

I was looking at the 2 Errors in the PHPUnit tests, and I am thinking that they are because the aria labels have changed

There were 2 errors:

1) Drupal\Tests\entity_embed\FunctionalJavascript\CKEditor5IntegrationTest::testLinkManualDecorator with data set "restricted" (false)
Behat\Mink\Exception\ElementNotFoundException: Element matching css ".ck-balloon-panel_visible .ck-balloon-rotator__content > .ck.ck-toolbar[aria-label="Drupal Entity toolbar"]" not found.

/var/www/html/vendor/behat/mink/src/WebAssert.php:418
/var/www/html/core/tests/Drupal/Tests/WebAssert.php:968
/var/www/html/core/modules/ckeditor5/tests/src/Traits/CKEditor5TestTrait.php:116
/var/www/html/modules/contrib/entity_embed/tests/src/FunctionalJavascript/CKEditor5IntegrationTest.php:608
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:703

2) Drupal\Tests\entity_embed\FunctionalJavascript\CKEditor5IntegrationTest::testLinkManualDecorator with data set "unrestricted" (true)
Behat\Mink\Exception\ElementNotFoundException: Element matching css ".ck-balloon-panel_visible .ck-balloon-rotator__content > .ck.ck-toolbar[aria-label="Drupal Entity toolbar"]" not found.

/var/www/html/vendor/behat/mink/src/WebAssert.php:418
/var/www/html/core/tests/Drupal/Tests/WebAssert.php:968
/var/www/html/core/modules/ckeditor5/tests/src/Traits/CKEditor5TestTrait.php:116
/var/www/html/modules/contrib/entity_embed/tests/src/FunctionalJavascript/CKEditor5IntegrationTest.php:608
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:703

And we could probably fix this by changing the text to "Drupal Media toolbar"

#135 was wrong, and we should probably revert that change, and #127 still stands as the path forward.

In my manual testing of the functionality covered in the testEntityArbitraryHtml test, it indeed did not work. Do we need to implement a version of the "DrupalMediaGeneralHtmlSupport" plugin the core CKEditor5 module has for this functionality?

I added a general html support plugin based off of the one in core media. This should fix the testEntityArbitraryHtml test.

I added a link plugin ported from core's DrupalLinkMedia plugin. This should fix the testLinkedEntityArbitraryHtml test.

I added the CKEditor link ui plugin to the entity embed balloon so the testLinkability and testLinkManualDecorator tests could run.

I did also have to update some of the assertions in those tests to reflect the test data. For instance the test data didn't have an image. I think I updated the assertions in such a way that they are still effectively testing the same functionality.

Of course all the changes I've made should be reviewed thoroughly.

The only test I see still failing is the UpgradePathTest test. I think that's related to the missing data-view-mode attribute.

OK, looks like all the tests are passing now. Setting to "Needs review".

Such dedication @PCate! Great job 😉

Added one comment, didn't check everything yet.

@PCate: 🤯🤩 HOLY COW! That is incredible, thank you so much! 👏🙏

I'm hoping that @bnjmnm or @lauriii can review this soon 🤓🤞 But it looks fantastic!

@PCate: 🔥🙌

Superb work ! thank you so much for pushing this.

I'm going to put some quality time into reviewing the current MR. Thanks to @PCate it is looking quite good!

Found an issue with core that prevents the embed module from allowing svg uploads for the button config: #3394406: FileUploadHandler::handleExtensionValidation does not have fallback for sites still using file_validate_extensions

This can also be addressed in the embed module with a one line addition. That might get committed faster, too, so I also created #3394408: Button icon extension validation settings do not work with versions >= 10.2 or < 11

The MR here still needs JS that accounts for non-SVG button images. More info in the MR.

The latest changes in MR12 seem to be working well, the linking is working great :)

The changes in MR12 seem working fine. I applied these changes for a project I'm working on. I run into issue with alignment. If the is aligned right, then I cannot select the entity.

I think the class align-right should be applied at the tag <figure> :

@darvanen in #109 you mention needing to preserve the ability to embed entities inline within text like you do with CKEditor 4. I did some manual testing with CKEditor 4 + Entity Embed and this kind of in-text embedding does not seem to be supported, but perhaps there's a step I'm missing. I'll demonstrate what currently happens for me:
If I start with this:

<h2>Rice milk</h2>

<p>The flavor of rice milk perfectly complements the cocoa beans and it was one of the first dairy milk alternatives to be used to create vegan milk chocolate. Rice milk is made from rice powder and is sometimes combined with hazelnut milk to create the correct texture and taste that is ideal for producing all kinds of chocolate bars.</p>

Then embed an entity mid-sentence or mid-word, the single paragraph is split into two paragraphs with the embed between them

<h2>Rice milk</h2>

<p>The fla</p>

<drupal-entity data-embed-button="node" data-entity-embed-display="entity_reference:entity_reference_label" data-entity-embed-display-settings="{&quot;link&quot;:1}" data-entity-type="node" data-entity-uuid="05f5c4f3-73e7-4eb5-a971-a297e445a8b0" data-langcode="en"></drupal-entity>

<p>vor of rice milk perfectly complements the cocoa beans and it was one of the first dairy milk alternatives to be used to create vegan milk chocolate. Rice milk is made from rice powder and is sometimes combined with hazelnut milk to create the correct texture and taste that is ideal for producing all kinds of chocolate bars.</p>

Based on the above I'm not sure this something directly supported by Entity Embed. Perhaps the inlining is made possible by a additional contrib or custom module? If you have steps to reproduce that accomplish this in CKEditor(4) with just Entity Embed, please share. If it's demonstrated that Entity Embed's CKEditor4 functionality supports inlining without additional help, then the CKEditor 5 version should support it as well, but it's not yet clear if Entity Embed can do that independently.

Re #151 good catch! Commit 066e6299 addresses this by moving the align classes to the widget container in the editing downcast conversion. This functionality is verified by \Drupal\Tests\entity_embed\FunctionalJavascript\CKEditor5IntegrationTest::testAlignClassAddedToWrapperWhileEditing. This might address the concerns in #109 as well, if the inlining was dependent on the alignment classes being applied to the correct element.

It looks like my recent commits have CKEditor5IntegrationTest failing. Based on the failures, it looks like CKEditor 5 is not initializing.
This is passing for me locally, but I'm on core 10.x lcoally. It looks like shortly after my commit @Dave Reid added the ability to test on 10 so I'll see if I also have that capability on my next push.

I just tagged a release of Embed module so we might need to re-run with that (and also bump the requirements to that version).

@bnjmnm Can you revert the changes to skipping tests? I think that would be resolved by using GitLab CI since I don't think DrupalCI was downloading the contributed ckeditor module.

This patch is just to test using the contrib CKEditor4 module for tests, to confirm it will cooperate with 9 and 10. Don't expect to see any actual functionality here 🙂

Re #156

Can you revert the changes to skipping tests? I think that would be resolved by using GitLab CI since I don't think DrupalCI was downloading the contributed ckeditor module.

On the way - I mistakenly thought it was necessary due to (long story). However, based on #157 (which is basically testing HEAD on 9 and 10) it does look like some adjustments will need to be made for a few tests using CKEditor 4 to pass on Drupal 10, but since some CK4 tests are still passing it hopefully means just updating some selectors.

Did a quick test of this and it looks and works amazing! Great work!

Merged the changes to 8.x-1.x. Thank you so much every, it means the world that all of you helped get this across the finish line. Thank you.

I will work on filing any follow-up issues needed:

1. Making the caption feature work better with native CKEditor5 instead of using the caption filter.

I think it would be good to give this a day more of testing with everyone else on the 8.x-1.x-dev before I tag a release as well, if that's okay.

When I install the new Oct 20th Dev version or apply the most recent version of the patch (https://git.drupalcode.org/project/entity_embed/-/merge_requests/12.diff) to 1.4.0 I am getting js errors in the console and the Ckeditor UI is not loading in the following scenario:

When I start a node creation page for any content with Ckeditor 5 field using a format that has the ability to embed entities the editor fails to load. When I switch to a format on that textarea that doesn't embed entities the editor loads. Also if I disable the embed entities ability in my format that is broken, and save it, the editor loads again for that format. The error I receive on the front end is the following:

ckeditor5.js?s2yxym:201 TypeError: Cannot read properties of null (reading 'getAttribute')
    at Object.callback (drupalentity.js?s2yxym:1:6039)
    at i.create (ckeditor5-dll.js?v=35.4.0:5:416819)
    at ckeditor5-dll.js?v=35.4.0:5:439488
    at Array.map (<anonymous>)
    at X.fillFromConfig (ckeditor5-dll.js?v=35.4.0:5:439398)
    at $.register (ckeditor5-dll.js?v=35.4.0:5:555056)
    at s.afterInit (drupalentity.js?s2yxym:1:6609)

Strangely enough, I used this patch on another site built practically the same exact way a couple weeks ago with that same patch from around Oct4 on module version 1.4.0 and it's still running fine. So something that has changed since then is what seems to be causing the issue. Just to be sure I copied that entity embed module patched version from the Oct 4th working site into the current one that isn't working and my formats are working again.

Not sure if anyone else is getting this issue. Hope this helps. Kind of a blocking issue for me to update a client site to Drupal 10. As they won't be able to manage any of their embedded media in any content without this patch. If I can help give any more context let me know.

Also this site is still on Durpal 9 that is busted. I am updating it to Ckeditor 5 in Drupal 9 before upgrading it to Drupal 10. If that matters or helps as well.

Yes, I am also facing this same above issue after using the latest dev version while updating from Ckeditor4 to Ckeditor5, the editor does not loads up. Also if I disable embed options on a text format the editor loads normally without any error, also my site is already on D10 latest version

1. After applying the patch mentioned in #163 and clearing the cache, I am getting the following error.
2. Error: Call to undefined method Drupal\entity_embed\Plugin\EmbedType\Entity::getModulePath() in Drupal\entity_embed\Plugin\EmbedType\Entity->getDefaultIconUrl() (line 243 of modules/contrib/entity_embed/src/Plugin/EmbedType/Entity.php).
3. And also, as mentioned in #162, the PR is not merged yet, so reopening this issue.
4. Correct me if I am wrong somewhere in my understanding.

@Deasly, @siddharthjain and @sahilgidwani: Did you also update drupal/embed to 1.7 or newer? (The minimum version of that dependency has been updated by this MR.)

@Wim Leers, Yes I have updated the embed module to latest version i.e. 1.7

I am facing the same issue as described in #163 using embed 1.7 and dev version (ac6f2fe1). For some reason getSelectedElement is null and therefore the succeeding code fails.

                e.ui.componentFactory.add("editEmbeddedEntity", (i=>{
                    const r = new a.ButtonView(i)
                      , n = e.model.document.selection.getSelectedElement()
                      , o = n.getAttribute("drupalEntityEntityUuid")
                      , s = n.getAttribute("drupalEntityEntityType")
                      , l = Drupal.url(`entity-embed/edit-embedded/${s}/${o}`);

My intent in that comment was the PR was in fact merged. I think we should file follow-up issues for anything that is being reported. Everyone, please file new bug reports only after confirming that your version of the Embed module is the 1.7 release.

Hi @Wim Leers i was also using the with the drupal/embed module at 1.7 as per the requirements and still having the issue.

I created a new issue queue as per Dave Reid's request here: https://www.drupal.org/project/entity_embed/issues/3396133

I hope I did this the right way.

We were having the same issue as described in #163, but it turned out to be our hosting provider being the problem (Pantheon). We applied the patch using cweagans/composer-patches. on a Pantheon multidev (which does not show composer logs).

Apparently, sometimes composer patches just doesn't work on multidevs (possibly due to the size of this patch now with lots of tests).

Long story short, if you see this issue and are using cweagans/composer-patches, on a pantheon multidev with integrated composer you can possibly fix it by forcing composer to rebuild a couple of times by deleting the lock file.

Anyway, I think this patch is ready to be included in a release, after testing this for 3 days.

FYI at this point you shouldn't be still using the patches, you should use the dev release and report any problems to new issues.

@bnjmnm in #153 you tried to embed an entity mid-sentence or mid-word, the single paragraph is split into two paragraphs with the embed between them is not a default behavior, there is a issue thread with patch regards to that to preserve the inline entity embed, I am using that patch for our D9 site.

please check this one https://www.drupal.org/project/entity_embed/issues/2640398, if we can make the inline entity embed to work on CKEditor5 would be great.

Re #173. Perhaps I'm reading this incorrectly, but are asking for functionality that is not part of a tagged release, but only available y applying a patch from the issue #2640398: Allow <drupal-entity> elements to be inline CKEditor Widgets?

@bnjmnm, yeah the patches in https://www.drupal.org/project/entity_embed/issues/2640398 don't work with any version of CK5. Would be great to be able to embed entities inline like we can with CK4.

@bnjmnm, yeah the patches in https://www.drupal.org/project/entity_embed/issues/2640398 don't work with any version of CK5. Would be great to be able to embed entities inline like we can with CK4.

The first step towards getting there would be completing the issue you referenced: #2640398: Allow <drupal-entity> elements to be inline CKEditor Widgets.

This issue's scope was for ensuring that any existing (I.e. in a tagged relase) CKEditor 4 features are also supported by CKEditor 5, and that appears to be complete.

The inline embed functionality is only provided in an issue that has not been completed (#2640398). It would be reasonable to expand the scope of #2640398 to include CKEditor 5, but this issue was specific to providing CKEditor 5 support for an unpatched entity embed.

For anyone needing a fix *right now* the work in MR28 was commissioned to unstick a client project, it adds an 'inline' embed style, thanks so much @jannakha!

I am using the drupal/embed module at 1.7 and applied the patch for entity_embed https://git.drupalcode.org/project/entity_embed/-/commit/ac6f2fe13b18f5f...

It's working fine for me with the Drupal 10 version.

@Dave Reid Please let me know when the new CK5 compatible version of the module will be released.

Release including this and the follow-up fix was made today. Please report any followups as separate issues.

What a story - thank you all for the hard and intense work!

I upgraded my Drupal version to 10.3.2 and met the same issue with entity_embed:
Error: Call to undefined method Drupal\entity_embed\Plugin\EmbedType\Entity::getModulePath() in Drupal\entity_embed\Plugin\EmbedType\Entity->getDefaultIconUrl() (line 243 of modules/contrib/entity_embed/src/Plugin/EmbedType/Entity.php).

I upgraded the version of entity_embed in my composer.json file to version 1.6 (1.7 doesn't exist apparently) but the issue remained,
I had to apply the same patch mentioned in https://www.drupal.org/project/entity_embed/issues/3272732#comment-15306099.

Was entity_embed 1.7 removed? How can we make sure the patch from https://git.drupalcode.org/project/entity_embed/-/commit/ac6f2fe13b18f5f... is included in the entity_embed package?

@pyxy - I think the 1.7 minimum requirement was for the Embed module not entity_embed.

this following patch is compatible for testing ckeditor5 plugin with d9.4 before migrate to d10.