Make WordPress Core

Opened 13 months ago

Closed 13 months ago

Last modified 13 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()" — at Version 10

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' ) ); ?>

You will see it fatal.

Change History (11)

#1 @helen
13 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.

13 months ago

#3 @rebasaurus
13 months ago

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

13 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
13 months ago

  • Keywords has-patch added; needs-patch removed

#5 @helen
13 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
13 months ago

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

#7 @Mista-Flo
13 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.

13 months ago

#9 @helen
13 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
13 months ago

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