Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom sized images using thumbnail images for srcset #216

Open
TedAvery opened this issue Oct 16, 2015 · 16 comments
Open

Custom sized images using thumbnail images for srcset #216

TedAvery opened this issue Oct 16, 2015 · 16 comments
Assignees
Labels

Comments

@TedAvery
Copy link

In a recent update to the RICG plugin (very recent, just started seeing this in the past few days), an image embedded in a post with a custom size has it's srcset populated by the thumbnail versions of an image rather than properly scaled versions of the original image.

Here's an example.

Scroll down a bit to the shoes section. The second image (yellow shoes) was embedded with custom dimensions of 220x148, and the original image is 300x162 (here). But the image displayed from the srcset is a cropped 1:1 ratio thumbnail version of the image (I'm served a 600x600 version here)

There are many more examples on this blog post when you scroll down (tops, bottoms, electronics, etc.) Previously the srcset contained only images at the same ratio as the original image.

@jaspermdegroot
Copy link
Member

@TedAvery - Thanks for reporting the issue.

@joemcgill - This is a probably a regression from c3057fb.
If image_get_intermediate_size can't find a match it returns the nearest image size larger than the specified size, ignoring aspect ratio.

@jaspermdegroot
Copy link
Member

I was just looking at the code of image_get_intermediate_size and noticed that it does check the aspect ratio. So I am not sure anymore if that change is the cause. Have to do some testing.

@joemcgill
Copy link
Member

Hi @TedAvery. Thanks for reporting the issue. Can you tell us what version of the RICG plugin you currently have installed and paste the HTML of one of the affected images as it appears in the content editor?

@TedAvery
Copy link
Author

@joemcgill Version 2.5.2. Example code below. Thanks!

<img class="alignnone wp-image-9913 " src="http://thriftynomads.com/wp-content/uploads/2015/08/71Mkv-k6gfL._UL1500_-300x162.jpg" alt="Fils Black and Yellow Shoes" width="220" height="148" />

@joemcgill
Copy link
Member

Thanks @TedAvery. One more favor—can you tell me what custom image sizes are available in your install and if they're hard or soft crops?

@mundschenk-at
Copy link

Since I've encountered a similar issue yesterday with custom responsive image code (functionally equivalent, but genealogically unrelated to the plugin), is that 600 x 600 image by any chance the actual thumbnail size in your WordPress site, @TedAvery? If so, the bug happens because of line 632 in image_get_intermediate_size. The code checks if the currently evaluated size is name thumbnail and if so, it uses the size even when the aspect ratio is wrong.

There is no possible fix (except using a custom function that does not treat thumbnail as special) in WordPress 4.3, but there was a recent commit in 4.4 that add's a filter to the function, so the thumbnail could be excluded there. Not very good efficiency-wise, though.

Addendum: I've filed a ticket on Trac, so you might want to chime in there.

Edit: Clarified a sentence.

@jaspermdegroot
Copy link
Member

I noticed the exception for the image size with the name "thumbnail" in the ratio check and I agree that this is odd. I have no idea why this exception was made, but maybe there is a good reason for it.

By default the thumbnail size is 150x150 but maybe @TedAvery changed it to 600x600 with set_post_thumbnail_size. We need to know what custom image sizes (and their names) he uses to be sure this is the cause of the problem.

@mundschenk-at
Copy link

I think the exception is purely historical, as once upon a time there were only the thumbnail and the full image. Since WP on its own does not (re-)generate missing image sizes, the thumbnail was used as a last resort.

@TedAvery
Copy link
Author

TedAvery commented Nov 4, 2015

@joemcgill Sorry for the delay. Stupid question, but how do I get the info you're requesting? I do know that the theme I am using requires you to run the "Force regenerate thumbnails" plugin when you first run it in order to ensure your images are compatible with the theme. So your suspicions about custom image sizes is most likely correct.

For now, on the example post I mentioned, we've removed the custom sizing entirely, just linking to "Medium" size images which doesn't seem to have the cropping issue.

@mundschenk-at
Copy link

@TedAvery You can look for update_option( 'thumbnail_size_w' ... in your theme files (could be add_image_size, but I don't think that works with the reserved names).

Addendum: Maybe easier to check: Settings > Media :-)

@TedAvery
Copy link
Author

TedAvery commented Nov 4, 2015

@mundschenk-at Thanks. I found a few options for add_image_size in the theme's functions.php, hopefully this helps.

add_image_size( 'cb-100-65', 100, 65, true ); // Widgets
add_image_size( 'cb-260-170', 260, 170, true ); // Megamenu
add_image_size( 'cb-360-490', 360, 490, true ); // Portrait thumbnails
add_image_size( 'cb-360-240', 360, 240, true ); // Blog Style A/Mega menu
add_image_size( 'cb-378-300', 378, 300, true ); // Slider C, Grid small thumbnails
add_image_size( 'cb-759-300', 759, 300, true ); // Grid Medium thumbnails, Grid 3 Static Big Thumbnail
add_image_size( 'cb-759-500', 759, 500, true ); // Slider B, Standard featured image, Blog Style D/F/G, Module D
add_image_size( 'cb-759-600', 759, 600, true ); // Grid big thumbnails
add_image_size( 'cb-1400-600', 1400, 600, true ); // Parallax/Full screen/Full screen slider

@mundschenk-at
Copy link

@TedAvery Those are custom sizes, try to look for "thumbnail". Or better, check your Settings > Media page.

@jaspermdegroot
Copy link
Member

@TedAvery

Thanks for the information. It would indeed be very helpful if you could also let us know what the dimensions of "thumbnail size", "medium size", and "large size" are on your media settings page. Thanks for your help!

@TedAvery
Copy link
Author

@jaspermdegroot Gah, sorry for another long silence!
OK, thumbnail size is 150x150 with "Crop thumbnail to exact dimensions" checked
Medium is 300x300
Large is 1024x1024

@mundschenk-at
Copy link

@TedAvery Do or did you ever regenerate your image variations (with a plugin like, well, Regenerate Thumbnails)? If not, the 600 x 600 image could still be an old size for thumbnail because by default uses whatever size was active when you uploaded the image. (I still think this might be the case because 600 x 600 cropped is not anywhere on the list of images sizes you have told us about.)

Could you look in the database for the actual metadata for that particular image?

PS: Did you change something on your site? The 600 x 600 image seems to be gone from the srcset.

@jaspermdegroot
Copy link
Member

@mundschenk-at - Thanks for helping out here!

It looks like 600x600 is the 'post-thumbnail' size indeed. Maybe @TedAvery can confirm by looking for set_post_thumbnail_size( 600, 600, true ); in his functions.php.

This ticket is from before version 3.0.0 of the plugin. Since that version image_get_intermediate_size() is not used anymore to add a srcset attribute to images in the content or post thumbnails. So if that was indeed the cause, the problem should be gone by now.

However, we still do call image_get_intermediate_size() indirectly when you use the new function wp_get_attachment_image_srcset() in your templates. So this is still something we need to test. Also to check if image_get_intermediate_size() is indeed the cause.

@jaspermdegroot jaspermdegroot self-assigned this Dec 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants