Make WordPress Core

Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#36246 closed defect (bug) (fixed)

wp_get_attachment_image should use wp_get_attachment_metadata

Reported by: JorritSchippers Owned by: wonderboymusic
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.4.2
Component: Media Keywords: blessed
Focuses: Cc:


wp_get_attachment_image uses get_post_meta( $attachment_id, '_wp_attachment_metadata', true ) to fetch attachment meta data, even though the function wp_get_attachment_metadata does exactly this. This means that the filter wp_get_attachment_metadata can't be used.

The function wp_get_attachment_image calls both wp_calculate_image_srcset and wp_calculate_image_sizes with the image meta data. These two functions specify that their inputs come from wp_get_attachment_metadata, but this is not the case in practise.

BTW: media.php contains more situations where get_post_meta( $attachment_id, '_wp_attachment_metadata', true ) could be replaced by wp_get_attachment_metadata($attachment_id).

Attachments (3)

36246-attachment-metadata.patch (3.6 KB) - added by JorritSchippers 6 years ago.
patch with test
36246-attachment-metadata.2.patch (3.8 KB) - added by JorritSchippers 6 years ago.
fix for previous patch
36246.patch (1.3 KB) - added by johnbillion 5 years ago.

Download all attachments as: .zip

Change History (12)

#1 @swissspidy
6 years ago

  • Keywords needs-patch needs-unit-tests added

6 years ago

patch with test

6 years ago

fix for previous patch

#2 @DrewAPicture
6 years ago

  • Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed
  • Milestone changed from Awaiting Review to Future Release

#3 @wonderboymusic
5 years ago

  • Milestone changed from Future Release to 4.7
  • Owner set to wonderboymusic
  • Status changed from new to assigned

#4 @wonderboymusic
5 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 38296:

Media: use wp_get_attachment_metadata() instead of get_post_meta() where appropriate.

Adds unit test.

Props JorritSchippers.
Fixes #36246.

#5 @pento
5 years ago

  • Keywords has-patch has-unit-tests removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

The unit tests for this are failing.

1) Tests_Media::test_wp_get_attachment_image_should_use_wp_get_attachment_metadata
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-<img width="999" height="999" src="http://example.org/wp-content/uploads/2016/03/test-image-testsize-999x999.png" class="attachment-testsize size-testsize" alt="test-image-large.png" srcset="http://example.org/wp-content/uploads/2016/03/test-image-large-150x150.png 150w, http://example.org/wp-content/uploads/2016/03/test-image-testsize-999x999.png 999w" sizes="(max-width: 999px) 100vw, 999px" />
+<img width="999" height="999" src="http://example.org/wp-content/uploads/2016/08/test-image-testsize-999x999.png" class="attachment-testsize size-testsize" alt="test-image-large.png" srcset="http://example.org/wp-content/uploads/2016/08/test-image-testsize-999x999.png 999w, http://example.org/wp-content/uploads/2016/08/test-image-large-150x150.png 150w" sizes="(max-width: 709px) 85vw, (max-width: 909px) 67vw, (max-width: 1362px) 62vw, 840px" />

#6 @wonderboymusic
5 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

#7 @pento
5 years ago

  • Keywords blessed added

5 years ago

#8 @johnbillion
5 years ago

In 38760:

Media: Correct the hostname used in the wp_get_attachment_metadata() test.

See #36246

#9 @johnbillion
5 years ago

Sort-of followup: #38264

Note: See TracTickets for help on using tickets.