Opened 15 months ago
Last modified 7 months ago
#51657 accepted defect (bug)
Correct displaying the _doing_it_wrong() message for add_theme_support( 'html5' )
Reported by: | SergeyBiryukov | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | |
Component: | Themes | Keywords: | has-screenshots has-patch needs-dev-note needs-refresh needs-unit-tests |
Focuses: | Cc: |
Description (last modified by )
As a result of [25193], [25235], and [25785] add_theme_support( 'html5' )
is supposed to return false
and display a _doing_it_wrong()
notice: "You need to pass an array of types".
As far as I can tell, this has never worked as expected, due to ! is_array( $args[0] )
check being in the elseif
condition after empty( $args[0] )
, which always succeeds in this case.
Currently, calling add_theme_support( 'html5' )
without passing an array of supported types just silently falls back to array( 'comment-list', 'comment-form', 'search-form' )
for backward compatibility, without any indication that it is not the recommended approach.
Attachments (2)
Change History (11)
#3
@
12 months ago
- Keywords has-screenshots has-patch added
Nice catch @SergeyBiryukov!
I think 51657.diff does the job :)
Just wonder if we want to change the version
param. I think it's fine to keep the reference to the version it was added, but on another hand, it would be nice to upgrade the version to 5.7.0 as the notice will be displayed for the very first time. Any thought?
#4
@
12 months ago
- Keywords needs-dev-note added
Adding needs-dev-note
workflow keyword. It won't need a full dev note, but a quick mention in the Miscellaneous changes won't hurt :)
This ticket was mentioned in Slack in #themereview by poena. View the logs.
12 months ago
#6
@
11 months ago
- Keywords needs-refresh needs-unit-tests added
Reviewing 51657.diff :
- The condition can be reduced to
if ( empty( $args[0] ) || ! is_array( $args[0] ) )
--empty()
is a short cut for! isset($var) || ! $var
. - I haven't run the full test suite but I am getting a few failures in
Tests_Theme_Support
- Some additional tests to make sure the feature is throwing a doing it wrong as expected would be ideal too
> phpunit --filter Tests_Theme_Support .....FFF.... 12 / 12 (100%) There were 3 failures: 1) Tests_Theme_Support::test_supports_html5 Unexpected incorrect usage notice for add_theme_support( 'html5' ) Failed asserting that an array is empty. /vagrant/wordpress-develop/tests/phpunit/includes/abstract-testcase.php:516 /vagrant/wordpress-develop/tests/phpunit/includes/abstract-testcase.php:528 2) Tests_Theme_Support::test_supports_html5_subset Failed asserting that null is false. /vagrant/wordpress-develop/tests/phpunit/tests/theme/support.php:101 3) Tests_Theme_Support::test_supports_html5_invalid Failed asserting that null is false. /vagrant/wordpress-develop/tests/phpunit/tests/theme/support.php:130
Themes: Correct displaying the _doing_it_wrong() message for add_theme_support( 'html5' )