Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
CKEditor5 only supports SVGs for the embed icons. If a user uploads an image for the icon which is not in an SVG format, it will not show up on the toolbar for CKEditor5. Embed should reflect this, and only allow SVGs, or at least add an info message when a user tries to upload anything else it might not show up on the toolbar. We might also want to consider a BC aspect of this change, as there might be sites using other extensions already for CKEditor4, and they would probably "lose" their icons when updating to CKEditor5.
Steps to reproduce
Proposed resolution
Change the file_validate_extensions value on EmbedButtonForm to only allow SVGs.
Remaining tasks
User interface changes
There might be some changes on the form, depending on the decision if we want to add an info message here.
API changes
-
Data model changes
-
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | status.png | 38.33 KB | samlerner |
Issue fork embed-3310328
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #4
dave reidIt probably makes sense to start a new 2.x branch only for ckeditor5 compatibility.
Comment #5
dave reidWe probably don't need file_validate_image_resolution anymore either if we're requiring SVGs.
Comment #6
dave reidSome ideas for how to handle existing icon images:
https://github.com/cesarmiquel/PNG2SGV
https://express.adobe.com/tools/convert-to-svg
https://convertio.co/png-svg/
https://image.online-convert.com/convert/png-to-svg
Comment #7
balintpekkerI agree with we don't need the
file_validate_image_resolutionupload validator anymore, however we would want to convert the size of the uploaded image on the list of embed buttons not to have different SVG sizes, which would ruin the UI.Comment #8
idiaz.roncero+1 to this. I opened an issue related to SVG sizing, I guess it could be marked as fixed when this issue is solved.
Added to related issues.
Comment #9
wim leersCan we not instead allow both SVG and PNG, to address the BC concern? #6 is interesting but we can't call those automatically from within Drupal. So perhaps a context-dependent validation would be appropriate: if the CKEditor 5 module is installed, trigger a validation error if Embed only has a PNG button icon and not also a SVG button icon?
Then there's zero effect for existing sites. There's a reasonable change in behavior for sites using CKEditor 5, but only an essential change.
What do you think about that middle ground, @Dave Reid?
Comment #10
dave reidYeah I feel like allowing for both PNG and SVG for now is the best solution. I think we could also add a hook_requirement runtime check that if only the ckeditor5 module is enabled (and not ckeditor), that if any embed buttons do not have SVGs to add an error that it needs to be fixed. We could potentially look at writing an update hook to programatically update PNGs to SVG, but that will require a larger effort.
Comment #11
dave reidI can work on updating the MR to get this ready today.
Comment #12
samlerner commentedI tested this out and it works great. I cloned the MR and when I went to the Status Report, I saw this output:
Comment #13
dave reidComment #14
dave reidComment #15
dave reidThe D10 test failure seems to be #3190024: Problem with test dependencies when testing issue forks
Comment #16
dave reidI've thoroughly been testing this and am going to merge it since it helps unblock Entity Embed and CKEditor5.
Comment #18
dave reidComment #19
wim leersEpic turn-around time here! 🤩 Talked to @Dave Reid yesterday, and now it's already fixed 🤯
Comment #20
dave reidI need to make it to more events! :D
Comment #21
wim leers😄 When I told Megh & Tim about our encounter on the street where you informed me of this having landed, they said "THAT IS SOOOOO DAVE!" 😁