Make WordPress Core

Opened 11 years ago

Closed 11 years ago

#23761 closed defect (bug) (fixed)

Empty header color after enabling header text via Customizer

Reported by: sergeybiryukov's profile SergeyBiryukov Owned by: nacin's profile nacin
Milestone: 3.6 Priority: normal
Severity: normal Version: 3.4
Component: Customize Keywords: has-patch commit
Focuses: Cc:

Description

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 11 years ago.
23761.diff (661 bytes) - added by obenland 11 years ago.
23761.1.diff (897 bytes) - added by obenland 11 years ago.
Adds comment

Download all attachments as: .zip

Change History (16)

#1 @kovshenin
11 years ago

  • Cc kovshenin added

#2 @agilexcmer
11 years 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.

#3 @lancewillett
11 years ago

  • Keywords has-patch commit added

#4 @lancewillett
11 years ago

  • Keywords has-patch commit removed

Ignore. :) Wrong ticket.

#5 follow-up: @lancewillett
11 years ago

  • Keywords reporter-feedback added

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

@obenland
11 years ago

#6 @obenland
11 years ago

  • Keywords has-patch added

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

@obenland
11 years ago

Adds comment

#7 @iamtakashi
11 years ago

  • Cc takashi@… added

#8 @aaroncampbell
11 years 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

#9 @aaroncampbell
11 years ago

  • Milestone changed from Future Release to 3.6

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

#10 in reply to: ↑ 5 @aaroncampbell
11 years 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.

#11 @nacin
11 years 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?

#12 @nacin
11 years ago

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

#13 @nacin
11 years 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.