@joelnewcomer Thanks for the report. I’ve not been able to reproduce this issue myself so wondering if you have any more details on how you’re using SVGs?
The fix you mention was to fix an issue with Gutenberg when using SVGs. In essence, we were setting the height and width of SVGs to be false in one of the image filters. This was a bug on our part as that filter expects the values of height and width to be an integer. We’ve changed that in the recent release to use the actual calculated sizes that WordPress generates and I’m guessing this is causing these SVGs to be larger for you.
But in my tests, using both the classic editor and the block editor, I don’t see any differences in the image markup that is output either before or after this fix. Curious if you’re using the block editor or classic editor here? Or if you’re doing some other integration to output the images? If possible, would also be nice to see the actual image markup you’re getting both before and after this change, to compare if there’s differences there.
@dkotter It could be something to do with the Enfold theme (Avia Layout Builder). With version 1.9.9 of Safe SVG, the img tag looks like this:
<img width="23" height="27" class="wp-image-40853 avia-img-lazy-loading-not-40853 avia_image" src="https://ninjastage1.wpengine.com/wp-content/uploads/2020/10/security-icon.svg" alt="security icon" title="security-icon" itemprop="thumbnailUrl">
After updating to 1.9.10 it looks like this:
<img class="wp-image-40853 avia-img-lazy-loading-not-40853 avia_image" src="https://ninjastage1.wpengine.com/wp-content/uploads/2020/10/security-icon.svg" alt="security icon" title="security-icon" height="1030" width="1030" itemprop="thumbnailUrl" data-srcset="https://ninjastage1.wpengine.com/wp-content/uploads//2020/10/security-icon.svg 80w, https://ninjastage1.wpengine.com/wp-content/uploads//2020/10/security-icon.svg 300w, https://ninjastage1.wpengine.com/wp-content/uploads//2020/10/security-icon.svg 1030w, https://ninjastage1.wpengine.com/wp-content/uploads//2020/10/security-icon.svg 1536w, https://ninjastage1.wpengine.com/wp-content/uploads//2020/10/security-icon.svg 2048w, https://ninjastage1.wpengine.com/wp-content/uploads//2020/10/security-icon.svg 36w, https://ninjastage1.wpengine.com/wp-content/uploads//2020/10/security-icon.svg 180w, https://ninjastage1.wpengine.com/wp-content/uploads//2020/10/security-icon.svg 1500w, https://ninjastage1.wpengine.com/wp-content/uploads//2020/10/security-icon.svg 705w, https://ninjastage1.wpengine.com/wp-content/uploads//2020/10/security-icon.svg 845w" data-sizes="(max-width: 1030px) 100vw, 1030px">
Notice the height and width issue.
-
This reply was modified 2 years, 5 months ago by
joelnewcomer.
Same here with Total Theme and WKBakery Page Builder. It seems values in viewbox are now used. Before width and height.
@joelnewcomer Thanks for the extra information. Definitely see the change in the height and width of that image tag which will cause this issue. I’ve still not been able to replicate myself. I don’t have access to the Enfold theme, do you happen to know how it’s outputting that image tag?
@tkrieger Same question for you, I don’t have access to the Total Theme but wondering if you know how they are outputting these SVG image tags? Using a core function or something custom?
@joelnewcomer @tkrieger Okay, I’ve looked into this some more and I think I’ve found the issue. I had assumed it was a problem with the fix mentioned originally (
Use calculated size for SVGs instead of using false
), but couldn’t reproduce any issues with that change.
I looked through the other things in the release and there was another fix put in place to ensure we use the height and width parameters of the SVG properly (can see that fix here: https://github.com/10up/safe-svg/pull/19/files#diff-05bf19b873cfb6358cd7c239366db6675635bf4b03a4b56353102e359d5c2859R429).
Safe SVG has logic to get the height and width of an SVG from the height and width attributes. If those don’t exist, we then try to get them from the viewbox attribute. If that also doesn’t exist, we default to just using 100×100. There was a bug in this logic though that would almost always disregard the height and width attributes, so the viewbox attribute would always be used (but if you didn’t have a viewbox set, you’d default to 100×100, which was reported as a bug).
This is now fixed so we properly use the height and width first and then fallback to using viewbox if needed. My assumption here is the values in those attributes must be different in your case. If you investigate the SVGs you’re using and look for those attributes, my guess is they are different.
@joelnewcomer From your example, I’d guess the SVG has a viewbox with a height of 27 and width of 23 but the actual height and width attributes must both be set to 1030, resulting in the larger image size showing up now.
So this is one of those unfortunate side effects where a bug needed to be fixed but for SVGs that have different values in those attributes, this could cause size issues now. We could change the logic in the plugin to use the viewbox first and then height and width, which should support any existing SVGs but not sure that’s the right approach going forward. You could also fix the size values your self in the SVG and re-upload it, though I understand that’s not ideal either
Thank you for your fast response! Yes, this values are different, e.g.
height="100px" width="100px" and viewBox="0 0 10 10"
This results in a very small image with the actual version of safe svg.
I think it’s the new logic in the one_pixel_fix function the one that applies a default 100x100px to the image. When those attributes were set to null in the previous version there was no issue. I don’t know if the standards changed and that’s the reason this function was rewritten, but in the meantime the solution seems to downgrade to the previous version.
FYI: I’m having the same trouble. Funny thing is that the height and width are being outputted twice. The first iteration occurs at the beginning of the IMG tag, and the last one at the end. The one at the beginning is the 100x100px, and the last one is the correct one. Sadly the browser takes the first parameters occurrence and renders that one, not the last ones.
@dkotter same here, the image tag output turned from
SafeSVG 1.9.9
<img src="https://example.com/wp-content/uploads/2020/09/image.svg" class="attachment-medium size-medium entered lazyloaded" alt="" height="36.984" width="223.756" data-lazy-src="https://example.com/wp-content/uploads/2020/09/image.svg" data-ll-status="loaded">
into
SafeSVG 1.9.10
<img width="300" height="300" src="https://example.com/wp-content/uploads/2020/09/image.svg" class="attachment-medium size-medium entered lazyloaded" alt="" data-lazy-srcset="https://example.com/wp-content/uploads//2020/09/image.svg 150w, https://example.com/wp-content/uploads//2020/09/image.svg 300w, https://example.com/wp-content/uploads//2020/09/image.svg 1024w, https://example.com/wp-content/uploads//2020/09/image.svg 1536w, https://example.com/wp-content/uploads//2020/09/image.svg 2048w, https://example.com/wp-content/uploads//2020/09/image.svg 100w" data-lazy-sizes="(max-width: 300px) 100vw, 300px" data-lazy-src="https://example.com/wp-content/uploads//2020/09/image.svg" data-ll-status="loaded" sizes="(max-width: 300px) 100vw, 300px" srcset="https://example.com/wp-content/uploads//2020/09/image.svg 150w, https://example.com/wp-content/uploads//2020/09/image.svg 300w, https://example.com/wp-content/uploads//2020/09/image.svg 1024w, https://example.com/wp-content/uploads//2020/09/image.svg 1536w, https://example.com/wp-content/uploads//2020/09/image.svg 2048w, https://example.com/wp-content/uploads//2020/09/image.svg 100w">
The output is made with <?php echo wp_get_attachment_image( $attachment_id, 'medium' ); ?> in a custom theme.
Also, please notice double slash in srcset URLs (the regular src attribute is correct).
The original SVG image is:
<?xml version="1.0" encoding="UTF-8"?><svg xmlns="http://www.w3.org/2000/svg" width="223.756" height="36.984" viewBox="0 0 223.756 36.984"> ... </svg>
Looks like image attributes are not respected after the update when displaying medium image size: we just have default 300×300.
Same here with theme from scratch using plain wp_get_attachment_image($image_id, 'custom_image_size', false).
For the following SVG the output has changed.
<svg xmlns="http://www.w3.org/2000/svg" width="68" height="70" viewBox="0 0 68 70"></svg>
Using 1.9.9 this outputs <img src="example-path/file.svg" loading="lazy" height="70" width="68" />
Using 1.9.10 this outputs <img width="1024" height="9999" src="example-path/file.svg" />
Where 1024 and 9999 correspond with add_image_size( 'custom_image_size', 1024, 9999 );.
I see the order of attributes also has changed, the width/height attributes are now before src and loading attributes (which might help to identify the problem).
Let me know if you need more information to reproduce this issue.
-
This reply was modified 2 years, 5 months ago by
pimschaaf.
-
This reply was modified 2 years, 5 months ago by pimschaaf.
-
This reply was modified 2 years, 5 months ago by pimschaaf.
Thanks all for the detailed reports, this is super helpful. We have a PR up to revert these specific changes, if anyone wants to test and see if this fixes your issues: https://github.com/10up/safe-svg/pull/41.
We will need to come back to these, as these were legitimate bugs that needed fixed, but we’ll want to find a better approach here to fix these and still maintain backwards compatibility
@dkotter thanks for the quick follow-up. I’ve tested the PR locally and found it to fix the issue I have posted above.
Thanks, this post saved me probably hours of troubleshooting :).
After upgrading, some of my SVGs were coming out at 100×100.
I notice they do not have an explicit width or height set, just viewBox="0 0 1000 400"
Applying the fix from PR41 worked for me.
With wpscan.com now reporting security issues in <1.9.10 it is becoming an urgent issue for me that no upgrade is available with the above fixed.
Public proof of concept for that security issue is landing in 8 days..
See https://wpscan.com/plugin/safe-svg
It appears this has been fixed in Safe SVG 2.0.0
