close
Skip to content

Tests: Add tests for playlist shortcode script loading for first invalid playlist #9228

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Adi-ty wants to merge 6 commits into from
Closed

Tests: Add tests for playlist shortcode script loading for first invalid playlist #9228

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
296e7e8
Tests: Add tests for playlist shortcode script loading
Adi-ty Jul 10, 2025
fd941ff
Tests: Fix class name for playlist shortcode tests
Adi-ty Jul 15, 2025
80b9976
Tests: Update playlist shortcode test to verify scripts load exactly …
Adi-ty Jul 24, 2025
224d33b
fix: PHPCS errors
Adi-ty Jul 24, 2025
8c50b00
fix: Move playlist script loading to the correct position in wp_playl…
Adi-ty Jul 24, 2025
010a9a4
Tests: Refactor script load count verification in wp_playlist_shortco…
Adi-ty Jul 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 15 additions & 12 deletions src/wp-includes/media.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3045,6 +3045,8 @@ function wp_playlist_shortcode( $attr ) {
static $instance = 0;
++$instance;

static $is_loaded = false;
Copy link
Copy Markdown

@justlevine justlevine Jul 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only took a click glance but can we reuse the static $instance, seems to be the equivalent of $instance > 1

I know the stakes are low for a shortcode, but anything to avoid this antipattern leaking into more places.

Copy link
Copy Markdown
Author

@Adi-ty Adi-ty Jul 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @justlevine,
Thanks for the review.

I have tried reusing the static $instance. Using $instance >= 1 can work, but it will trigger the action for every valid playlist after the first shortcode. Using a static $is_loaded variable ensures the action is only triggered once, for the first valid playlist.

Let me know if you have further thoughts!


if ( ! empty( $attr['ids'] ) ) {
// 'ids' is explicitly ordered, unless you specify otherwise.
if ( empty( $attr['orderby'] ) ) {
Expand Down Expand Up @@ -3134,6 +3136,19 @@ function wp_playlist_shortcode( $attr ) {
return '';
}

if ( ! $is_loaded ) {
/**
* Prints and enqueues playlist scripts, styles, and JavaScript templates.
*
* @since 3.9.0
*
* @param string $type Type of playlist. Possible values are 'audio' or 'video'.
* @param string $style The 'theme' for the playlist. Core provides 'light' and 'dark'.
*/
do_action( 'wp_playlist_scripts', $atts['type'], $atts['style'] );
$is_loaded = true;
}

if ( is_feed() ) {
$output = "\n";
foreach ( $attachments as $att_id => $attachment ) {
Expand Down Expand Up @@ -3225,18 +3240,6 @@ function wp_playlist_shortcode( $attr ) {
$safe_style = esc_attr( $atts['style'] );

ob_start();

if ( 1 === $instance ) {
/**
* Prints and enqueues playlist scripts, styles, and JavaScript templates.
*
* @since 3.9.0
*
* @param string $type Type of playlist. Possible values are 'audio' or 'video'.
* @param string $style The 'theme' for the playlist. Core provides 'light' and 'dark'.
*/
do_action( 'wp_playlist_scripts', $atts['type'], $atts['style'] );
}
?>
<div class="wp-playlist wp-<?php echo $safe_type; ?>-playlist wp-playlist-<?php echo $safe_style; ?>">
<?php if ( 'audio' === $atts['type'] ) : ?>
Expand Down
32 changes: 32 additions & 0 deletions tests/phpunit/tests/media/wpPlaylistShortcode.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php
/**
* @group media
* @covers ::wp_playlist_shortcode
*/
class Tests_Media_WpPlaylistShortcode extends WP_UnitTestCase {
Comment thread
Adi-ty marked this conversation as resolved.
Outdated

/**
* @ticket 63583
*/
public function test_should_load_scripts_when_first_playlist_is_invalid() {
$scripts_loaded = false;

add_action(
'wp_playlist_scripts',
function () use ( &$scripts_loaded ) {
$scripts_loaded = true;
}
);

$attachment_id = self::factory()->attachment->create_upload_object(
DIR_TESTDATA . '/uploads/small-audio.mp3'
);

wp_playlist_shortcode( array( 'ids' => '999999' ) );
wp_playlist_shortcode( array( 'ids' => $attachment_id ) );

$this->assertTrue( $scripts_loaded, 'Scripts should load even when first playlist has invalid ID' );

remove_all_actions( 'wp_playlist_scripts' );
}
}
Loading