Make WordPress Core

Opened 17 months ago

Last modified 3 weeks ago

#28634 reopened defect (bug)

Upload images. does not clear Thumbnails metadata (+30kb from camera for each thumbnails)

Reported by: alexufo Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.5
Component: Media Keywords: has-patch needs-docs
Focuses: Cc:



add_image_size( 'thumb100x100', 100, 100, true ); 
add_filter( 'jpeg_quality', create_function( '', 'return 30;' ));

to function.php

upload picture to library.

look at upload folder.


How can will be 30kb file size with visible jpg artifacts?
I look like save twice. With 30% compression and with 100% quality

Attachments (9)

P1020094-100x100-GD.jpg (1.4 KB) - added by DH-Shredder 17 months ago.
P10200944-100x100-Imagick.jpg (30.4 KB) - added by DH-Shredder 17 months ago.
P1020094-noexif.JPG (5.6 MB) - added by DH-Shredder 17 months ago.
P1020094-noexif-100x100-Imagick.jpg (1.0 KB) - added by DH-Shredder 17 months ago.
28634.patch (490 bytes) - added by juliobox 17 months ago.
Strips an image of all profiles and comments striping off the exif information)
28634-2.patch (551 bytes) - added by juliobox 17 months ago.
Same with a filter to keep a retrocompatibility
28634-3.patch (540 bytes) - added by juliobox 17 months ago.
Add a size parameter, filter in resize() instead of load()
28634.diff (660 bytes) - added by wonderboymusic 2 months ago.
28634.2.diff (1.5 KB) - added by adamsilverstein 3 weeks ago.

Change History (41)

#1 @alexufo
17 months ago

  • Severity changed from normal to critical

testing second server, problem available. Perhaps many users are deceived about jpeg compression efficiency in wp. 100x100 size must be about 9kb max! test it save for web in photoshop.

#2 @DH-Shredder
17 months ago

  • Severity changed from critical to normal

I'm not able to reproduce this -- sizes are as expected with both Imagick and GD.

Could you please attach the original image you're using to test (before resizing)?

#4 @DH-Shredder
17 months ago

I took a look, and basically it looks like, from what I can tell, the original image has a lot of metadata.

This is resulting in larger relative file sizes at small resolutions. (Meaning, it's more noticiable)

With Imagick, we maintain metadata. With GD, it gets thrown out due to engine differences.

If you run a diff on the two files, you can see the additional meta.

If you strip the exif before uploading the image, it resizes to under 2kb, even with Imagick.

Attached files to illustrate.

#5 @alexufo
17 months ago

  • Summary changed from jpeg_quality hook fake compress images to Upload images. does not clear Thumbnails metadata (+30kb from camera for each thumbnails)

of course!how i can forgot it.
hm.. i didn't think wp save metadata to thumbnail. is just preview and it must be clear.

Many users can save photograph from camera directly in site. I think it will be good idea clear metadata at least for thumbnail by default. 30 kb is soooo much.
Shortcode gallery consist of these thumbnail images with metadata. 15 photographs have 450kb metadata! Is bad.

Metadata can have thumbnail for thumbnail!

Last edited 17 months ago by alexufo (previous) (diff)

#6 @wonderboymusic
17 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

Needs a solution via patch to move forward

17 months ago

Strips an image of all profiles and comments striping off the exif information)

17 months ago

Same with a filter to keep a retrocompatibility

#7 @alexufo
17 months ago

wp use imagic in core by default? not GD2 extension? Your patch good, but i didnt know about capability.

#8 @juliobox
17 months ago

There is no capability here, just a filter to avoid that since the patch, exif data are stripped down.

For the choice between imagic or GD2, yes :

$implementations = apply_filters( 'wp_image_editors', array( 'WP_Image_Editor_Imagick', 'WP_Image_Editor_GD' ) );

From https://core.trac.wordpress.org/browser/trunk/src/wp-includes/media.php#L2481

#9 @alexufo
17 months ago

sorry, I mean that imagick isnt installed at hosting, filter will not work. GD library have not strip exif function. I prefer imagick or obvious reasons. but how wp community look on functions works only in imagick.

#10 @juliobox
17 months ago

yes, so if you do not have imagick, do not touch this filter?

#11 @markoheijnen
17 months ago

  • Keywords has-patch added; needs-patch removed

I only have one issue with 28634-2.patch and that is the fact that I think the location should be different. I can see myself using the filter but then only for certain image sizes. Like only for thumbnails. Maybe that is given the default editor to much power since it could be done by subclassing the Imagick editor.

#12 @alexufo
17 months ago

perfect logic.) But i think strip exif function should switch on by default. Exif meta needs only for photographers but not in thumbnail for them too. 30kb very much for all wordpress users. All wordpress living no one ask why thumbs have metadata. All wordpress thumbs in web allready have useless meta data in random kb.
Clearing meta in GD it easy by creation new jpg. All cms that we know do not save meta for thumbs.

Last edited 17 months ago by alexufo (previous) (diff)

#13 @juliobox
17 months ago

@alex: You can not strip exif by default since now. Also, my filter is only in the imagick class, so if not installed the filter won't be triggered ;)
Imagine all photographers, already using exif data, maybe from thumbs, you don't know.
Then, after the patch, all is gone!? They have to apply a filter to retreive exif like before?
Not logical! Even if it's logic to strip it for thumbs, you can not do it by default from now, too late.
@marko: new patch added, tested like that, works:

add_filter( 'keep_image_exif', 'exif_if_size', 10, 2 );
function exif_if_size( $value, $size_data ) {
	return $size_data['height'] >= 500 && $size_data['width'] >= 500;

If any of the sizes are < 500 px, the exif data will be stripped.

17 months ago

Add a size parameter, filter in resize() instead of load()

#14 @markoheijnen
17 months ago

That looks great :). Thanks for your work.

I do agree with Juliobox by not stripping the exif data if possible. Not exactly for those reasoning. This due the fact that Imagick isn't installed on a lot of server. I do believe that most of our users don't care a lot about bandwidth. So it all becomes on making a decision what makes the most sense. I like to have it in there because it's content and we should not destroy content.

#15 @juliobox
17 months ago

Remember, this filter can only be used if imagick is installed and in usage. This is not related to GD2 here.

#16 @DH-Shredder
17 months ago

While I have no problem with using a filter or other solution to allow stripping exif, I do not think we should strip exif by default here, since one of the big benefits of Imagick is keeping exif data -- in particular, color profiles.

#17 @alexufo
17 months ago

@DH-Shredder color profiles are not supported official in any browsers. Not all Desktop software can read this in exif.

Guys, we need to do wp better in the details and apply best practice web and seo). when will be time, when we can broke backward capability?)

#18 @juliobox
17 months ago

We can not strip exif by default because of backward capability, even to do a better WP.
I dont think "strip exif" is a best practice or recommanded thing. This is a thing.

#19 @markoheijnen
17 months ago

@alexufo I think we can close this discussion now. We will not strip EXIF data because it's not in the best interest for our users and the web. Browsers do support one way or the other color profiles.

And if you as a user don't need them then you can simply use the filter.

#20 @alexufo
17 months ago

  • Resolution set to invalid
  • Status changed from new to closed

Yes. My mistake. Color managing in new browsers works, last two years i do not check it . I cant see Official articles for users who realy want use icc in web. Instead this Adobe Photoshop add check-box in the save for web mode "convert to sRGB" because today people export own photographs with adobeRGB profile and do not understanding why they little desaturated on some browsers or image viewers.
I all understand. Blame this http://developers.google.com/speed/pagespeed/insights/

Last edited 17 months ago by alexufo (previous) (diff)

#21 @juliobox
17 months ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

Why closed? The filter is a good idea, only the strip by default is not i guess.

#22 @alexufo
17 months ago

Sorry, markoheijnen say close this discussion now.))) Ow.. discussion meant discussion?) no problem. This filter must be live and grow roots.

#23 @johnbillion
16 months ago

  • Version changed from trunk to 3.5

#24 follow-up: @markoheijnen
15 months ago

I never say close :) Also just seeing it now otherwise would have reopened it.
What about renaming the filter to strip_image_exif which is default false.

#25 in reply to: ↑ 24 @alexufo
13 months ago

Replying to markoheijnen:

What about renaming the filter to strip_image_exif which is default false.


#26 @juliobox
3 months ago

Hello guys, what do we need to clode this ticket? Thanks !

#27 @wonderboymusic
2 months ago

  • Keywords needs-docs good-first-bug added
  • Milestone changed from Future Release to 4.4

This ticket was mentioned in Slack in #core by sergey. View the logs.

6 weeks ago

#29 @SergeyBiryukov
6 weeks ago

  • Keywords good-first-bug removed

Not sure if 'keep_image_exif' is a good idea, since it won't work for GD. Should be documented well, otherwise seems like a wontfix.

#30 @alexufo
6 weeks ago

I suggest for GD low level cleaner https://github.com/subabrain/php-jpeg-cleaner/blob/master/jpeg_cleaner_class.php
This is follow jpg standard but but i think it is not tested on big auditory.

#31 @wonderboymusic
3 weeks ago

  • Milestone changed from 4.4 to Future Release

#32 @adamsilverstein
3 weeks ago


  • default to not stripping data
  • rename filter imagick_strip_image_exif
  • add some hook docs
Note: See TracTickets for help on using tickets.