Make WordPress Core

Opened 10 years ago

Closed 8 years ago

#30299 closed defect (bug) (maybelater)

Inconsistent custom-background Body Class Behavior

Reported by: philiparthurmoore's profile philiparthurmoore Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Customize Keywords: 2nd-opinion
Focuses: Cc:

Description

Possibly related to #22030 and [21054] (specifically this)

Currently there’s a problem with the following code:

	if ( get_theme_mod( 'background_color' ) || get_background_image() )
		$classes[] = 'custom-background';

On newly installed blogs get_theme_mod( 'background_color' ) will return false, while on blogs that have a custom background set and then reverted to the default background color declared by the theme get_theme_mod( 'background_color' ) will return the hex string of the default background.

  1. Activate a new site.
  2. var_dump( get_theme_mod( 'background_color' ) ) will give you false.
  3. Set a custom background color.
  4. Set the background back to its default color declared by the theme.
  5. var_dump( get_theme_mod( 'background_color' ) ) will give you the string of the background color.

What this means is that themes cannot safely rely on .custom-background as a helper class without inconsistent behavior. I’ve had to write my own function to determine when a background has really been customized:

<?php
function truly_customized_background( $classes ) {
	if ( get_background_image() || ( 'fafafa' !== get_theme_mod( 'background_color' ) ) ) {
		$classes[] = 'customized-background';
	}

	return $classes;
} // end function truly_customized_background
add_filter( 'body_class', 'truly_customized_background' );

The default background color for this theme is fafafa.

I'm not really sure what the best way of handling this would be, but I do know that as it stands now, the logic behind custom-background isn't reliable if it's supposed to mean "This site has a custom background or image set."

My very quick and lazy look into a way to handle this on the core side looked like this:

	if ( current_theme_supports( 'custom-background' ) ) {
		$args = get_theme_support( 'custom-background' );
		if ( get_theme_mod( 'background_color' ) === $args[0]['default-color'] ) {
			var_dump('mod is same as default');
		}
	}

So would stuff break if we changed the original custom-background logic to make sure that the theme mod exists and isn't the same as the default color declared by the theme?

Change History (8)

This ticket was mentioned in Slack in #core-themes by philip. View the logs.


10 years ago

#2 @philiparthurmoore
10 years ago

Regarding the custom function that I wrote above, this is actually a more reliable function that I'm using, which takes into account false for get_theme_mod( 'background_color' ) for new blogs with no theme mods:

<?php
/**
 * Ensure that the background is truly customized.
 *
 * @link https://core.trac.wordpress.org/ticket/30299
 */
function truly_customized_background( $classes ) {
	$background_color = get_theme_mod( 'background_color' );

	if ( get_background_image() || ( ! empty( $background_color ) && 'fafafa' !== $background_color ) ) {
		$classes[] = 'customized-background';
	}

	return $classes;
} // end function truly_customized_background
add_filter( 'body_class', 'truly_customized_background' );

#3 follow-up: @boonebgorges
10 years ago

  • Version trunk deleted

In part, this seems like a problem in semantics. You're interpreting "custom" to mean "something other than the default", while the current meaning of "custom" in core is "having been changed by the user". IMO, there's some value in the latter intepretation: if an admin has explicitly chosen to set a value for the background color, then it shouldn't matter what that color is. Presumably, you are then using the '.custom-background' class (or the lack thereof) to do some style overrides, and I would argue that you ought not to be doing these overrides if the admin has explicitly declared the color.

In any case, it appears that this is not a regression from 4.0, so I'm going to remove the 'Version' label to clear some of our pre-4.1 release reports. Thanks!

#4 in reply to: ↑ 3 @philiparthurmoore
10 years ago

Hi!

Replying to boonebgorges:

...if an admin has explicitly chosen to set a value for the background color, then it shouldn't matter what that color is. Presumably, you are then using the '.custom-background' class (or the lack thereof) to do some style overrides, and I would argue that you ought not to be doing these overrides if the admin has explicitly declared the color.

This is certainly understandable, but here's my thinking when I change background colors:

  1. Activate a new theme.
  2. Change the background color just to see what it would look like, playing around.
  3. Change things back to normal (default state, not customized).
  4. Expect things to be the way that they were before I changed my site to have a custom background color. This isn't possible if a theme leverages custom-background as it is now.

As it stands now, custom-background isn't a reliable helper class to figure out when a background has truly been customized. A background that is the same as a default hasn't really been customized. But you are totally right that it has been set by a user and then put back to normal. But I would think that in this case a user wants things to go back to the way they were before they set a custom background color. This is at least how I deal with it.

So if the logic for custom-background cannot be updated, would there be a case for customized-background as a new class? We need a way to figure out when a background has truly been customized away from the default and at present there does not appear to be a way to do that.

Thanks!

#5 @celloexpressions
9 years ago

  • Keywords 2nd-opinion added

Realistically, I see this as an example of why we probably shouldn't have classes like custom-background, since they don't easily align with the way that it works and also don't necessarily provide much benefit. Unless we want to clear the theme mod when it's set back to the default (which could maybe be done), but I'm not sure if there are advantages there (and also keep in mind the postMessage support in the customizer, which is probably why it sets the option to the default value). Any calls to get_theme_mod( 'background-color' ) should have the default hex color as a second argument to cover the default case. Should other CSS be changing when there's a custom background color?

This ticket was mentioned in Slack in #core by wido. View the logs.


8 years ago

This ticket was mentioned in Slack in #core-customize by melchoyce. View the logs.


8 years ago

#8 @melchoyce
8 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to maybelater
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.