SVG preview in Media Manager #1066
Conversation
|
The inspection completed: No new issues |
| $xml = simplexml_load_file(mediaFN($image, $rev)); | ||
| $attr = $xml->attributes(); | ||
| $w = (int) $attr->width ; | ||
| $h = (int) $attr->height ; |
There was a problem hiding this comment.
The problem here is that width and height could contain arbitrary SVG units. Eg. 105cm or 100% so simply casting them to integer would result in something wrong.
We could argue it's better than nothing though...
There was a problem hiding this comment.
Given the main advantage of SVG is that they are (mostly) size agnostic, shouldn't we just set a max width and a max height. I'm not even sure I set the width and height of my SVGs, just the viewbox.
There was a problem hiding this comment.
Well, I assume most users will not configure the attributes themselves. The svgpureinsert plugin works around this problem by reconfiguring the SVGs to use 100% for width and height and setting an appropriate aspect ratio view box.
But yes, simply setting the a max-width and max-height and then including the SVG as is without specifying any dimensions would probably (mostly) work.
|
The whole media handling in DokuWiki is suboptimal and needs a major rewrite. That's out of the scope of this PR though.
|
|
If the "pain" is minimal enough, I would vote for merging this. I think it's better to have some kind of SVG support (even in a sub optimal way) than to have none. It only depends on how much pain gets introduced and how much pain this could potentially create later when/if someone rewrites the media system. That seems to be not much in this case, right? |
|
There is now improved svg support, see the pull requests referred from #1094 . There are here some comments, which needs to be addressed. |
Minimal support of SVG files in Media Manager