get_custom_header() should return false when there is no header

[nacin]

Follow-up to #20871, discovered when working on #14531 (see [25135]).

When a custom header is removed but a default is specified, get_custom_header() will return that default information. This incorrect.

When the header_image theme mod shows that a header is removed, we should immediately return false.

I am suggesting a false return value in lieu of an empty stdClass as objects are not false-y, and thus it allows for if ( get_custom_header() ) {. (Accessing a property of a non-object merely triggers a notice, versus a fatal error for methods, so this is backwards compatible.)

I imagine a has_custom_header() function may be a good addition at some point.

This wasn't noticed because Twenty Twelve (and other default themes) check get_header_image() first, which returns false when there is no custom header. (has_custom_header() would probably be a bool cast of either get_header_image() or get_custom_header().) Twenty Thirteen doesn't use get_custom_header() at all, in part (I'm guessing) because it doesn't support variable width or variable height.

[nacin]

Asked kovshenin to look this over.

[nacin]

Looking at $url = get_theme_mod( 'header_image', get_theme_support( 'custom-header', 'default-image' ) ); in get_header_image(). Seems like we need to handle the case where get_theme_mod() returns false and there is no default-image or default random image. Not sure if this does that.

[kovshenin]

Seems like we need to handle the case where get_theme_mod() returns false and there is no default-image or default random image. Not sure if this does that.

It does, but it returns an empty string, not false. get_custom_header on the other hand doesn't. Most themes check the result of get_header_image before running get_custom_header. However, looking around the themes directory I found at least five themes that attempt to access the properties of get_custom_header directly:

• ​http://themes.svn.wordpress.org/_second-foundation/1.0.4.3/header.php (old version)
• ​http://themes.svn.wordpress.org/alexandria/2.0.1/slider-cheader.php
• ​http://themes.svn.wordpress.org/aplau/1.2.1/header.php
• ​http://themes.svn.wordpress.org/bitlumen/1.0.1/functions-settings.php
• ​http://themes.svn.wordpress.org/spartan/1.2.32/archive.php

As you mentioned, they'll trigger a notice, but I don't think it will really break anything because the image URL is empty either way. Still looking into default and random, maybe we should return false there for consistency too.

[kovshenin]

25156.2.diff​ returns false for get_header_image and get_custom_header if a default header image was not set and random is turned off.

[lancewillett]

I imagine a has_custom_header() function may be a good addition at some point.

+1

[wonderboymusic]

Is this Unit Test-able, @kovshenin?

[kovshenin]

I don't see why not. Will throw in some tests early next week.

[kovshenin]

Looked into tests, they would be much easier with #25333 because tests are run after wp_loaded.

[nacin]

It's a bug, but not a regression, so moving to future. I'm a bit concerned what we might break here.

[celloexpressions]

Closing for now per the slack conversation above due to lack of interest. It sounds like a good idea, but doesn't seem to be something people run into much in practice. If anyone's interested in working on this, please feel free to reopen.