Make WordPress Core

Opened 8 months ago

Last modified 3 months ago

#61398 accepted defect (bug)

Missing src attribute on <audio> tag prevents m4a audio files playing in safari.

Reported by: ericmulder's profile ericmulder Owned by: joedolson's profile joedolson
Milestone: 6.8 Priority: normal
Severity: normal Version: 6.6
Component: Media Keywords: has-patch needs-testing
Focuses: Cc:

Description

We have discovered an issue where the audio shortcode doesn't work in safari. When you add a m4a file to the shortcode, and you don't use the wp-mediaelement library the <audio> tag is created without a src element. Safari expect a src element, and if we add one the m4a audio tag works.

We have patched this with a filter, but I think it would be better to fix this in the core.

I see that on wp-includes/media.php:3067 $html_atts are set, but no 'src' attribute is present. Shall I create a pull request for this?

Our patch:

function peek_audio_shortcode($html, $atts, $audio, $post_id, $library) {
    // check if the <audio> tag has a src attribute (use a preg match <audio src="" >). If not, get the first <source> tag
    if (!preg_match('/<audio[^>]*src\s*=\s*"[^"]*"[^>]*>/', $html)) {
        // get the first source tag
        preg_match('/<source[^>]*src\s*=\s*"([^"]*)"[^>]*>/', $html, $matches);
        $src = $matches[1];
        // now add the src="" to the audio tag
        $html = preg_replace('/<audio/', '<audio src="' . $src . '"', $html);
    }
    return $html;
}
add_filter('wp_audio_shortcode', 'peek_audio_shortcode', 10, 5);

Change History (12)

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


8 months ago
#1

  • Keywords has-patch added

Add the src attribute in an <audio> tag as required by safari.

Trac ticket: https://core.trac.wordpress.org/ticket/61398

This ticket was mentioned in Slack in #core-test by mai21. View the logs.


8 months ago

#4 @mai21
8 months ago

@ericmulder Thanks for reporting the issue.

Reproduction Report

Description

The issue is reproducible using BrowserStack Sonoma Safari 17.3 (The m4a is displayed and not working)

Environment

  • WordPress: 6.6-beta2-58396
  • PHP: 8.3.6
  • Server: nginx/1.24.0
  • Database: mysqli (Server: 10.6.16-MariaDB-0ubuntu0.22.04.1-log / Client: mysqlnd 8.3.6)
  • Browser: Sonoma on Browserstack, Safari 17.3 latest
  • OS: Linux
  • Theme: Twenty Eleven 4.6
  • MU Plugins: None activated
  • Plugins:
    • Code Snippets 3.6.5.1
    • Test Reports 1.1.0
    • WordPress Beta Tester 3.5.5

Steps to Reproduce

  1. Create a page with m4a shortcode i.e
    [audio src="http://domain/wp-content/uploads/2024/06/sample1.m4a"]
    
  2. Use a code snippet plugin to activate this code in additional note
  3. Visit the page on chrome and play the audio => audio is working
  4. Visit the page on Safari and play the audio

Actual Results

  1. 🐞 Audio isn't working

Expected Results

  1. ✅ Audio is working as it does on Chrome

Additional Notes

  • To be able to reproduce, we need to make sure that no media files are in the source. We may need this code snippet to disable it. (thanks to @Boniu91 for the snippet)
//Dequeue Styles
function project_dequeue_unnecessary_styles() {
    wp_dequeue_style( 'wp-mediaelement' );
    wp_deregister_style( 'wp-mediaelement' );
}
add_action( 'wp_print_styles', 'project_dequeue_unnecessary_styles' );

//Dequeue JavaScripts
function project_dequeue_unnecessary_scripts() {
    wp_dequeue_script( 'wp-mediaelement' );
    wp_deregister_script( 'wp-mediaelement' );
}
add_action( 'wp_print_scripts', 'project_dequeue_unnecessary_scripts' );
Last edited 8 months ago by mai21 (previous) (diff)

#5 @mai21
8 months ago

Test Report

Description

This report validates that the indicated patch works as expected.

Patch tested: Adding patch code in code snippet

function peek_audio_shortcode($html, $atts, $audio, $post_id, $library) {
    // check if the <audio> tag has a src attribute (use a preg match <audio src="" >). If not, get the first <source> tag
    if (!preg_match('/<audio[^>]*src\s*=\s*"[^"]*"[^>]*>/', $html)) {
        // get the first source tag
        preg_match('/<source[^>]*src\s*=\s*"([^"]*)"[^>]*>/', $html, $matches);
        $src = $matches[1];
        // now add the src="" to the audio tag
        $html = preg_replace('/<audio/', '<audio src="' . $src . '"', $html);
    }
    return $html;
}
add_filter('wp_audio_shortcode', 'peek_audio_shortcode', 10, 5);

Environment

  • WordPress: 6.6-beta2-58396
  • PHP: 8.3.6
  • Server: nginx/1.24.0
  • Database: mysqli (Server: 10.6.16-MariaDB-0ubuntu0.22.04.1-log / Client: mysqlnd 8.3.6)
  • Browser: Chrome 125.0.0.0
  • OS: Linux
  • Theme: Twenty Eleven 4.6
  • MU Plugins: None activated
  • Plugins:
    • Code Snippets 3.6.5.1
    • Test Reports 1.1.0
    • WordPress Beta Tester 3.5.5

Actual Results

  1. ✅ Issue resolved with patch => m4a is working either added by media or shortcode

Additional Notes

  • Tested on Chrome, Safari, Firefox and Opera

#6 @ericmulder
8 months ago

Thank you very much for validating and reproducing the bug. To clarify, when you use the wp-mediaelement packages the problem doesn't occur since that js package adds the src element to the <audio> tag. We found the issue using an in-app browser in our app on iOS (which apparently doesn't load the wp-mediaelement)

Good to see that the patch works, please take into account this is a hotfix from our side to not tinker with the WP core code and use a filter. I believe the pull request will be the permanent fix for this issue: https://github.com/WordPress/wordpress-develop/pull/6755

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


8 months ago

#8 @joedolson
8 months ago

  • Milestone changed from Awaiting Review to 6.7
  • Owner set to joedolson
  • Status changed from new to accepted

This ticket was mentioned in Slack in #core by chaion07. View the logs.


4 months ago

#10 @chaion07
4 months ago

  • Keywords needs-testing added

Thanks @ericmulder for reporting this. We reviewed this Ticket during a recent bug-scrub session. Here's a summary of the feedback received:

  1. The reporter has raised a PR that might need testing. So we are adding the keyword for this.
  2. We would appreciate specific instructions for testing as the reporter says the issue doesn't occur when using wp-mediaelement packages (we suspect that we may have to manually dequeue the wp-mediaelement JS to be able to test this).

Thanks.

Props to @pratiklondhe

Cheers!

#11 @davidbaumwald
4 months ago

  • Milestone changed from 6.7 to 6.8

With 6.7 RC 1 releasing today this ticket is being moved to 6.8.

If any maintainer or committer feels the remaining work can be competed in time for 6.7 feel free assume ownership and update the milestone accordingly.

#12 @dvershinin
3 months ago

Affected by this as well, and I can confirm that the patch is working fine.

#13 @im3dabasia1
3 months ago

Test Report

Description

This report validates whether the indicated patch works as expected.
Patch tested: https://github.com/WordPress/wordpress-develop/pull/6755.diff

Added this shortcode in the new post
[audio src="http://localhost:9889/wp-content/uploads/2024/11/sample-1-1.m4a"]

Environment

WordPress: 6.8-alpha-59274-src
PHP: 8.2.25
Server: nginx/1.27.2
Database: mysqli (Server: 8.0.40 / Client: mysqlnd 8.2.25)
Browser: Chrome 131.0.0.0
OS: macOS
MU Plugins: None activated

Actual Results

✅ The audio tag now has src attribute in safari and the audio is playable, m4a audio clip is not playable.

Supplemental Artifacts

Video: https://jmp.sh/bZEleaAj

Added this code

// Dequeue Styles
function project_dequeue_unnecessary_styles() {
	wp_dequeue_style( 'wp-mediaelement' );
	wp_deregister_style( 'wp-mediaelement' );
}
add_action( 'wp_print_styles', 'project_dequeue_unnecessary_styles' );

// Dequeue JavaScripts
function project_dequeue_unnecessary_scripts() {
	wp_dequeue_script( 'wp-mediaelement' );
	wp_deregister_script( 'wp-mediaelement' );
}
add_action( 'wp_print_scripts', 'project_dequeue_unnecessary_scripts' );

Note: See TracTickets for help on using tickets.