Make WordPress Core

Opened 9 years ago

Closed 8 years ago

#38390 closed defect (bug) (fixed)

Twenty Seventeen: Playlists don't render on blog/archive pages

Reported by: melchoyce's profile melchoyce Owned by: joemcgill's profile joemcgill
Milestone: 4.7 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch needs-testing
Focuses: Cc:

Attachments (1)

38390.diff (1.7 KB) - added by joemcgill 8 years ago.

Download all attachments as: .zip

Change History (14)

#1 @karmatosed
9 years ago

  • Keywords good-first-bug added

#2 @karmatosed
9 years ago

  • Keywords good-first-bug removed

#3 @davidakennedy
9 years ago

This definitely seems like something we should figure out. It's weird that playlists aren't included there, and more problematic that they show up broken right now.

This ticket was mentioned in Slack in #core-themes by davidakennedy. View the logs.


8 years ago

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


8 years ago

#6 @davidakennedy
8 years ago

This was talked about in the weekly meeting for Twenty Seventeen. The group decided the best option here for now is to update the core function get_media_embedded_in_content() to check to make sure there’s actually a src in the tag before pulling out content. @joemcgill offered to help there.

This ticket was mentioned in Slack in #core-themes by davidakennedy. View the logs.


8 years ago

#8 @joemcgill
8 years ago

  • Owner set to joemcgill
  • Status changed from new to accepted

Will own addressing this in 4.7beta.

#9 @joemcgill
8 years ago

As @laurelfulford mentioned on GitHub, the issue here is a result of the way the content-audio.php and content-video.php template partials in TwentySeventeen attempt to highlight media found in post content by using get_media_embedded_in_content() to attempt to extract videos from the content and display their HTML differently.

However, the HTML being generated by the playlist shortcode relies on JS to update the video element with the markup needed to display the playlist properly. The get_media_embedded_in_content() function wasn't designed to handle this use case.

We should probably look into adding some logic to get_media_embedded_in_content() that accounts for the playlist markup, but in the short-term, I would suggest addressing this in the theme by adding a check for the presence of the playlist shortcode before trying to modify the content that is displayed in ! is_single() contexts.

@joemcgill
8 years ago

#10 @joemcgill
8 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

38390.diff is one way we could address this issue in Twenty Seventeen. It looks for the presence of wp-playlist-script in the content and shows the standard content rather than trying to pluck the media elements from the content using get_media_embedded_in_content().

If we want to be able to support displaying playlists the same way we handle other embedded audio or video in their respective post format partials, then we will need to come up with a better way by either making get_media_embedded_in_content() smarter about handling playlists or by changing the way we detect playlists in the partials themselves.

This ticket was mentioned in Slack in #core-themes by joemcgill. View the logs.


8 years ago

This ticket was mentioned in Slack in #core-themes by davidakennedy. View the logs.


8 years ago

#13 @davidakennedy
8 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 39146:

Twenty Seventeen: Fix playlists not rendering on blog/archive pages when using video or audio post format

TwentySeventeen attempts to highlight media found in post content by using get_media_embedded_in_content() to extract videos from the content and display their HTML differently. However, the HTML being generated by the playlist shortcode relies on JavaScript to update the video element with the markup needed to display the playlist properly. The get_media_embedded_in_content() function wasn't designed to handle this use case.

The patch looks for the presence of wp-playlist-script in the content and shows the standard content rather than trying to pluck the media elements from the content using get_media_embedded_in_content().

Props joemcgill.

Fixes #38390.

Note: See TracTickets for help on using tickets.