unregister_default_headers() will sometimes return null

[conner_bw]

File: wp-includes/theme.php
Function: unregister_default_headers()
Line: ~1046

Problem:
PHPDoc comment says: @return True on success, false on failure.

Expected:
true or false:

Actual:
var_dump(unregister_default_headers(array('FAKE'))); will return NULL

Suggested fix, change:

function unregister_default_headers( $header ) {
	global $_wp_default_headers;
	if ( is_array( $header ) ) {
		array_map( 'unregister_default_headers', $header );
	} elseif ( isset( $_wp_default_headers[ $header ] ) ) {
		unset( $_wp_default_headers[ $header ] );
		return true;
	} else {
		return false;
	}
}

To:

function unregister_default_headers( $header ) {
	global $_wp_default_headers;
	if ( is_array( $header ) ) {
	        $_ret = array_map( 'unregister_default_headers', $header );
	        return in_array(false, $_ret, true) ? false : true;
	} elseif ( isset( $_wp_default_headers[ $header ] ) ) {
		unset( $_wp_default_headers[ $header ] );
		return true;
	} else {
		return false;
	}
}

Thank you for your consideration.

[MikeHansenMe]

patch based on code sample above minus shorthand

[aubreypwd]

Updated patch

[obenland]

I'm usually always in support of functions returning a single data type. But in this case I wonder if it wouldn't be a legitimate case of having two types, bool|void.

What is gained by having the function return true as soon as we have one successful removal of a number of header to be removed? Would you want the function to return true if one header was removed, but the other 5 that you passed weren't?

[SergeyBiryukov]

Replying to obenland:

What is gained by having the function return true as soon as we have one successful removal of a number of header to be removed? Would you want the function to return true if one header was removed, but the other 5 that you passed weren't?

Unless I'm missing something, the patch should only return true if all the headers were removed.

21876.4.diff​ was my initial thought as well, but 21876.3.diff​ seemed a bit less cryptic.

[aubreypwd]

Unless I'm missing something, the patch should only return true if all the headers were removed.

What is gained by having the function return true as soon as we have one successful removal of a number of header to be removed?

If we did something like true if all were removed, and an array() of headers that weren't removed, I'm not sure what the receiving function would do with that either.

I am assuming (because of my lack of full understanding of the function) that a person that does:

function _remove_twenty_ten_headers(){
    unregister_default_headers( array(
        'berries',
        'cherryblossom',
        'concave',
        'fern',
        'forestfloor',
        'inkwell',
        'path' ,
        'sunset')
    );
}
 
add_action( 'after_setup_theme', 'jorbin_remove_twenty_ten_headers', 11 );

...they just want to make sure those are removed, as they maybe cause problems with the child theme, etc.

I think the same array() could be returned with some information that would help the function or developer find out why false was returned. Might be more helpful for the developer if a header wasn't removed for any other reason than it was not in the array. Maybe help the developer take out unnecessary code?

array(
        'berries'=>true, //removed
        'cherryblossom'=>true,

        'concave'=>array( 
        	false, //not removed
        	'not_present' //why
        ),
        
        'fern'=>array(
        	false, //not removed
        	'unable_to_remove' //why
        ),
        'forestfloor'=>true,
        'inkwell'=>true,
        'path'=>true,
        'sunset'=>true
    );

I know var_dump() is one of my favorite functions :)

[nacin]

I think I'm with obenland on having this continue to return void when multiple headers are passed.

[nacin]

In 27575:

Clarify that unregister_default_headers() returns void in some situations. fixes #21876.