WordPress.org

Make WordPress Core

Opened 11 days ago

Closed 11 days ago

Last modified 11 days 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 11 days 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
11 days 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.


11 days ago

#3 @rebasaurus
11 days ago

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

@rebasaurus
11 days 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
11 days ago

  • Keywords has-patch added; needs-patch removed

#5 @helen
11 days 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
11 days ago

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

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


11 days ago

#9 @helen
11 days 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
11 days ago

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