backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 40 forks source link

Follow up: Complement SVG image support in image library and elsewhere #6497

Open indigoxela opened 6 months ago

indigoxela commented 6 months ago

Description of the task

As of #5541 core now supports using SVG files as image. :rocket:

Some details are still pending discussion and implementation. Im collecting here, what comes to my mind. It might be, that we split this into several issues. We'll see.

Additional information

indigoxela commented 6 months ago

Here's a PR to consider viewBox as (additional) fallback. That seemed to be the easiest of the four tasks.

Handy to test: our brand new iconset in core (/core/misc/icons), as none of them has width/height, but all of them have viewBox set. :wink:

Edit: PR removed (as not planned)

indigoxela commented 6 months ago

Interesting finding, why image_get_info doesn't work for newly uploaded svg files:

The first check is to ask the image toolkit, if this is something usable - SVG is not, so there's a second check. That check, down the road, relies on the file extension. And what we have at that point of uploading is something like /tmp/phpABCXYZ without any extension.

I belief, that's a bug. @docwilmot what do you think? Or was it @herbdool, who struggled with that?

I guess, this is relevant for CKEditor, as it uses file_validate_is_image() as upload validator for drag-and-drop upload.

herbdool commented 6 months ago

Regarding viewBox, I'm not certain yet if we want to use it for setting the width and height as a fallback. The aspect ratio I can understand, but I'm looking at https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/viewBox and trying to wrap my head around the three example svg with appear the same size but have different viewBox's. The viewBox could be dramatically different, depending on the relationship to the sizes and units of the paths and shapes within it. At least that's how I understand it. Do we want one to be set at width="10" and another at width="100"?

indigoxela commented 6 months ago

@herbdool re viewBox: I absolutely understand, that you hesitate. Things look even more complicated, when switching to the official spec. We could postpone that viewBox parsing, until someone comes up with a request...

Any opinion re the other three subtasks?

herbdool commented 6 months ago

Sounds like the ckeditor drag and drop is important enough to be it's own issue. I don't feel strongly about the others.

I'm guessing pre existing SVG files aren't a big issue. If people were actively uploading bunches of them they were probably installing the svg_image module already.