Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#52196 closed defect (bug) (fixed)

wp_get_attachment_metadata() is broken if no first argument is passed in.

Reported by: cfinke's profile cfinke Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.6.1 Priority: normal
Severity: normal Version: 5.6
Component: Media Keywords: has-patch commit
Focuses: Cc:

Description

In r49084 (for #50679), wp_get_attachment_metadata() was changed to improve performance, but it had the side effect of eliminating the ability to call it with no arguments and have it default to using the global $post.

It should be possible to keep the performance improvements for all cases where an attachment ID is passed in by only calling get_post() when $attachment_id is 0. I'll upload a patch shortly.

Attachments (3)

52196.diff (570 bytes) - added by cfinke 4 years ago.
If no attachment ID is supplied, try to use the global $post's ID.
52196.2.patch (1.0 KB) - added by dilipbheda 4 years ago.
Improve inline Doc.
52196.3.diff (1.0 KB) - added by audrasjb 4 years ago.
patch refreshed against trunk

Download all attachments as: .zip

Change History (15)

@cfinke
4 years ago

If no attachment ID is supplied, try to use the global $post's ID.

#1 @hellofromTonya
4 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 5.6.1

Good catch @cfinke for this regression!

Moving this ticket into the next point release.

#2 @SergeyBiryukov
4 years ago

  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#3 @mukesh27
4 years ago

As per doc @param int $attachment_id Attachment post ID. Defaults to global $post. default value for $attachment_id is global $post but in function use default value to 0.

https://github.com/WordPress/WordPress/blob/master/wp-includes/post.php#L6075

I think we have to change the doc block for that also? like

* @param int $attachment_id Attachment post ID. Default 0.

@dilipbheda
4 years ago

Improve inline Doc.

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


4 years ago

#5 @Mista-Flo
4 years ago

Hi, thanks for your help guys.

I am not sure the last doc update is not confusing.

Because if not attachment_id is passed, it equals to 0 yes, but then it fallbacks to global post with the modified patch, so it should stay like this IMO.

@audrasjb
4 years ago

patch refreshed against trunk

#6 @audrasjb
4 years ago

I refreshed the patch and I ran a quick manual test:

// Used outside the loop.
$data = wp_get_attachment_metadata();
var_export( $data );

// Returns false

#7 @audrasjb
4 years ago

  • Keywords commit added

As per today's bug scrub, let's mark this ticket for commit.

#8 @whyisjake
4 years ago

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

In 50039:

Media: Ensure that wp_get_attachment_metadata can return values from the global $post, if avaiable.

In [49084] (for #50679), wp_get_attachment_metadata() was changed to improve performance, but it had the side effect of eliminating the ability to call it with no arguments and have it default to using the global $post.

This change restores that ability, while keeping the performance improvements from the original change.

Fixes #52196.

Props cfinke, hellofromTonya, mukesh27, dilipbheda, Mista-Flo, audrasjb, SergeyBiryukov, whyisjake.

#9 @whyisjake
4 years ago

In 50040:

Media: Ensure that wp_get_attachment_metadata can return values from the global $post, if avaiable.

In [49084] (for #50679), wp_get_attachment_metadata() was changed to improve performance, but it had the side effect of eliminating the ability to call it with no arguments and have it default to using the global $post.

This change restores that ability, while keeping the performance improvements from the original change.

This changeset brings [50039] to the 5.6 branch.

Fixes #52196.
Props cfinke, hellofromTonya, mukesh27, dilipbheda, Mista-Flo, audrasjb, SergeyBiryukov, whyisjake.

#10 @SergeyBiryukov
4 years ago

In 50051:

Docs: Revert documentation change for wp_get_attachment_metadata().

This more accurately describes the behavior of the function, and is more consistent with the documentation for other post and attachment functions.

Follow-up to [50039].

Props Mista-Flo.
See #52196.

#11 @SergeyBiryukov
4 years ago

In 50052:

Docs: Revert documentation change for wp_get_attachment_metadata().

This more accurately describes the behavior of the function, and is more consistent with the documentation for other post and attachment functions.

Follow-up to [50039].

Props Mista-Flo.
Merges [50051] to the 5.6 branch.
See #52196.

Note: See TracTickets for help on using tickets.