default-preset option for custom background cannot be set by default

[cristian-ungureanu]

I am trying to set a custom background by default for this theme ​https://wordpress.org/themes/islemag/ . This is my code:

		add_theme_support( 'custom-background', array(
			'default-image'          => get_template_directory_uri() . '/img/islemag-background.jpg',
			'default-preset'         => 'fill',
			'default-repeat'         => 'no-repeat',
			'default-position-x'     => 'center',
			'default-attachment'     => 'fixed',

		) );

When I open the customizer, the default preset option is set on "fill screen" but the background image is still small. Everything works great if I change the default preset and then change it back on "fill screen"

I talked with developers in core channel on slack and @celloexpressions told me this:
"Currently you need to define the default for each individual property (size, repeat, etc.) in addition to defining the preset. It would probably be better for consistency if each property is set based on the preset if the theme sets a preset."

Tried to set each property for custom background but with no luck.

[celloexpressions]

Moving to 4.7.1 for investigation since this is introduced in 4.7. Could move to a 4.7.2 if there isn't a solution ready in time. CC @cdog since I have a feeling you may know the best way to fix this.

Ideally, themes should only need to set the default-preset in the add theme support and the other properties would be based on that where applicable.

[cdog]

Thanks for reporting! Looking into it.

[celloexpressions]

Let's aim for 4.7.2; this is a fairly major bug with the feature introduced in 4.7 in terms of themes implementing it, but it'll be better to give a bit more time to work out the correct fix. If there's a minimal patch ready by tomorrow we could reconsider getting it in sooner, though.

[cdog]

OK, this should be better documented. It works as designed.

This should get you working (you've omitted the background size setting):

add_theme_support( 'custom-background', array(
	'default-image'      => get_template_directory_uri() . '/img/islemag-background.jpg',
	'default-size'       => 'cover',
	'default-repeat'     => 'no-repeat',
	'default-attachment' => 'fixed',
) );

Anytime the Default preset is selected it will revert to your theme's default settings.

Also make sure to remove any theme mods if already set (using wp-cli):

$ wp theme mod remove background_size background_position_x background_position_y background_repeat background_attachment

The Default preset is intended to be altered by themes. A theme can define its own defaults by overriding core defaults. See wp-includes/theme.php:

$defaults = array(
	'default-image'          => '',
	'default-preset'         => 'default',
	'default-position-x'     => 'left',
	'default-position-y'     => 'top',
	'default-size'           => 'auto',
	'default-repeat'         => 'repeat',
	'default-attachment'     => 'scroll',
	'default-color'          => '',
	…
);

The other presets can't be altered (and shouldn't; doing so can cause unexpected results). The 'default-preset' should be set other than 'default' only when you want to use an already defined preset exactly as it is. Doing so requires you to set all options according to the preset (maybe we can improve this). See wp-admin/js/customize-controls.js:

values = { // position_x, position_y, size, repeat, attachment
	'default': defaultValues,
	'fill': [ 'left', 'top', 'cover', 'no-repeat', 'fixed' ],
	'fit': [ 'left', 'top', 'contain', 'no-repeat', 'fixed' ],
	'repeat': [ 'left', 'top', 'auto', 'repeat', 'scroll' ]
};

Any thoughts on this are welcome. @celloexpressions, @cristian-ungureanu what do you think?

[celloexpressions]

Thanks for looking into this @cdog. Do you think it would be possible for the the various properties to default to the defaults associated with the default preset that the theme sets? For example, if a theme does this:

add_theme_support( 'custom-background', array(
	'default-image'  => get_template_directory_uri() . '/img/islemag-background.jpg',
	'default-preset' => 'fill',
));

Could the default background-size, etc. be set automatically?

[cdog]

Thanks for the input @celloexpressions. I'll think of a solution and propose a patch. When should this be ready?

[celloexpressions]

It can go in whenever it's ready :) I think 4.7.2 is at least a few weeks out if we want to target that release.

[cdog]

Sorry for delay, got busy lately. The good news is that I just got 2 tickets to WordCamp Europe! See you there :)

Now on the patch. 39561.diff​ (misspelled it, can someone rename it?) provides a solution for what we discussed and addresses a display issue when loading the customizer. Specifically some controls can toggle the display of other controls. Choosing a background image was either showing or hiding all background settings overriding the selected preset related controls (for example, the Default preset shouldn't display any controls; choosing an image was showing them).

Next, I'll provide some working examples for a better understanding of the changes and how this can be used. The workflow from the interface remains the same.

Example 1. Choosing a default preset (this could be default, fill, fit, repeat or custom)

add_theme_support( 'custom-background', array(
	'default-image'  => get_template_directory_uri() . '/img/autumn.jpg',
	'default-preset' => 'fill'
));

Specifying a value for default-preset will set the active preset and inherit all properties of the preset.

Example 2. Overriding default settings for presets

add_theme_support( 'custom-background', array(
	'default-image'      => get_template_directory_uri() . '/img/autumn.jpg',
	'default-position-x' => 'center',
	'default-position-y' => 'center'
));

This will apply for all presets. Prior to the patch this was affecting the Default preset only. Switching a preset from one to another will revert to these values instead of core defaults.

add_theme_support( 'custom-background', array(
	'default-image'  => get_template_directory_uri() . '/img/autumn.jpg',
	'default-preset' => 'fit'
	'default-repeat' => 'no-repeat'
));

Because the Fit to Screen preset allows you to toggle the Repeat Background Image option from the interface now you can do the same from your theme.

add_theme_support( 'custom-background', array(
	'default-image'      => get_template_directory_uri() . '/img/autumn.jpg',
	'default-preset'     => 'fit'
	'default-attachment' => 'fixed'
));

The default-attachment property will be ignored in this case. It will affect only the presets that allow this change (like Default, Repeat and Custom). All values are sanitized to drop any invalid settings.

Example 3. Owning the Default preset from a theme (revert to your own defaults instead of core defaults)

add_theme_support( 'custom-background', array(
	'default-image'      => get_template_directory_uri() . '/img/autumn.jpg',
	'default-preset'     => 'default',
	'default-position-x' => 'center',
	'default-position-y' => 'center',
	'default-size'       => 'cover',
	'default-repeat'     => 'no-repeat',
	'default-attachment' => 'fixed'
));

Doing this you will have your own Default preset.

@celloexpressions hope this helps. I aimed for greater flexibility and to fix existing issues. Any feedback is welcome. Thanks!

[swissspidy]

Moving to the 4.8 milestone for now as decided during today's bug scrub.

[celloexpressions]

9 and 39561.diff​ needs a review still; it's been long enough since I've dug into this that it would probably be most effective if a committer can do it. It looks pretty good to me at a glance.

[cdog]

Refreshed the patch and added a missing docblock. See: 39461.diff​. This should fix the op issue and a few others (minor changes) described in my previous comments without changing the existing functionality in core. I'm also here if there's something to be discussed/updated prior to commiting the current patch or any feedback on a better approach.

[cdog]

@melchoyce Hello, anything I can do to help here? Thanks!

[melchoyce]

Triaged today on Slack: ​https://wordpress.slack.com/archives/C02RQBWTW/p1507568628000213

Needs someone to take ownership of reviewing. Maybe @celloexpressions, if he has time?

[peterwilsoncc]

Switching milestone due to the focus on the new editor (Gutenberg) for WordPress 5.0.

[celloexpressions]

As noted in 18, the patch looks good but needs committer-level review and more testing.

I'm also concerned about changing the behavior at this point. Since we didn't get this into the initial minor releases after the background presets were introduced in 4.7, themes have been assuming the somewhat-broken/unexpected behavior for the past few years. The information in 9 should probably be documented with a dev-note on make/core as well.