#58576 closed enhancement (fixed)
locate_template does not check to see site has parent theme.
| Reported by: | spacedmonkey | Owned by: | flixos90 |
|---|---|---|---|
| Priority: | normal | Milestone: | 6.4 |
| Component: | Themes | Version: | |
| Severity: | normal | Keywords: | good-first-bug has-patch commit |
| Cc: | Focuses: | performance |
Description
In locate_template, function checks to exist if a file exists in parent and child themes. But if the current theme is not a child theme, this check is pointless and checks if the same file exists twice per request.
So
if ( file_exists( STYLESHEETPATH . '/' . $template_name ) ) { $located = STYLESHEETPATH . '/' . $template_name; break; } elseif ( file_exists( TEMPLATEPATH . '/' . $template_name ) ) { $located = TEMPLATEPATH . '/' . $template_name; break; }
Should become
if ( file_exists( STYLESHEETPATH . '/' . $template_name ) ) { $located = STYLESHEETPATH . '/' . $template_name; break; } elseif ( STYLESHEETPATH !== TEMPLATEPATH && file_exists( TEMPLATEPATH . '/' . $template_name ) ) { $located = TEMPLATEPATH . '/' . $template_name; break; }
Attachments (2)
Change History (14)
#3
@
3 years ago
Patch is tested and it works as expected. However, due to use of STYLESHEETPATH and TEMPLATEPATH in locate_template we are unable write testcase around here.
STYLESHEETPATH and TEMPLATEPATH are set to default theme path well before setup() function in a test file.
We generally use switch_theme() inside a testcase to switch to a parent or child test themes.
Unfortunately, this doesn't effect the constants and so locate_template gives out the initial default value irrespective of active theme.
#4
@
3 years ago
I have added a new ticket https://core.trac.wordpress.org/ticket/58866#ticket for the bug found during writing testcases here.
#5
follow-up:
↓ 6
@
3 years ago
@nihar007 The patch looks good, one recommendation though: Instead of manually checking the condition, I think we should use the is_child_theme() function instead which already contains that logic.
An additional small benefit from using that will be that if we implement #58866 we would not need to update this logic here, but only the used function internally.
#6
in reply to: ↑ 5
@
3 years ago
Replying to flixos90:
@nihar007 The patch looks good, one recommendation though: Instead of manually checking the condition, I think we should use the
is_child_theme()function instead which already contains that logic.
An additional small benefit from using that will be that if we implement #58866 we would not need to update this logic here, but only the used function internally.
Hi @flixos90,
Thanks for your feedback. I am working on it
#7
@
3 years ago
- Keywords commit added
Thank you, @spacedmonkey, for creating this ticket and also, thanks to @nihar007 for providing the patch.
@thekt12, you are absolutely right. We cannot test the functionality with a constant in the unit test. Great catch on issue #58866.
The changes made in the patch look good and appear to be ready for deployment.
Adding the commit keyword to request further consideration. Since Jonny is currently on vacation, @flixos90, could you please handle the commit process for this ticket?
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
Patch added