Opened 2 years ago
Last modified 2 years ago
#56025 new defect (bug)
wp_validate_boolean() not doing what it describes, causes issues with [video] shortcode
Reported by: | arnodeleeuw | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | 6.0 |
Component: | General | Keywords: | has-patch dev-feedback |
Focuses: | Cc: |
Description
The function in question:
/** * Filter/validate a variable as a boolean. * * Alternative to `filter_var( $var, FILTER_VALIDATE_BOOLEAN )`. * * @since 4.0.0 * * @param mixed $var Boolean value to validate. * @return bool Whether the value is validated. */ function wp_validate_boolean( $var ) { if ( is_bool( $var ) ) { return $var; } if ( is_string( $var ) && 'false' === strtolower( $var ) ) { return false; } return (bool) $var; }
Steps to recreate the issue:
Add the following shortcodes to a page:
[video src="YOUR-SOURCE-HERE"] [video src="YOUR-SOURCE-HERE" loop="off"] [video src="YOUR-SOURCE-HERE" loop="0"] [video src="YOUR-SOURCE-HERE" loop="false"]
- The first shortcode works as intended, rendering a video on the frontend without the loop attribute.
- The second shortcode's <video> element will have the attribute
loop="1"
despite the loop attribute being set to off. - The third and fourth shortcode's <video> elements will not have the loop attribute set.
Description:
wp_validate_boolean() says it 's an alternative to filter_var( $var, FILTER_VALIDATE_BOOLEAN )
, however
filter_var( $var, FILTER_VALIDATE_BOOLEAN )
will return FALSE when you pass the string "off" whereas wp_validate_boolean()
will return TRUE in this case.
(See this for the documentation for filter_validate_boolean: https://www.w3schools.com/php/filter_validate_boolean.asp).
Currently, wp_validate_boolean()
only returns FALSE if the $var is a string and if it is === "false", or if the $var passed is the int 0.
This causes unexpected behaviour in the [video] shortcode. The documentation of the video shortcode (https://wordpress.org/support/article/video-shortcode/) mentions that the loop attribute needs to be either "on"/"off". Because they are strings wp_validate_boolean()
will return TRUE, causing the loop attribute to always be added with a value of "1" unless you change the value of the attribute to be specifically the string "false", the int 0, or leave it completely empty.
The parsing of the attributes of the [video] shortcode happens here: (https://github.com/WordPress/wordpress-develop/blob/6.0/src/wp-includes/media.php#L3322-L3344).
As you can see on line 3340 it uses wp_validate_boolean()
to get filter value of the loop attribute. Passing "off" here returns true, causing the loop attribute to always be added to the eventual HTML output later down the line with a value of "1".
Potential fix:
- Change the working of
wp_validate_boolean()
to do as it says in its description. Make it smarter, and thus also return FALSE when the strings "off","OFF","no","NO", etc. are passed just likefilter_var( $var, FILTER_VALIDATE_BOOLEAN )
does (See this documentation link: https://www.php.net/manual/en/function.filter-var.php#121263).
- Change how the parsing of attributes happens in the [video] shortcode so that it also works as the documentation says: not adding certain attributes when they are set to "off". However there may be other instances where
wp_validate_boolean()
is used that I'm currently not aware of, so this would be more like a band-aid fix and similar issues may arise in the future.
update wp_validate_boolean() to validate false-y strings