Make WordPress Core

Opened 7 years ago

Last modified 16 months ago

#38906 reopened defect (bug)

wp_get_attachment_image_src() sometimes gives incorrect width and height values

Reported by: pressupinc's profile pressupinc Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Media Keywords: needs-patch 2nd-opinion
Focuses: administration Cc:

Description

The following is an example of a problem that happens to me regularly across multiple sites.

I have an image size registered as follows:

add_image_size( 'featured-home', 1600, 600, true ); // width, height, crop

When I run wp_get_attachment_image_src() as follows:

$image = wp_get_attachment_image_src( $post_id, 'featured-home' );

...and then print_r() the result, I get this:

Array
(
    [0] => http://localhost:8080/lacoastalservices/wp-content/uploads/2016/09/wetlands-1600x600.jpg
    [1] => 1080
    [2] => 405
    [3] => 1
)

The image itself is actually 1600 by 600 pixels wide, but for some reason the width and height values given in the array are "scaled down" to the width of the next largest image size on the site (1080px), and the corresponding image height if it were actually that wide (405px).

Note that WordPress's "large" default image size is still at its default of 1024px, so I don't think that's the problem.

You can hopefully reproduce this by running the "Display All Image Sizes" plugin on a few sites and looking for images whose larger image sizes have a mismatch between their identified dimensions and their actual urls. "Display All Image Sizes" is using wp_get_attachment_image_src() to generate the text strings that describe image sizes, which is how I became aware of this bug.

Attachments (1)

mismatch.png (27.3 KB) - added by pressupinc 7 years ago.

Download all attachments as: .zip

Change History (19)

@pressupinc
7 years ago

#1 follow-up: @joemcgill
7 years ago

  • Keywords reporter-feedback added
  • Owner set to joemcgill
  • Status changed from new to reviewing

Thanks for the report @pressupinc and welcome to Trac!

This appears to be an issue with image_constrain_size_for_editor() being applied at an inappropriate time. Can you confirm that you're experiencing this issue on trunk (nightly versions) or if you're on a stable release (e.g., 4.6.1). If this issue is on nightlies, can you reproduce the same issue on previous releases or is this a change in behavior?

Joe

#2 in reply to: ↑ 1 @pressupinc
7 years ago

Replying to joemcgill:

Thanks for the report @pressupinc and welcome to Trac!

This appears to be an issue with image_constrain_size_for_editor() being applied at an inappropriate time. Can you confirm that you're experiencing this issue on trunk (nightly versions) or if you're on a stable release (e.g., 4.6.1). If this issue is on nightlies, can you reproduce the same issue on previous releases or is this a change in behavior?

Joe

This is on 4.6.1. I first remember noticing it 6 months or so ago, so it's been an issue on various stable releases for at least that amount of time.

#3 @joemcgill
7 years ago

  • Keywords needs-patch added; reporter-feedback removed
  • Milestone changed from Awaiting Review to Future Release
  • Owner joemcgill deleted
  • Version trunk deleted

Thanks for the quick feedback @pressupinc. In that case, I believe this is probably related to #34225 as clarifying (or removing) the content width dependency would ultimately fix this issue as well. Since this doesn't seem to be a regression, I'm going to move it to a future release.

#4 @pressupinc
7 years ago

Just wanted to check in on this—is still curtailing the functionality of Display All Image Sizes. I'd be happy to help fix it with a bit of handholding.

Thank you!

Last edited 7 years ago by pressupinc (previous) (diff)

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


7 years ago

This ticket was mentioned in Slack in #core-media by pressupinc. View the logs.


7 years ago

#7 @Hareesh Pillai
3 years ago

  • Milestone changed from Future Release to 5.9

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


3 years ago

#9 @antpb
3 years ago

  • Milestone changed from 5.9 to Future Release

Hello @hareesh-pillai ! I'm curious what brought this into the 5.9 milestone. Are there any discussions in other components that brought it to light?

Just mainly curious if this is still a valid issue and how the best path to solving it can be achieved by the 5.9 release period. Some items that this ticket could use are reproduction steps and maybe a desired outcome instead of the current behavior. Moving this back to future release until we understand a little more about how it came to be in the 5.9 milestone. If you are owning and working on this, feel free to move it back!

#10 @antonvlasenko
2 years ago

I'm testing this issue.
I will submit a patch If this bug can still be reproduced.

#11 @antonvlasenko
2 years ago

  • Keywords close added; needs-patch removed

Test Report

Testing Environment:

  • Browser: Safari 15.4
  • Theme: Twenty Twenty-Two
  • Server: macOS 12.3, PHP 8.0.18 (native), Apache 2.4.53 (native)
  • WordPress: 6.0-alpha-52448-src

TL;DR

I can't reproduce this bug.
The width of the image is limited by the value of the $content_width global variable.
There is no bug in the wp_get_attachment_image_src function.
More info can be found here: https://codex.wordpress.org/Content_Width

Steps to test

  1. Activate Twenty Twenty-Two theme.
  2. Go to the src/wp-content/themes/twentytwentytwo/functions.php file and add the following code to the very end of the file:
global $content_width;
$content_width = 2000;
add_action( 'after_setup_theme', function() {
	add_image_size( 'featured-home', 1600, 600, true );
} );
  1. Go to the Media Library and upload a test image that's exactly 1600 x 600 in size.
  2. Go to the database and check the last ID in the posts (wp_posts) table.
  3. Add the following code to the very end of the src/wp-content/themes/twentytwentytwo/functions.php file (don't forget to replace the ID):
    $image = wp_get_attachment_image_src( 'replace this value with ID of the last post', 'featured-home' );
    
  4. Check the contents of the $image array (either with print_r or XDebug).

Actual result:

https://cldup.com/_jxDZKLgee.png

Expected result:

It should return an image that is 1600 x 600 in size.
That's exactly what's happening here, meaning there is no bug.

#12 @antonvlasenko
2 years ago

@pressupinc
I'd like to know which WordPress theme you used during the testing, as it seems that $content-width value depends on a particular theme.

#13 @pressupinc
2 years ago

@antonvlasenko This is still an issue that I was running into on a site this past week. However, it's not across the board, image sizes dropdown works properly on some sites.

--

Bug from site last week:

"Large" size is misidentified in the media dropdown (you can also tell that various other sizes are incorrectly stuck at 625px):
https://wpshout.com/large-size.png

Actual large size is 1024 wide:

https://wpshout.com/large-size-actual.png

Media settings are standard:

https://wpshout.com/media-settings.png

Site theme (old client site) is Twenty Twelve.

--

Original site mentioned in the report was built on Understrap.

Last edited 2 years ago by pressupinc (previous) (diff)

#14 @antonvlasenko
2 years ago

Thank you for sharing the info, @pressupinc .
I still believe this is not a bug.
Why?

  1. The current theme limits the width of an image. It’s not a WordPress limitation. The Understrap theme sets the value of the $content_width variable to 640 pixels. The Twenty Twelve theme sets $content_width to be 625 pixels. You can verify it here: https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-content/themes/twentytwelve/functions.php#L29
  2. When you upload an image, WordPress will create thumbnails no wider than the value of the$content_width variable. As far as I understand, this is by design and is needed not to break the layout.
  3. To overcome this limitation, you can manually increase the value of the $content_width variable (at your own risk) and then re-upload the images.
Last edited 2 years ago by antonvlasenko (previous) (diff)

#15 @antonvlasenko
17 months ago

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

It's been 6 months with no follow-up. Closing this ticket with the assumption the issue is not a bug and is expected behaviour.
@pressupinc Please feel free to re-open this ticket if you think that the issue isn't expected behaviour as outlined here.

#16 follow-up: @desrosj
16 months ago

  • Keywords close removed
  • Resolution invalid deleted
  • Status changed from closed to reopened

While I don't disagree that this is likely not truly a bug (listing the size the image will be inserted into the editor at is reasonable), I do think it's quite confusing. Especially when image sizes have names like 2048x2048 and don't match the listed dimensions.

I'd also like to know, does this only surface in "classic" editing contexts? Is this still present when inserting images using the image/media blocks? Content width is handled a bit differently in themes and block because of cover image blocks, etc. Maybe this way of listing image sizes no longer benefits users in more modern WordPress experiences.

I'd love for the media team to have the chance to discuss this a bit more before closing out.

#17 @desrosj
16 months ago

Also noting that #34225 and #35390 are very closely related and these issues should probably be tackled and resolved together.

#18 in reply to: ↑ 16 @azaozz
16 months ago

  • Keywords needs-patch 2nd-opinion added

Replying to desrosj:

While I don't disagree that this is likely not truly a bug (listing the size the image will be inserted into the editor at is reasonable), I do think it's quite confusing.

Yes, it is confusing. This is a UI problem. Thinking that the preferred fix here is to remove the pixel dimensions from the "Size" drop-down, and only leave the "Thumbnail", "Medium", "Large" sizes. The reason is that the pixel sizes give the wrong impression that the selected sub-size will be used or that it matters at all. In reality the browser will determine which size to use from the sizes listed in the srcset attribute.

Also thinking that the two custom sizes named "1536x1536" and "2048x2048" don't belong there. They are virtually unusable in the editor and were added to satisfy the srcset requirements for high-res displays.

I'd also like to know, does this only surface in "classic" editing contexts? Is this still present when inserting images using the image/media blocks?

Only in the classic editor. When selecting an image for the image block, or other blocks that support images, the "Large" size is always used. The user can select to constrain the image dimensions in different ways (dragging, selecting "Medium", etc.), but as mentioned above the browser makes the final decision of how the image will look and which file to load.

I'd love for the media team to have the chance to discuss this a bit more before closing out.

Yea, thinking it would be best to resolve this ticket together with #34225 and #35390 at the same time. Ideally the resolution would include fixing the UI (removing pixel dimensions), and deprecating image_constrain_size_for_editor() which is being misused a lot by themes (was supposed to be only for images displayed in the old editor...).

Also see https://core.trac.wordpress.org/ticket/35390#comment:17.

Last edited 16 months ago by azaozz (previous) (diff)
Note: See TracTickets for help on using tickets.