Make WordPress Core

Opened 5 weeks ago

Last modified 10 days ago

#63583 new defect (bug)

Issue when having multiple Audio and/or Video playlists at the same page

Reported by: guido07111975's profile Guido07111975 Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 6.8
Component: Embeds Keywords: has-test-info has-screenshots has-patch has-unit-tests
Focuses: Cc:

Description

When having multiple audio and/or video playlists (shortcodes) on the same page (or a combination), removing the audio/video file of the first playlist (via media library or by changing the ids ID), will break all playlists on that page in the frontend.

Example:

Playlist 1:
[playlist ids="230"]

Playlist 2:
[playlist ids="230"]

Playlist 3:
[playlist ids="230"]

Quick test: change ID of the first playlist, save page, visit the frontend.

Attachments (2)

Playlist Before.jpg (78.7 KB) - added by Guido07111975 5 weeks ago.
Playlist Before
Playlist After.jpg (32.6 KB) - added by Guido07111975 5 weeks ago.
Playlist After

Download all attachments as: .zip

Change History (13)

@Guido07111975
5 weeks ago

Playlist Before

@Guido07111975
5 weeks ago

Playlist After

#1 @rollybueno
5 weeks ago

  • Keywords needs-patch has-test-info has-screenshots added

Reproduction Report

Description

This report validates whether the issue can be reproduced.

When multiple [playlist] shortcodes are present on a single page, if the first shortcode contains an invalid or non-existent media ID, it causes all subsequent playlists (even those with valid IDs) to fail rendering or break on the frontend. The failure seems tied to how the JavaScript initialization for playlists is handled based on the first instance.

Environment

  • WordPress: 6.8.1
  • PHP: 8.3.17
  • Server: nginx/1.26.1
  • Database: mysqli (Server: 8.0.16 / Client: mysqlnd 8.3.17)
  • Browser: Chrome 136.0.0.0
  • OS: Linux
  • Theme: Twenty Twenty-Four 1.3
  • MU Plugins: None activated
  • Plugins:
    • Query Monitor 3.18.0
    • Test Reports 1.2.0

Actual Results

✅ Error condition occurs (reproduced).

Steps:

  • Used [playlist ids="121212121212121"] as the first shortcode (invalid ID) - make sure this is in fact invalid or not existed in your environment!
  • Followed by two [playlist ids="10"] shortcodes (valid media ID) - find any free media(audio) and upload it, make sure you replace ids to your own media id
  • On the frontend, none of the playlists render

Additional Notes

  • The issue does not occur if the first [playlist] contains a valid ID.
  • Inspecting the browser console shows no explicit JS errors, but the markup or player initialization script is incomplete or missing.
  • The issue may stem from how wp-playlist.js expects valid data from the first rendered playlist block.
  • Behavior consistent across audio and video playlists.

Supplemental Artifacts

Before: https://i.imgur.com/gepoUDG.png
After: https://i.imgur.com/P9px00t.png

Last edited 5 weeks ago by rollybueno (previous) (diff)

This ticket was mentioned in PR #9003 on WordPress/wordpress-develop by @vipulgupta003.


5 weeks ago
#2

  • Keywords has-patch added; needs-patch removed

#3 follow-up: @hbhalodia
5 weeks ago

Hi @rollybueno @Guido07111975, Looks like the we are only checking for the first instance to load the intended script, which is ideally not correct because, before loading the script based on first instance, we are checking for empty attachment, and returning early, so it never reaches the action which loads the script, now on subsequent playlist, since if we find the attachment, we proceed, but the chcek related to first instance would fail and hence it won't load the script. So we need to update it with proper check instead of first instance check.

Thank You,

#4 in reply to: ↑ 3 @rollybueno
5 weeks ago

  • Keywords needs-testing added

Yes, correct. We have patch 1 minute before your comment, so interesting to see if that's the case. Adding proper keywords, I'll check if there's existing test unit.

Replying to hbhalodia:

Hi @rollybueno @Guido07111975, Looks like the we are only checking for the first instance to load the intended script, which is ideally not correct because, before loading the script based on first instance, we are checking for empty attachment, and returning early, so it never reaches the action which loads the script, now on subsequent playlist, since if we find the attachment, we proceed, but the chcek related to first instance would fail and hence it won't load the script. So we need to update it with proper check instead of first instance check.

Thank You,

Last edited 5 weeks ago by rollybueno (previous) (diff)

#5 @rollybueno
4 weeks ago

Test Report

Description

This report validates whether the indicated patch works as expected.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/9003

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.2.28
  • Server: nginx/1.27.5
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
  • Browser: Chrome 136.0.0.0
  • OS: Linux
  • Theme: Twenty Twenty-Five 1.2
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Actual Results

  1. ✅ Issue resolved with patch.

Additional Notes

  • I was able to reproduce the issue again on the nightly version, consistent with my previous report in https://core.trac.wordpress.org/ticket/63583#comment:1.
  • Patch was applied using the following steps:
    • git fetch upstream pull/9003/head:pr-9003
    • git checkout pr-9003
  • I had to perform a hard restart of Docker to ensure the patch was properly applied in the testing environment, using the following steps.:
    1. npm run env:stop
      
    2. Close the Docker Desktop app and re-open
    3. npm run env:start
      
    4. Refresh the playlist page
  • After applying the patch, the issue no longer occurs.

Supplemental Artifacts

▶️ Primary Screencast (Recommended | Full Walkthrough): See the patch in action, including reproduction and resolution steps. https://drive.google.com/file/d/1HXbZCSuv7RETaLr2bnWXdin7H1bBeDs2/view?usp=sharing

Before checkout to pr-9003:
https://i.imgur.com/uxdnAQK.png

After checking out to pr-9003:
https://i.imgur.com/KtoC7DM.png

#6 follow-up: @rollybueno
4 weeks ago

  • Keywords needs-unit-tests added

I'll leave the needs testing keyword open so that other contributors can test as well, though I'll add needs-unit-tests since I can't find any related to this issue. Feel free to remove if you think otherwise.

Last edited 4 weeks ago by rollybueno (previous) (diff)

#7 @Guido07111975
4 weeks ago

Fix in #9003 looks good!

#8 in reply to: ↑ 6 @iamadisingh
3 weeks ago

Replying to rollybueno:

I'll leave the needs testing keyword open so that other contributors can test as well, though I'll add needs-unit-tests since I can't find any related to this issue. Feel free to remove if you think otherwise.

Hi @rollybueno and @Guido07111975,

I see this ticket needs unit tests as mentioned in the comment. I can write a unit test that covers the core issue reported:

  • Multiple playlists with invalid first playlist (the main bug)

The test will validate that the fix in PR https://github.com/WordPress/wordpress-develop/pull/9003 works correctly. Would it be helpful if I create a separate PR for this unit test?

Thanks!

Last edited 3 weeks ago by iamadisingh (previous) (diff)

#9 @iamadisingh
3 weeks ago

Test Report

Description

This report validates whether the indicated patch works as expected.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/9003

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.2.28
  • Server: nginx/1.27.3
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
  • Browser: Chrome 137.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty-Five 1.2
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Actual Results

  1. ✅ Issue resolved with patch.

Additional Notes

  • Applied patch using: npm run grunt patch:https://github.com/WordPress/wordpress-develop/pull/9003
  • Testing Process:
    • Setup: Created test post with multiple playlist shortcodes using audio file attachment IDs from media library
    • Tested scenario: [playlist ids="999999"] followed by [playlist ids="11"] and playlist ids="12"] (valid audio file)
      • ✅ Invalid first playlist fails gracefully (returns empty)
      • ✅ Valid second and third playlist renders correctly with working audio controls

Supplemental Artifacts

Editor Screenshot: https://ibb.co/FLstHzMd
(WordPress admin editor showing test setup with playlist shortcodes)

Frontend Screenshot: https://ibb.co/nqY5ZnSk
(Frontend view showing fix working - invalid playlist fails gracefully, valid playlists render)

#10 @SirLouen
3 weeks ago

  • Keywords needs-testing removed

This ticket was mentioned in PR #9228 on WordPress/wordpress-develop by @iamadisingh.


10 days ago
#11

  • Keywords has-unit-tests added; needs-unit-tests removed

Trac ticket: #63583

Patch Used: #9003

---

## Description

Added a unit test for wp_playlist_shortcode to ensure that playlist scripts are loaded when a valid playlist follows an invalid one.

## Testing Instructions

Run the following command to verify the tests:

npm run test:php -- --filter=Tests_Media_WpPlaylistShortcode
Note: See TracTickets for help on using tickets.