WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#21130 closed defect (bug) (fixed)

Custom Header Preview Display Issue

Reported by: JarretC Owned by: ryan
Milestone: 3.5 Priority: normal
Severity: normal Version: 3.4
Component: Themes Keywords: has-patch commit
Focuses: Cc:

Description

If you have a theme that utilizes the custom header function but a height value isn't defined the site title and site description clash Select Image table row.

The patch which adds height: auto !important; to #headimg should rectify this issue.

Screenshot shows the issue especially with a large amount of text used in the site description.

Attachments (8)

headerimage-preview.jpg (117.1 KB) - added by JarretC 7 years ago.
customheader-preview-fix.patch (353 bytes) - added by JarretC 7 years ago.
21130.twenty-eleven.png (24.9 KB) - added by SergeyBiryukov 7 years ago.
21130.patch (1023 bytes) - added by SergeyBiryukov 7 years ago.
21130-2.patch (991 bytes) - added by JarretC 7 years ago.
Header ‹ Beta — WordPress.png (34.1 KB) - added by JarretC 7 years ago.
21130.3.patch (1.3 KB) - added by SergeyBiryukov 7 years ago.
21130.4.patch (1.3 KB) - added by georgestephanis 7 years ago.
Better to save the result as $custom_header than calling get_custom_header() four times?

Download all attachments as: .zip

Change History (20)

#1 @SergeyBiryukov
7 years ago

  • Keywords reporter-feedback added; ui-feedback removed

Hmm, I can only reproduce this if a theme provides a custom admin preview callback, and sets a fixed height for #headimg in that function.

In Twenty Eleven, I tried removing height value from $custom_header_support array, setting it to 0 and removing flex-height as well. Looks fine in all cases: 21130.twenty-eleven.png.

Sounds like a particular theme issue.

#2 @JarretC
7 years ago

Twentyeleven issues callbacks to correct these issues. If you remove the height option, wp-head-callback, admin-head-callback and admin-preview-callback from Twentyeleven functions.php the issue shows up.

So most likely not an issue in most cases, but it does appear.

#3 @SergeyBiryukov
7 years ago

  • Keywords reporter-feedback removed

Ah, I see now. With the height option not set and the callbacks removed, we get this:

<div id="headimg" style="background-image:url();max-width:1000px;height:0px;">

In 3.3, with HEADER_IMAGE_HEIGHT not set (I guess it was required though) and the callbacks removed:

<div id="headimg" style="max-width:1000px;height:HEADER_IMAGE_HEIGHTpx;background-image:url();">

#4 @SergeyBiryukov
7 years ago

  • Component changed from UI to Themes
  • Milestone changed from Awaiting Review to 3.5

We could probably only add max-width and height attributes if they are not empty (21130.patch).

@JarretC
7 years ago

#5 follow-up: @SergeyBiryukov
7 years ago

The patch has an additional bonus of properly escaping the header image URL. Currently the esc_url() call is pointless, header_image() prints the value directly.

#6 @JarretC
7 years ago

Height issue still applies when a value is define, see screenshot. Setting height to auto fixes though not sure if best approach.

#7 @SergeyBiryukov
7 years ago

If a theme sets some value, I guess we should respect that for the most accurate results.

21130.3.patch adds overflow: hidden to prevent the overlapping.

#8 @JarretC
7 years ago

True, was thinking that originally although wasn't too keen on cutting off the text.

@georgestephanis
7 years ago

Better to save the result as $custom_header than calling get_custom_header() four times?

#9 @georgestephanis
7 years ago

Minor tweak, just pulled out a function call and stashed the result in a variable.

#10 @nacin
7 years ago

  • Keywords commit added

21130.4.patch looks good.

#11 in reply to: ↑ 5 @SergeyBiryukov
7 years ago

Replying to SergeyBiryukov:

The patch has an additional bonus of properly escaping the header image URL. Currently the esc_url() call is pointless, header_image() prints the value directly.

Related: #21433

#12 @ryan
7 years ago

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

In [21508]:

Fix display issues in the custom header screen when height is not specified. Use get_header_image() instead of header_image() so that esc_url() can do its job. Props JarretC, SergeyBiryukov, georgestephanis. fixes #21130 #21433

Note: See TracTickets for help on using tickets.