WordPress.org

Make WordPress Core

Opened 10 months ago

Closed 10 months ago

Last modified 10 months ago

#51791 closed defect (bug) (fixed)

When wp_prepare_attachment_for_js() is called too early, a fatal occurs "Call to undefined function get_media_states()"

Reported by: rebasaurus Owned by: helen
Milestone: 5.6 Priority: normal
Severity: major Version: 5.6
Component: Media Keywords: has-patch commit
Focuses: Cc:

Description (last modified by SergeyBiryukov)

When we call wp_prepare_attachment_for_js() too early, we see the fatal:

Call to undefined function get_media_states()

Introduced in [49223] from #42063.

To test in twentytwenty:

--- a/themes/twentytwenty/header.php
+++ b/themes/twentytwenty/header.php
@@ -26,6 +26,7 @@

        <body <?php body_class(); ?>>

+               <?php var_dump( wp_prepare_attachment_for_js( '28' ) ); ?>
                <?php
                wp_body_open();
                ?>

You will see it fatal.

Attachments (1)

51792.patch (716 bytes) - added by rebasaurus 10 months ago.
Since I see that we check for the existence of get_compat_media_markup above, I think we can do the same behavior here?

Download all attachments as: .zip

Change History (11)

#1 @helen
10 months ago

  • Milestone changed from Awaiting Review to 5.6

Just for reference, what's the use case for calling this in such a context (JS front end, something else)? This will help make sure we make the right fix and don't break it again in the future.

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


10 months ago

#3 @rebasaurus
10 months ago

@helen Correct, specifically, we've seen usages for it in shortcodes, template tags, etc.

@rebasaurus
10 months ago

Since I see that we check for the existence of get_compat_media_markup above, I think we can do the same behavior here?

#4 @Mista-Flo
10 months ago

  • Keywords has-patch added; needs-patch removed

#5 @helen
10 months ago

  • Keywords commit added

Yeah I think it makes sense to just do it the same as get_compat_media_markup given the context. More broadly I kind of feel like it should actually be filtered in closer to usage time since it's just a string and not super useful without more futzing outside of the specific admin usage but that's probably too much to be messing with for not a ton of benefit.

#6 @antpb
10 months ago

Just tested this and it's working after patch with the example code given to fatal.

#7 @Mista-Flo
10 months ago

I can reproduce and the fix works as expected, thanks for catching it @rebasaurus

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


10 months ago

#9 @helen
10 months ago

  • Owner set to helen
  • Resolution set to fixed
  • Status changed from new to closed

In 49613:

Media: Avoid fatal error in `wp_prepare_attachment_for_js().

In certain contexts, in particular on the front-end, get_media_states() is not availble.

Props rebasaurus.
Fixes #51791.

#10 @SergeyBiryukov
10 months ago

  • Description modified (diff)
Note: See TracTickets for help on using tickets.