WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 3 years ago

Last modified 14 months ago

#26256 closed defect (bug) (fixed)

SVG images get width and height attributes with values of 1

Reported by: lippe Owned by: wonderboymusic
Milestone: 4.1 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch needs-testing
Focuses: Cc:

Description

Even though SVG files are not supported by core, it's rather easy (and might be common?) to allow them to be uploaded by filtering the 'mime_types' hook or using the ALLOW_UNFILTERED_UPLOADS.

However, on some places throughout WP admin, uploaded SVG images aren't displayed correctly due to width and height attributes being set to 1 for the <img> tag. The places I've found this error so far is in the Media admin table and the Featured Image metabox.

Removing the width and height attributes (using the Chrome inspector) displays the images correctly, as far as I can tell.

Attachments (2)

svg-featured-image.patch (763.6 KB) - added by arippberger 4 years ago.
Fixes CSS to allow 1x1 featured image SVGs to fill their containers
svg-featured-image.2.patch (599 bytes) - added by arippberger 4 years ago.
CSS changes to help show SVG files in admin - minified files removed

Download all attachments as: .zip

Change History (18)

#1 follow-up: @lippe
4 years ago

Since the SVG:s don't seem likely to be supported by Core anytime soon, some fix specifically for SVG seems unnecessary.

Allowing for some filtering of the width and height attributes maybe would be a more general possible solution.

#3 @lippe
4 years ago

A possible workaround for this is to filter 'image_downsize' and for SVG files return null values for width and height:

/**
 * Removes the width and height attributes of <img> tags for SVG
 * 
 * Without this filter, the width and height are set to "1" since
 * WordPress core can't seem to figure out an SVG file's dimensions.
 * 
 * For SVG:s, returns an array with file url, width and height set 
 * to null, and false for 'is_intermediate'.
 * 
 * @wp-hook image_downsize
 * @param mixed $out Value to be filtered
 * @param int $id Attachment ID for image.
 * @return bool|array False if not in admin or not SVG. Array otherwise.
 */
function wg_fix_svg_size_attributes( $out, $id )
{
	$image_url  = wp_get_attachment_url( $id );
	$file_ext   = pathinfo( $image_url, PATHINFO_EXTENSION );

	if ( ! is_admin() || 'svg' !== $file_ext )
	{
		return false;
	}

	return array( $image_url, null, null, false );
}
add_filter( 'image_downsize', 'wg_fix_svg_size_attributes', 10, 3 );
Version 0, edited 4 years ago by lippe (next)

#4 in reply to: ↑ 1 ; follow-up: @nacin
4 years ago

Replying to lippe:

Since the SVG:s don't seem likely to be supported by Core anytime soon, some fix specifically for SVG seems unnecessary.

Allowing for some filtering of the width and height attributes maybe would be a more general possible solution.

They'll never be supported for security reasons, as far as I can see. Making it so, if you have unfiltered uploads, things work OK is a good idea.

#5 @SergeyBiryukov
4 years ago

#27285 was marked as a duplicate.

#6 @arippberger
4 years ago

There is a simple CSS fix to get SVG images set to 1x1 to fill their container. Here is the CSS code to fix the issue for the admin featured image section:

#postimagediv .inside img {
   max-width:100%;
   height:auto;
   width:auto; /*this line has been added*/
}

Adding "width:auto" lets the SVG fill its container in Chrome (webkit) and Firefox. IE 9-11 don't seem to have an issue with displaying the 1x1 SVG without the CSS addition.

I've attached a patch which should fix this issue (updates CSS of "#postimagediv .inside img").

@arippberger
4 years ago

Fixes CSS to allow 1x1 featured image SVGs to fill their containers

#7 @arippberger
4 years ago

#26460 was marked as a duplicate.

#8 @arippberger
4 years ago

  • Keywords has-patch needs-testing added
  • Summary changed from SVG images get width and height attributes with values 1 to SVG images get width and height attributes with values of 1

#9 follow-up: @SergeyBiryukov
4 years ago

No need to patch minified files, a post-commit task takes care of that.

There's a constant you can add to your wp-config.php file to use unminified files:
http://codex.wordpress.org/Debugging_in_WordPress#SCRIPT_DEBUG

#10 in reply to: ↑ 9 @arippberger
4 years ago

Replying to SergeyBiryukov:

No need to patch minified files, a post-commit task takes care of that.

There's a constant you can add to your wp-config.php file to use unminified files:
http://codex.wordpress.org/Debugging_in_WordPress#SCRIPT_DEBUG

Thanks Sergey. I'll add a new patch without the minified files.

@arippberger
4 years ago

CSS changes to help show SVG files in admin - minified files removed

#11 in reply to: ↑ 4 ; follow-up: @ericlewis
3 years ago

Replying to nacin:

They'll never be supported for security reasons, as far as I can see.

What are the security worries here?

#12 in reply to: ↑ 11 ; follow-up: @johnbillion
3 years ago

Replying to ericlewis:

What are the security worries here?

The main issue is XXE attacks, but there are others such as recursive entity expansion bombs.

#13 in reply to: ↑ 12 @ericlewis
3 years ago

Replying to johnbillion:

The main issue is XXE attacks, but there are others such as recursive entity expansion bombs.

Isn't unfiltered html just as dangerous? If a user can be trusted with unfiltered html, perhaps they should also be trusted with SVG?

#14 @wonderboymusic
3 years ago

  • Owner set to wonderboymusic
  • Resolution set to fixed
  • Status changed from new to closed

In 29832:

In the admin, give media list table icons auto for width and height. In the post thumbnail metabox, apply width: auto.

If someone turns on support SVG files, this will allow them to show up.

Props arippberger for an initial patch.
Fixes #26256.

#15 @DrewAPicture
3 years ago

  • Milestone changed from Awaiting Review to 4.1

#16 @isaumya
14 months ago

+1 to official SVG support. :)

Note: See TracTickets for help on using tickets.