Can't remove theme support for certain custom header arguments

[kovshenin]

While working on a ticket I tried to remove_theme_support the custom-header feature and add it back with a different default image, which did not work. Removing theme support *does* remove the feature array from the features global, but adding anything back resurrects the old values ignoring the new ones for:

• header-text
• height and width
• default-text-color
• default-image

This seems to be happening because of the back-compat code that works with the older constants. It defines the constants if they were not defined, and uses the values next time add_theme_support is called, bringing back the values from the first call to add_theme_support.

The workaround would be to not use remove_theme_support and instead call add_theme_support with the needed values *before* the call you're trying to override, which was sort of intended for child themes, because they run earlier, so this issue is far from major, but still lame. Run this during theme setup to reproduce:

add_theme_support( 'custom-header', array( 'default-image' => '%s/foo.png', ) );
var_dump( get_theme_support( 'custom-header', 'default-image' ) ); // foo.png

remove_theme_support( 'custom-header' );
add_theme_support( 'custom-header', array( 'default-image' => '%s/bar.png', ) );
var_dump( get_theme_support( 'custom-header', 'default-image' ) ); // expected bar.png but got foo.png

Excuse my debugging skills ;)

[kovshenin]

Introduced in [20212]

[nacin]

The constants aren't defined until after init. If you need to do any removal, you need to do it before then — on after_setup_theme or on init. Is that what is happening here?

[kovshenin]

It looks like the constants are defined during the first call to add_theme_support regardless of when it's called, tested with plugins_loaded. Most likely because of this:

if ( defined( 'HEADER_IMAGE' ) )
    $args[0]['default-image'] = HEADER_IMAGE;
elseif ( isset( $args[0]['default-image'] ) )
    define( 'HEADER_IMAGE', $args[0]['default-image'] );

So the second time you call add_theme_support (even if you called remove_theme_support before that) the values are taken from the defined constants.

[nofearinc]

Confirmed, I can reproduce and the problem here is in the defines (they're indeed triggered in add_theme_support for the first call and just reused for all other calls).

I was thinking of using a global array instead of the defines in add_theme_support and then converting to defines afterwards, but most themes (including default ones) use defines on after_setup_theme.

Maybe nacin has a wiser idea, I see it as wontfix with hooking earlier in the life cycle if that needs to be refined.

[kovshenin]

Hooking earlier to change the values actually makes sense, and that behavior was explicitly set in r20212 so that child themes can easily override custom header arguments, since their functions.php is loaded before the parent's. And I'm fine with that.

What bugs me is the fact that after I explicitly remove_theme_support and add it again with different arguments, some of them get rewritten, and others (ones that don't use constants, like flex-width, etc) don't.

[kovshenin]

Sooooo, more back compat code, yey! 25333.diff​ makes sure that values from defines is read only once and doesn't set any defines before wp_loaded runs. With this approach, child themes can override the parent theme's values by providing them first, either with a define or with add_theme_support.

However, any theme or plugin at any stage can use remove_theme_support and add_theme_support to provide new values, regardless of any defines and prior calls to add_theme_support. Tests included with the patch. And since our tests run after wp_loaded I also created a little ​plugin to test those defines.

If this works, we can use the same approach for the defines in custom background.

[kovshenin]

This blocks unit tests for #25156.

[chriscct7]

Patch still applies cleanly