Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#32240 closed defect (bug) (fixed)

/theme-compat/header.php loads images from Kubrick in header, causing 404s if not using Kubrick

Reported by: shelob9's profile Shelob9 Owned by:
Milestone: 4.3 Priority: low
Severity: normal Version: 1.2
Component: Themes Keywords: good-first-bug has-patch
Focuses: template Cc:


This issue is super edge case, but using get_header() is not deprecated, so I am reporting.

In /theme-compat/header.php there is some inline CSS in a conditional that will output:
/images/kubrickbg-<?php bloginfo('text_direction'); ?>.jpg or #page { background: url("<?php bloginfo('stylesheet_directory'); ?>/images/kubrickbgwide.jpg") repeat-y top; border: none; }. This assumes your current theme has the same image directory as kubrick does, and if it doesn't causes a 404 error when the browser tries to load the image.

Attachments (2)

32240.patch (687 bytes) - added by Mte90 9 years ago.
32240.1.patch (689 bytes) - added by Mte90 9 years ago.

Download all attachments as: .zip

Change History (14)

#1 @obenland
9 years ago

  • Keywords needs-patch good-first-bug added

Nice find. Yes, I suppose a check if Kubrick is the current theme can't hurt.

#2 @Shelob9
9 years ago

@obenland Wouldn't it make more sense to do a file_exists() check?

#3 @Mte90
9 years ago

I want to try with this ticket as my first bug.
i think that the better solution is check the theme, check for the file is not a good idea for performance.
how i can simulate this bug? what is the theme for use this file?

#4 @Mte90
9 years ago

I think that for fix this the solution is replace the 27 line of theme-compat/header.php with

if ( wp_get_theme() !== 'Default' && empty($withcomments) && !is_single() ) {

#5 @SergeyBiryukov
9 years ago

  • Summary changed from /theme-compat/header.php loads images from Kurbick in header, causing 404s if not using Kubrick to /theme-compat/header.php loads images from Kubrick in header, causing 404s if not using Kubrick

#6 @Shelob9
9 years ago

@Mte90 No theme should be doing this, but if you create a theme without a header.php file, and then use get_header() it will pull in the /theme-compat/header.php file, which has references to image files that are in the Kubrick theme.

My problem with checking if Kubrick is the theme is that you can't do so reliably. get_stylesheet() is effectively a reference to the name of the directory that contains the active theme. If I install Kubrick in a directory called hats, then get_stylesheet() will return hats, not Kubrick, even though I'm using Kubrick.

In my opinion, the only reliable way to fix this issue, is to use a file_exists() or similar for 3 reasons:
1) The reason I just gave.
2) If Kubrick is in use, the file still could have been deleted.
3) A theme besides Kubrick could have that file in it, and be counting on it.

#7 @Mte90
9 years ago

with your explanation I think you're right. Someone can assign to me as a first good bug?

9 years ago

#8 @Mte90
9 years ago

  • Keywords needs-patch removed

This patch check if the file exist for print the style tag

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

9 years ago

#10 @johnbillion
9 years ago

  • Focuses ui removed
  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 4.3
  • Priority changed from normal to low

Thanks for the patch Mte90. All that needs correcting is the file_exists() check should use get_stylesheet_directory() instead of get_template_directory(), as that's what's used in the image URL in the CSS.

9 years ago

#11 @Mte90
9 years ago

patch updated with the suggestion

#12 @johnbillion
9 years ago

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

Fixed in r32496

Note: See TracTickets for help on using tickets.