#33388 closed defect (bug) (invalid)
WP_Theme should use get_file_data() for retrieving page templates
| Reported by: | dd32 | Owned by: | SergeyBiryukov |
|---|---|---|---|
| Priority: | normal | Milestone: | |
| Component: | Themes | Version: | |
| Severity: | normal | Keywords: | needs-testing has-patch needs-refresh needs-unit-tests |
| Cc: | Focuses: |
Description
Currently WP_Theme::get_page_templates() uses a direct regular expression on the result of get_file_contents(), it should instead use the get_file_data() helper method we have to interact with file header data.
Currently switching will result in a back-compat break, as some themes have been using headers such as <?php // Template Name: Something ?> as the header, which get_file_data() doesn't like - See #33387
See r21117 for where we previously did use it temporarily during the 3.4 cycle, but broke due to #33387
Attachments (4)
Change History (20)
#3
follow-up:
↓ 4
@
7 years ago
- Keywords close added
This ticket proposes a "should", but says it is not backward compatible. So why "should" it be changed?
It's not broken, so why change it?
#4
in reply to: ↑ 3
@
5 years ago
- Keywords has-patch added; close removed
- Milestone Awaiting Review → 5.9
- Owner set to
- Status new → reviewing
Replying to joyously:
This ticket proposes a "should", but says it is not backward compatible. So why "should" it be changed?
The backward compatibility concerns are addressed in [51182] / #33387, so this can now be addressed as well.
Why? Using an existing function instead of partially duplicating its functionality should make future maintenance easier.
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
5 years ago
#6
@
5 years ago
- Milestone 5.9 → 6.0
As per today's bug scrub, let's move this ticket to the next milestone for proper review/refresh.
@
5 years ago
Patch refreshed against trunk. Added missing {} in if statements, removed extraneous semi-colon + PHPCS fixes.
This ticket was mentioned in Slack in #core by mike. View the logs.
5 years ago
This ticket was mentioned in PR #2538 on WordPress/wordpress-develop by peterwilsoncc.
5 years ago
#8
@cosdev's patch https://core.trac.wordpress.org/ticket/33388
#9
follow-up:
↓ 14
@
5 years ago
- Keywords needs-refresh added
I created a pull request based in 33888.3.diff to get the tests running but there are few errors and failures that will need a little investigation.
This ticket was mentioned in Slack in #core by chaion07. View the logs.
4 years ago
peterwilsoncc commented on PR #2538:
4 years ago
#11
Closing this as it was just to get tests running.
#12
@
4 years ago
- Milestone 6.0 → Future Release
Per discussion in recent bug scrub, I'm moving this off 6.0.
Props @costdev, @chaion07
This ticket was mentioned in Slack in #core-test by sirlouen. View the logs.
14 months ago
#14
in reply to: ↑ 9
@
14 months ago
Replying to peterwilsoncc:
I created a pull request based in 33888.3.diff to get the tests running but there are few errors and failures that will need a little investigation.
Investigated and found the errors and fixed them with this patch https://core.trac.wordpress.org/attachment/ticket/33388/33888.4.diff
Test and see
#15
@
14 months ago
- Keywords needs-unit-tests added
- Resolution → invalid
- Status reviewing → closed
Test Report
Description
This report validates whether the indicated patch works as expected.
Patch tested:
33888.4.diff (1.3 KB) - added by mosescursor 8 minutes ago.
Patch fixes the incorrect variable check when reading the "Template Post Type" header in theme files. Changed from $header to $type so declared post types are correctly applied.
Environment
- WordPress: 6.9-alpha-60093-src
- PHP: 8.2.29
- Server: nginx/1.29.0
- Database: mysqli (Server: 8.4.6 / Client: mysqlnd 8.2.29)
- Browser: Chrome 138.0.0.0
- OS: macOS
- Theme: Standard Template Theme 1.0, Legacy Template Theme 1.0
- MU Plugins: None activated
- Plugins:
- Test Reports 1.2.0
Actual Results
- ✅ Issue resolved with patch.
Additional Notes
After applying the patch, the template appears for the specified post types as expected.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
depends on #33387