Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support uploading SVG images #674

Closed
davidpolberger opened this issue Nov 17, 2017 · 8 comments Β· Fixed by ckeditor/ckeditor5-image#323
Closed

Support uploading SVG images #674

davidpolberger opened this issue Nov 17, 2017 · 8 comments Β· Fixed by ckeditor/ckeditor5-image#323
Assignees
Labels
type:feature This issue reports a feature request (an idea for a new functionality or a missing option).

Comments

@davidpolberger
Copy link

🐞 Is this a bug report or feature request? (choose one)

  • Feature request

πŸ’» Version of CKEditor

v1.0.0-alpha.2

πŸ“‹ Steps to reproduce

  1. Browse to https://ckeditor5.github.io/
  2. Attempt to upload an SVG file.

βœ… Expected result

The uploaded image should be properly displayed.

❎ Actual result

Nothing happens, or only the textual file URL appears.

πŸ“ƒ Other details that might be useful

CKEditor 4 does not appear to support SVG files either. However, SVG's growing popularity may make it an attractive format to support.

@Reinmar Reinmar added status:confirmed type:feature This issue reports a feature request (an idea for a new functionality or a missing option). labels Nov 17, 2017
@Reinmar
Copy link
Member

Reinmar commented Nov 17, 2017

Great idea. Thanks for reporting!

On CKEditor 5 side:

But in order to make SVGs work with Easy Image we'll also need to add support for them in CKEditor Cloud Services.

@Reinmar Reinmar added this to the unknown milestone Feb 14, 2019
@Maqsyo
Copy link

Maqsyo commented Aug 27, 2019

Is here any progress?

@Reinmar
Copy link
Member

Reinmar commented Aug 27, 2019

No, we didn't look into this. But it seems to be a trivial change assuming that the backend that you're using will support SVGs. Easy Image doesn't support them AFAIK. CKFinder should work fine (may require some configuration). If you use your own backend then it's basically up to you.

So, anyway, I think we could try to add SVGs to https://github.com/ckeditor/ckeditor5-image/blob/42ad41bea2e406233c17ce8a319fdd66b1ba495d/src/imageupload/utils.js#L18 and see what happens. It may be a quick win.

@Reinmar Reinmar modified the milestones: unknown, iteration 27 Aug 27, 2019
@jodator
Copy link
Contributor

jodator commented Aug 27, 2019

Looks like it's working fine with EasyImage. It will work with CKFinder as well (you only need to enable SVG file type in the CKFinder backend configuration).
Peek 2019-08-27 12-47

@jodator
Copy link
Contributor

jodator commented Aug 27, 2019

And the model contents:

Selection_014

@jodator
Copy link
Contributor

jodator commented Aug 27, 2019

It looks like adding this will work but it is a potential security risk if image backend does not properly filter SVG files. Also when thinking about any future or existing image format that might or might not be supported on the backend I think that the best way is to provide a configuration option for image upload for extending supported image types.

ClassicEditor
	.create( document.querySelector( '#editor' ), {
		image: {
			toolbar: [ 'imageStyle:full', 'imageStyle:side', '|', 'imageTextAlternative' ],
			upload: {
				types: [ 'jpeg', 'png', 'gif', 'bmp', 'svg+xml' ]
			}
		}
	} );

Underneath it will create the current RegExp:

https://github.com/ckeditor/ckeditor5-image/blob/8fa4acf7850e958526d0f049899f88cb91282a74/src/imageupload/utils.js#L19

The default value will be:

[ 'jpeg', 'png', 'gif', 'bmp', 'webp', 'tiff' ]

I'm not sure is the tiff should be enabled by default so we might skip it and only add the webp to default image subtypes.

ps.: Here is a full list of possible image subtypes

@Reinmar
Copy link
Member

Reinmar commented Aug 27, 2019

I think we can go this way if it will also affect this place: https://github.com/ckeditor/ckeditor5-image/blob/42ad41bea2e406233c17ce8a319fdd66b1ba495d/src/imageupload/imageuploadui.js#L38

Otherwise, this configuration is a bit redundant because the backend will need to do the same check and should throw if it doesn't want to support for example SVGs. So, the backend gives you the final confirmation (once it receives the file). The config option will affect the UI, so will give you immediate result (which files are available in the native file browser).

@jodator
Copy link
Contributor

jodator commented Aug 27, 2019

Yeah... this should go there as well: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/file#Limiting_accepted_file_types so we should construct the accept key according to allowed image types... or allow all image types always (this is image would check for image/ only and left it to the server.

ps.: I'm for the configuration option to make CKEditor not upload SVG by default:

  1. Add configuration option image.upload.types
  2. Use image.upload.types in ImageUploadUI to browse for compatible file types only.

Reinmar added a commit to ckeditor/ckeditor5-image that referenced this issue Sep 2, 2019
Feature: Introduced `config.image.upload.types` configuration option for setting allowed image mime-types. Closes #295. Closes ckeditor/ckeditor5#674.

BREAKING CHANGE: Removed `isImageType()` util.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
4 participants