Removing background image and/or reverting background color to default doesn't remove custom-background body class and remove theme mods

[slobodanmanic]

Scenario:

• Set custom background color, then revert do default or
• Set custom background image, then remove it

Problem:

• None of the theme mods are removed
• Because of this .custom-background is still being added

This is what I have in theme_mods_twentyfourteen after setting background color and image, then removing them both:

s:16:"background_color";s:6:"f5f5f5";
s:16:"background_image";s:0:"";
s:22:"background_image_thumb";s:0:"";
s:17:"background_repeat";s:6:"repeat";
s:21:"background_position_x";s:4:"left";
s:21:"background_attachment";s:6:"scroll";

Sorry if this has been filed before. I checked for tickets that could relate to this, but no luck.

I'll work on a patch today, but it would be great if someone could say if there's a reason for this behaviour.

[slobodanmanic]

28687-2.diff checks if background color theme mod value is the same as theme default and if yes, doesn't add .custom-background to body classes.

Another problem remains, it's related, but not the same, and it has to do with all background related theme mods being kept in the database.

With background image:

• when you remove it, it sets background_image theme mod to empty string, to make sure missing fallback to theme default image doesn't happen
• when you reset it, theme mod is deleted and fallback to theme default happen

Color should work the same way, resetting to default should delete background_color theme mod, so it can fallback to theme default background color. 28687.diff does this.

Posted as two separate patches because these are two different, although closely related problems. I can post unified patch if that makes more sense.

[slobodanmanic]

There's also Customizer problem, it also doesn't remove any background theme mods when set to default. Is that possible in Customizer?

[helen]

Related: #22030

I imagine this has happened since before 3.9, but please do let me know if this is only happening in trunk.

[slobodanmanic]

Replying to helen:

Related: #22030

I imagine this has happened since before 3.9, but please do let me know if this is only happening in trunk.

It's not only trunk and looking at #22030 it seems like it's been like that for a while.

[obenland]

I could reproduce it. Let's fix that.

[slobodanmanic]

Replying to obenland:

I could reproduce it. Let's fix that.

I'll post a patch over the weekend.

[obenland]

slobodanmanic, have you had a chance to take another look?

[slobodanmanic]

Replying to obenland:

slobodanmanic, have you had a chance to take another look?

Still working on it, sorry. Will post an update soon.

[slobodanmanic]

Replying to obenland:

slobodanmanic, have you had a chance to take another look?

Can I get your opinion?

28687-3.diff is similar to 28687-2.diff, with one extra check and, if you agree with the implementation is the only file that needs to be changed.

Compared to current version of the file in trunk, it will now:

1. Check if get_theme_mod( 'background_color' ) is set
2. Check if its value is not equal to theme default
3. (And check if there's a background image, which is all it currently does)

Theme mods are still not deleted and set to empty string instead, but .custom-background class will not be applied if both color and image are not set, or if color is set to theme default value.

Deleting theme mods when saving them from Custom Background page is not a problem, but I just can't figure it out when doing it from Customizer. I'd love to figure it out, but after looking at this for a while, I think that either:

• Removing theme these two theme mods is necessary at all, since empty values do the trick OR
• Removing background_image theme mod should also automatically remove background_repeat, background_position_x and background_attachment, and not leave them as they are, since they are tied to the image that was used

Let me know what you think, and if you have an idea how to remove background_image theme mod when Customizer saves after you removed background image, please let me know.

I tried adding remove_theme_mod to update in WP_Customize_Background_Image_Setting class, but that didn't work.

[slobodanmanic]

In other words, this should be two issues:

1. Remove .custom-background class when not needed, fixed in 28687-3.diff
2. Remove theme mods, if this even needs to be done in first place

[obenland]

I'll take a look soon, thanks!

[nitkr]

The condition check in this 28687-4.diff seems enough to fix this bug, theme mods doesn't seem like an issue.

[valendesigns]

The previous patch didn't apply to the root, you had to cd into src. I created a new patch 28687.5.diff​ to fix that. I also tested the proposed change and it does work as expected. Cheers!

[DrewAPicture]

@obenland: Can you pretty please test 28687.5.diff​ and make a recommendation? :-)

[ocean90]

The patch needs to be checked against [21001].

[obenland]

Replying to ocean90:

The patch needs to be checked against [21001].

r21054 reverted parts of r21001, and 28687.5.diff​ is coherent with the last sentence in that commit message:

[…] the custom-background class should not be shown when only a default color is in use.

The patch allows .custom-background only to be added if background color or image divert from their defaults.

The question whether a theme mod should be removed if reset to its default value, could be material for another discussion or ticket. @ocean90 and I only ​started talking about it.

[ocean90]

Thanks @obenland. 28687.5.diff​ makes sense and would be consistent with the behaviour of _custom_background_cb().

But we need to update the callback for the customize preview too: trunk/src/wp-includes/js/customize-preview.js@31697#L139​

[obenland]

Replying to ocean90:

But we need to update the callback for the customize preview too: trunk/src/wp-includes/js/customize-preview.js@31697#L139​

Is there ever a situation where color() doesn't return a color?

Also: With wp.customize holding the currently saved value, do we have defaults available anywhere?

[ocean90]

In 32081:

Don't print the custom-background class in body_class() when a default color is in use.

.custom-background should only be added if a background color or an image divert from their defaults. This behavior exists already in _custom_background_cb().

props slobodanmanic, nitkr, valendesigns, obenland.
see [21054], [21001].
fixes #28687.

[arenddeboer]

This might be a new bug, but when changing the wordpress url scheme to https from http, the database entry with the absolute url to the background image css is not updated, resulting in a mixed content warning.