Opened 4 months ago
Last modified 3 months ago
#61515 accepted defect (bug)
wp_audio_shortcode() outputs invalid HTML
Reported by: | shub07 | Owned by: | 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
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.
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.
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.