Make WordPress Core

Opened 4 months ago

Last modified 3 months ago

#61515 accepted defect (bug)

wp_audio_shortcode() outputs invalid HTML

Reported by: shub07's profile shub07 Owned by: joedolson's profile joedolson
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch
Focuses: Cc:

Description

Did an audit of a website and found several invalid HTML for audio tags that were output with wp_audio_shortcode(). The errors:
Bad value 1 for attribute loop on element audio
Bad value 1 for attribute autoplay on element audio
Bad value 1 for attribute muted on element audio
Based on documentation from Mozilla, all 3 are boolean attributes. Here is an example of function usage that produced the HTML validation errors:

<?php
echo wp_audio_shortcode( [
    'src'      => wp_get_attachment_url(9999),
    'class'    => 'my-custom-audio',
    'poster'   => '',
    'loop'     => 'true',
    'autoplay' => 'true',
    'muted'    => 'true',
    'height'   => 1080,
    'width'    => 1920
] );

This part in wp_audio_shortcode() is the culprit:

<?php
$attr_strings = array();
foreach ( $html_atts as $k => $v ) {
        $attr_strings[] = $k . '="' . esc_attr( $v ) . '"';
}

Currently, we are using the filter to clean up these attributes like this:

<?php
foreach ( $html_atts as $k => $v ) {
                $attr_strings[] = $k . '="' . esc_attr( $v ) . '"';
                if ( in_array( $k, array( 'loop', 'autoplay', 'muted' ), true ) && true === $v ) {
                        $attr_strings[] = esc_attr( $k );
                } else {
                        $attr_strings[] = $k . '="' . esc_attr( $v ) . '"';
                }
        }

Similar to: https://core.trac.wordpress.org/ticket/60178

Change History (5)

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


4 months ago
#1

  • Keywords has-patch added

wp_audio_shortcode() was chaging boolean attributes to 1, as of true === 1, in debug i notice it was directly happening after foreach, not by eascaping, so I have add a if statement to handle boolean attributes and directly pass our key, as Key can be directly work for boolean attributes in HTML.

@narenin commented on PR #6917:


4 months ago
#2

Hi @shubham07kb

The PR looks good to me.

@shub07 commented on PR #6917:


3 months ago
#3

Hi @dmsnell ,
Thank you for your feedback. I have implemented the suggested changes to utilize the HTML API for generating the <audio> tag. I agree that utilizing the internal HTML API is a better approach for generating the <audio> tag. It simplifies the code and efficiently handles boolean attributes.

@dmsnell commented on PR #6917:


3 months ago
#4

@shubham07kb there are some existing tests for this in tests/phpunit/tests/media.php.
do you think you could add a new test covering this invalid boolean attribute behavior? and then verify that the test fails before applying your fix?

I guess it's worth stating that the boolean attributes with a value of "1" should actually still be true and semantically equivalent to their value-less counterparts. I'm sure a validator will say they are invalid, but behaviorally the existing code should have properly conveyed the boolean true nature of the attribute.

#5 @joedolson
3 months ago

  • Owner set to joedolson
  • Status changed from new to accepted
Note: See TracTickets for help on using tickets.