CKEditor 5 Support

Unsure about the architecture questions. kinda weird to have the embed dependency at all which has the multiple instances thing built-in and then not actually use that, so maybe, maybe removing that might make sense, but that's a fairly big change too.

@mark_fullmer, I created the PR because is easiest to know the changes without applying the patch on a clone project, in this case, the patch was huge. Also, for maintainers is easy to merge and understand what is going on.

As discussed, we need to decide what to do with the plugin, possibly flag as deprecated, with a label/explanation in the UI and remove in an upcoming major version.

That said, the removal of the embed module dependency seems barely related to CkEditor 5 support (it's 90% copy pasting of the code from the embed module that we *do* need), so I'd suggest we split that up into two issues either way, get this in first and then the dependency change becomes easier to review and manage?

I'm not sure when I'll have time to really test this myself, so what would help a lot is people reporting back on whether or not this works for them.

I applied your patch to 2.x-dev running Drupal 9.5.5, with CKEditor 5. And it works, except for Facebook and Instagram, they are throwing the error, below. Which may or may not be related to this issue: https://www.drupal.org/project/url_embed/issues/3272702

(I tested it with Vimeo, Youtube, Twitter, Facebook and Insta.)

Warning: Attempt to read property "html" on null in Drupal\url_embed\Plugin\Filter\UrlEmbedFilter->process() (line 78 of modules/contrib/url_embed/src/Plugin/Filter/UrlEmbedFilter.php).
Drupal\url_embed\Plugin\Filter\UrlEmbedFilter->process('
', 'en') (Line: 118)
Drupal\filter\Element\ProcessedText::preRenderText(Array)
call_user_func_array(Array, Array) (Line: 101)
Drupal\Core\Render\Renderer->doTrustedCallback(Array, Array, 'Render #pre_render callbacks must be methods of a class that implements \Drupal\Core\Security\TrustedCallbackInterface or be an anonymous function. The callback was %s. See https://www.drupal.org/node/2966725', 'exception', 'Drupal\Core\Render\Element\RenderCallbackInterface') (Line: 788)
Drupal\Core\Render\Renderer->doCallback('#pre_render', Array, Array) (Line: 374)
Drupal\Core\Render\Renderer->doRender(Array, 1) (Line: 204)
Drupal\Core\Render\Renderer->render(Array, 1) (Line: 160)
Drupal\Core\Render\Renderer->Drupal\Core\Render\{closure}() (Line: 580)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 161)
Drupal\Core\Render\Renderer->renderPlain(Array) (Line: 91)
Drupal\embed\Controller\EmbedController->preview(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 580)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 169)
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: 50)
Drupal\ban\BanMiddleware->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: 718)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

re: #19

I'm a goof, I did not know you needed an API key for Facebook/Instagram. I did not have one when I tested, and I am fairly certain that is probably why it failed.

In a perfect world, an exception might be helpful there.

(Sorry we just don't use them, I just thought I'd try them. I need it for Youtube and Vimeo.)

So someone who has those API credentials should test those. Otherwise it worked great for me.

Thanks.

My organization has been using the latest patch on multiple sites in production, without incident, for over a week, so I think this patch is good to go.

Confirming also that the latest patch was used on multiple sites in production without issues, so I also agree that this patch is good to go.

Yet another confirmation that the patch in the merge request works as intended. Thanks!

Hi,

I wanted to provide an update regarding the patch I received from the merge request and my attempt to apply it. Unfortunately, the patch did not produce the expected results. Here is what I did:

1. Updated the contrib module to version 2.x.
2. Added the patch to the composer.json file.
3. Made an attempt to apply the patch.

Also, I found an error in the output:
"Could not apply patch! Skipping. The error was: Cannot apply patch."

I would appreciate any help in this regard

No, I didn't use that syntax. Instead, I used the following syntax:

            "drupal/url_embed": {
                "CKEditor 5 Support": "./patches/10.patch"
            }

I used the syntax in #26 and it worked properly, but I'm curious as to why the patch file is not functioning as expected. Just to clarify, I downloaded the patch file from the merge request and copied it into my project. Is there anything else that needs to be done in order to apply the patch file successfully?

@mark_fullmer Great news! I successfully applied the patch, and I'm sharing it here just in case anyone else might find it useful

I applied the patch from #30. Patch applies cleanly but when i set text format as CK5 i still get: The CKEditor 4 button url does not have a known upgrade path.

I am pretty sure that "url" button is this module; is it not?

@liquidcms

I am indeed also experiencing the same issue. Not only would you get that, but also adding your own embed buttons through admin/config/content/embed on the type url is not possible, as they do not show in the toolbar.

Update:
I see in the topic started thread both have been chosen on purpose. So I guess we need to write our own migration, but also for users who had their own embed button defined (in my case to also alter the behaviour) we need to remove this and alter the original one.

I am just coming here as I have used this plugin with wysiwyg module and ckeditor 4.

Actually I have migrated to ckeditor 5, however where I had previously added via url_embed module, iframes are not viewable in ckeditor. That said the embeds are not updated until I save the node, so iframe is still there where i have not updated/saved yet.

My guess is that these will be stripped out if I try to save these nodes with out ckeditor 5 support.

I have tried this patch url_embed_ckeditor5_support.patch and could not notice any change here? (btw this is one of the biggest patches I have ever applied)

I propose simplifying the implementation to be designed around a single button.

Probably should be a separate issue to this one.

All I am really interested in is how to get this working with ckeditor 5. What is the change here required to achieve that?

Ok, I have discovered the iframe and iframe_media module. Works with CK editor 5 like any other media type entity.

https://www.drupal.org/project/media_iframe

Recently we are updating to Drupal CKEditor 5, drupal core media uses oembed, but only has Vimeo and youtube.

But there is a contrib module that allows you to choose more providers https://www.drupal.org/project/oembed_providers

Should we keep the URL Embed if the Media Core already provides it?

Maybe we should provide a migration from URL embed to core media.

As of patch https://www.drupal.org/project/embed/issues/3310328 in Embed module all buttons require and SVG icon to be displayed.

I have amended the patch to use CKEditor5 icon.

I humbly submit this svg icon I made for the button.
Note: I had to wrap it in a zip file to upload.. just extract and review.

Helps if I upload the zip too.

Finally got around to properly test this on Drupal 10 with CKEditor5.

This seems to work well enough and I'm going ahead and committing it, we can make further adjustments later on if necessary.

@Spurlos: Can you create a new issue for that? If you use ckeditor 5 then you no longer need an embed button at all, that would only be when still using CKEditor4 and therefore unrelated to this issue.

@emptyvoid: I think I prefer the current svg icon in the project, feel free to create a new issue and propose that, if others like it too then I'm open to changing it.

@mark_fullmer: Do I understand correctly that if we remove the ckeditor4 integration, this module wouldn't depend on embed anymore at all and we could do a 3.x release (maybe, in the future) that removes both that plugin and the embed.module dependency?

@mark_fullmer please note that MR10 does not contain the svg fix that #37 has. Not sure if that is intentional, but I was assured by frontend team that it is vital. Which means this issue isn't completely fixed yet. We're currently applying this patch on alpha2.

It is my understanding that this is only the CKEditor 4 integration and unused for CKEditor5 and that's why I suggested doing that as it's own issue. If you migrated to CKEditor5 you can remove that embed configuration AFAIK.

Even if not, at this point it should be filled in a new issue.