Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#45676 closed defect (bug) (invalid)

wp_get_attachment_img_srcset() ignores $size, and returns all $image_meta.

Reported by: justlevine's profile justlevine Owned by:
Milestone: Priority: normal
Severity: normal Version: 5.0.1
Component: Media Keywords:
Focuses: Cc:

Description

The expected behavior for calling wp_get_attachment_img_srcset($attachment_id, $size) would be to return an srcset for all images up to and including $size['width'].

Currently, it returns an srcset of all the $sizes registered to the attachment (that are less than $max_srcset_image_width).

This occurs because the $image_meta that is passed to wp_calculate_image_srcset() is just wp_get_attachment_metadata( $attachment_id ), and any sizes larger than that aren't filtered out.

Suggested Fix

The suggested fix is to filter out all the values in $image_meta whose widths are greater than $size['width'].

I'm not comfortable with the coding standards or svn to submit a pr, but here's a possible way to code it: (wp-includes/media.php, line 1020)

if ( ! is_array( $image_meta ) ) { 
  foreach ( $image_meta['sizes'] as $size_name->$meta) {
    if ( $image_meta['width'] > $size['width'] )
      unset( $image_meta['sizes']['size_name'] );
  } 
}

Change History (4)

#1 @SergeyBiryukov
6 years ago

  • Component changed from Post Thumbnails to Media

#2 in reply to: ↑ description ; follow-up: @subrataemfluence
6 years ago

Could this be that are larger than $max_srcset_image_width, if I understand correctly?

Replying to justlevine:

Currently, it returns an srcset of all the $sizes registered to the attachment (that are less than $max_srcset_image_width).

Also

<?php
if ( ! is_array( $image_meta ) ) { 
  foreach ( $image_meta['sizes'] as $size_name->$meta) {
    ...
  } 
}

you are running a foreach loop inside a ! is_array( $image_meta ) check, i.e. trying to iterate an element which is clearly not an array. This will throw an error, since foreach cannot be applied here.

#3 in reply to: ↑ 2 @justlevine
6 years ago

Replying to subrataemfluence:

Could this be that are larger than $max_srcset_image_width, if I understand correctly?

No. It currently takes all the registered images sizes that are less than $max_srcset_image_width. At default, it's all the image sizes less than 1600px. It should return all the image sizes smaller than the registered image name your passing OR the max srcset width.

E.g passing 'full' should return all sizes < $max_srcset_image_width, but passing medium should return all sizes smaller than medium. Currently, both return the same list of all sizes.

you are running a foreach loop inside a ! is_array( $image_meta ) check, i.e. trying to iterate an element which is clearly not an array. This will throw an error, since foreach cannot be applied here.

Good point. Another reason why im not qualified to submit a PR :p.

The intended logic is to test if $image_meta is being passed manually, and only if it is coming from wp_get_attachment_metadata() to filter the sizes.

Logically, if someone is passing an image_size, you'd assume that any array of sizes they're passing manually would be smaller than the declared size, but since the current functionality is to ignore image_size, filtering the manually defined array could break back-compat.

#4 @peterwilsoncc
6 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

@justlevine,

Thanks for the ticket and welcome to trac.

srcset is intended to include images wider than the displayed size to allow for retina and other high density screens. For an image displayed at 300 pixels wide on a 2x display, a 600 pixel wide image is required; 900px for a 3x display. Browsers may take other issues into account when choosing which of the offered image sizes to display.

I'm closing this ticket as invalid, which is trac's unfriendly way of noting the inclusion of these images is by design.

Note: See TracTickets for help on using tickets.