Make WordPress Core

Opened 13 months ago

Closed 9 months ago

#23761 closed defect (bug) (fixed)

Empty header color after enabling header text via Customizer

Reported by: SergeyBiryukov Owned by: nacin
Milestone: 3.6 Priority: normal
Severity: normal Version: 3.4
Component: Appearance Keywords: has-patch commit
Focuses: Cc:


Background: #23722

  1. Go to Appearance > Header, disable header text.
  2. Go to Customizer, enable header text.
  3. Go to Appearance > Header again. get_header_textcolor() will return an empty value.

Bundled themes handle this situation differently:

If you disable and re-enable header text on Appearance > Header screen, the correct color will be restored.

Attachments (3)

23761.empty-header-color.png (159.7 KB) - added by SergeyBiryukov 13 months ago.
23761.diff (661 bytes) - added by obenland 13 months ago.
23761.1.diff (897 bytes) - added by obenland 13 months ago.
Adds comment

Download all attachments as: .zip

Change History (16)

comment:1 kovshenin13 months ago

  • Cc kovshenin added

comment:2 agilexcmer13 months ago

  • Cc agilexcmer added

This is for TwentyThirteen. In Line 217 of wp-admin/custom-header.php:

if ( isset( $_POST['text-color'] ) && ! isset( $_POST['display-header-text'] ) ) {
			check_admin_referer( 'custom-header-options', '_wpnonce-custom-header-options' );
			set_theme_mod( 'header_textcolor', 'blank' );
		} elseif ( isset( $_POST['text-color'] ) ) {
			check_admin_referer( 'custom-header-options', '_wpnonce-custom-header-options' );
			$_POST['text-color'] = str_replace( '#', '', $_POST['text-color'] );
			$color = preg_replace('/[^0-9a-fA-F]/', '', $_POST['text-color']);
			if ( strlen($color) == 6 || strlen($color) == 3 )
				set_theme_mod('header_textcolor', $color);
			elseif ( ! $color )
				set_theme_mod( 'header_textcolor', 'blank' );

If the header text is disabled, WP sets the colour option to the string "blank" (and also for cases where the colour is invalid). This is how it knows whether you have the header text set or not.

When the customizer loads, the backend will provide it with an invalid color (string "blank") to begin with. I think this is filtered somewhere to then become an empty string.

Enabling the header in the customizer shows the header color option, which deceptively shows a colour. The actual colour is invalid (just a "#"), but then the browser applies the default colour for anchor tags. When you save the header in the customizer, it saves it with an empty string colour (you can check this in the database, serialized value theme_mods_twentythirteen in wp_options).

In the Appearance->Header, the colour is again invalid, but the header anchor tag has a default colour blue, which is what you see. The default colour for the colour-picker's anchor is different, which is why it shows something whitish.

I think the problem comes from the fact that we're re-using the header colour to determine if the header is enabled or not.

comment:3 lancewillett13 months ago

  • Keywords has-patch commit added

comment:4 lancewillett13 months ago

  • Keywords has-patch commit removed

Ignore. :) Wrong ticket.

comment:5 follow-up: lancewillett13 months ago

  • Keywords reporter-feedback added

Can you test with patch on #22030 ? Could be related.

obenland13 months ago

comment:6 obenland13 months ago

  • Keywords has-patch added

Patch adds a check for an empty color string and uses the default text color in that case.

obenland13 months ago

Adds comment

comment:7 iamtakashi12 months ago

  • Cc takashi@… added

comment:8 aaroncampbell9 months ago

  • Milestone changed from 3.6 to Future Release

Since this isn't new and I don't think the offered solutions are really what we want, I'm punting this one out of 3.6

comment:9 aaroncampbell9 months ago

  • Milestone changed from Future Release to 3.6

Sorry, too many Trac windows open I guess. That was meant for a different ticket.

comment:10 in reply to: ↑ 5 aaroncampbell9 months ago

  • Keywords commit added; reporter-feedback removed

23761.1.diff works to avoid this issue. If you've already had the issue you'll need to save in the Customizer or Appearance->Header to fix it, but I doubt anyone really left it broke, so I think we're ok here.

Replying to lancewillett:

Can you test with patch on #22030 ? Could be related.

They don't seem to be related (background color and header text color), the patch from there has no effect here, and the solution seems to be different anyway because we have a theme specify a default header text color via add_theme_support( 'custom-header', $args ); whereas the default background color is supposed to be specified in the CSS.

comment:11 nacin9 months ago

This seems related to #22030, tangentially — in that case, we're saving the default when we should really be deleting/emptying the theme mod.

In this case, we're now *trying* to deliberately save the default. While it may work, is this the right fix?

comment:12 nacin9 months ago

I think Aaron is right here. I don't love it, but it's how custom-header.php works too.

comment:13 nacin9 months ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 24687:

Avoid empty header color after enabling header text via Customizer. props obenland, fixes #23761.

Note: See TracTickets for help on using tickets.