#59723 closed defect (bug) (fixed)
Pattern file containing PHP constant from theme causes fatal error on update to WordPress 6.4
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.4 | Priority: | high |
Severity: | normal | Version: | 6.4 |
Component: | Upgrade/Install | Keywords: | has-patch has-testing-info commit dev-reviewed fixed-major |
Focuses: | Cc: |
Description
I have a theme that sets up some PHP constants in its functions.php
file. These constants for example reference the dist directory of the built assets.
<?php define( 'NAMESPACE_THEME_TEMPLATE_URL', get_template_directory_uri() ); define( 'NAMESPACE_THEME_DIST_URL', NAMESPACE_THEME_TEMPLATE_URL . '/dist/' );
This theme then also contains some patterns in the patterns
directory.
Some of these patterns reference one of the globals defined in the functions.php
file to load an image from the dist folder.
<?php <?php /** * Title: Hero - Default * Slug: namespace/hero-default * Categories: header * Viewport Width: 500 */ ?> <!-- wp:cover {"url":"<?php echo esc_url( NAMESPACE_THEME_DIST_URL . 'images/placeholder/pattern--no-text.jpg' ); ?>","dimRatio":70,"minHeight":70,"minHeightUnit":"vh","contentPosition":"bottom left","isDark":false,"align":"full"} -->
This works great when just using the site. But when installing WordPress 6.4 RC1 or RC2 via CLI the update appears to run through but when I try to log into the Admin I get a blank white screen on the URL: /wp-admin/upgrade.php
When I look at the debug.log
file I get a fatal error saying that the constant used in the pattern file isn't defined.
So it appears the patterns are getting loaded before the theme is loaded.
Change History (32)
This ticket was mentioned in Slack in #core by fabiankaegy. View the logs.
16 months ago
#3
@
16 months ago
- Milestone changed from Awaiting Review to 6.4
- Owner set to joemcgill
- Priority changed from normal to high
- Status changed from new to reviewing
#4
@
16 months ago
- Milestone changed from 6.4 to Awaiting Review
- Priority changed from high to normal
I can replicate the error. If we use the theme shared, we get the below error.
PHP Fatal error: Uncaught Error: Undefined constant "NAMESPACE_IMAGE_URL" in ..\public\wp-content\themes\wp-core-59723-sample-theme-main\patterns\sample.php:9
#5
@
16 months ago
Also, I am able to replicate it, when I am trying to downgrade from 6.4 to 6.3.2. It seems that this is calling the patterns before the theme itself, whenever the DB update is required after an update.
Specifically, on this page: http://my-site.localhost/wp-admin/upgrade.php?_wp_http_referer=%2Fwp-admin%2F
#6
@
16 months ago
- Keywords needs-patch added
- Milestone changed from Awaiting Review to 6.4
- Priority changed from normal to high
- Status changed from reviewing to accepted
Thanks, @fabiankaegy. It looks like what is happening here is that the first time wp_get_active_and_valid_themes()
is run one the upgrade page, via this line, the function returns an empty array, which means the functions file isn't included.
Patterns are registered on init
and do not include a check for whether the functions file has already been loaded, but we should probably add an extra check here, so patterns aren't loaded whenever wp_installing()
returns true.
This ticket was mentioned in PR #5568 on WordPress/wordpress-develop by @joemcgill.
16 months ago
#7
- Keywords has-patch added; needs-patch removed
This fixes a bug on upgrade screens where constants defined in a theme's functions.php file are not defined before the patterns are loaded, leading to potential exceptions.
Trac ticket: https://core.trac.wordpress.org/ticket/59723
#8
follow-up:
↓ 12
@
16 months ago
Just a side query, why are we loading the patterns on the init instead of after_theme_setup? 🧐
The patterns are a part of that theme after all.
#9
@
16 months ago
After more investigation, it looks like this issue existed prior to the 6.4 release cycle, and was previously reported in #59020. It's possible this issue extends all the way back to WP version 6.0, when this was first introduced in [53152].
I'll close that ticket as a duplicate if we decide to move forward with this fix.
@rajinsharwar, to answer your question:
Just a side query, why are we loading the patterns on the init instead of after_theme_setup?
It's a good question...I'm not sure what the original rationale was for choosing that hook. However, it this case it wouldn't matter because the after_theme_setup
hook fires before init
in wp-settings.php
and the functions.php
file would still not be loaded in this case, leading to the same error.
#10
follow-up:
↓ 24
@
16 months ago
Steps to reproduce
- start w/ a fresh site running 6.3.2
- install/activate the sample theme in comment:2
- update to 6.4-RC2 via WP_CLI,
wp core update --version=6.4-RC
- check the PHP error logs, and notice
PHP Fatal error: Uncaught Error: Undefined constant "NAMESPACE_IMAGE_URL" in path-to-site/wp-content/themes/wp-core-59723-sample-theme-main/patterns/sample.php:9
Steps to test the PR
- roll the site back to 6.3.2,
wp core update --version=6.3.2 --force
- note: another error will be logged, you can ignore it the purposes of this testing
- update
wp-includes/block-patterns.php
in the WP_CLI ZIP file in~/.wp-cli/cache/core/wordpress-6.4-RC2-en_US.zip
so that the PR has been applied (i.e., add the early return from_register_theme_block_patterns()
ifwp_is_installing()
is true) - clear your PHP error log file
- update to 6.4-RC2,
wp core update --version=6.4-RC2
- check the PHP error logs and notice no error was logged
This ticket was mentioned in Slack in #core by pbiron. View the logs.
16 months ago
#12
in reply to:
↑ 8
@
16 months ago
Replying to rajinsharwar:
Just a side query, why are we loading the patterns on the init instead of after_theme_setup? 🧐
The patterns are a part of that theme after all.
I have the same question. Registering a theme's pattens on after_theme_setup
(w/ a priority of 20) seems to make much more sense. Maybe there's some underlying reason the editor needs them on init
?
#13
@
16 months ago
I can reproduce error. following comment:10
PHP Fatal error: Uncaught Error: Undefined constant "NAMESPACE_IMAGE_URL" in /Users/huzaifa/Project/wp/test/wp-content/themes/wp-core-59723-sample-theme/patterns/sample.php:9
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
16 months ago
#15
@
16 months ago
- Keywords changes-requested needs-testing has-testing-info added
Updating the keywords:
Patch: https://github.com/WordPress/wordpress-develop/pull/5568
needs-testing
is testing the patch, once it's ready.has-testing-info
: found in comment:10changes-requested
: discussion is happening in the patch of possible changes.
Alerted contributors in the Make/Core core channel with a call for testing and review.
Cross-sharing here:
👉 Should it go into 6.4 RC3?
IMO yes, fatals should be fixed. But there must be high confidence that the patch does indeed resolve the fatal without side effects.
"high confidence" comes from code reviews and test reports. The more test reports, the higher the confidence.
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
16 months ago
This ticket was mentioned in Slack in #core-upgrade-install by pbiron. View the logs.
16 months ago
This ticket was mentioned in Slack in #core by pbiron. View the logs.
16 months ago
16 months ago
#19
Other than the minor changes requested in the reviews I just posted, the PR and tests LGTM.
@joemcgill commented on PR #5568:
16 months ago
#20
Thanks @pbiron. These are great suggestions, which I've applied in d8f3501.
#21
@
16 months ago
@fabiankaegy if you have a chance, can you please test out the latest PR and see if it resolves the issue for you?
16 months ago
#22
I've confirmed that this fixes the bug and that the tests are testing what they intend to test and pass w/ TT3.
For future consideration: we might consider adding a test theme whose patterns assume that functions.php
has been loaded and use that theme in the tests.
16 months ago
#23
The PHPUnit Tests / PHP 7.4 / PHP 7.4 / MariaDB 10.11 multisite (pull_request)
test run errored out. Looks unrelated to this PR.
@joemcgill Are you able to restart the unit tests?
#24
in reply to:
↑ 10
@
16 months ago
Replying to pbiron:
Steps to reproduce
- start w/ a fresh site running 6.3.2
- install/activate the sample theme in comment:2
- update to 6.4-RC2 via WP_CLI,
wp core update --version=6.4-RC
- check the PHP error logs, and notice
PHP Fatal error: Uncaught Error: Undefined constant "NAMESPACE_IMAGE_URL" in path-to-site/wp-content/themes/wp-core-59723-sample-theme-main/patterns/sample.php:9
Steps to test the PR
- roll the site back to 6.3.2,
wp core update --version=6.3.2 --force
- note: another error will be logged, you can ignore it the purposes of this testing
- update
wp-includes/block-patterns.php
in the WP_CLI ZIP file in~/.wp-cli/cache/core/wordpress-6.4-RC2-en_US.zip
so that the PR has been applied (i.e., add the early return from_register_theme_block_patterns()
ifwp_is_installing()
is true)- clear your PHP error log file
- update to 6.4-RC2,
wp core update --version=6.4-RC2
- check the PHP error logs and notice no error was logged
Following these test steps I am also no longer able to reproduce the issue :)
#25
@
16 months ago
- Keywords changes-requested removed
I've cleaned up the unit tests for this fix to avoid looping through all of the theme patterns. I would like to get a +1 from another committer (cc: @peterwilsoncc or @hellofromTonya) before committing this to trunk
, and then will need that commit dev-reviewed in order to include this in the 6.4 branch.
In the meantime, I'd be happy for others to post the results of their testing, so we can demonstrate a high level of confidence (thanks, @pbiron and @fabiankaegy).
#27
@
16 months ago
- Keywords commit dev-reviewed added; needs-testing removed
Patch: https://github.com/WordPress/wordpress-develop/pull/5568
- Multiple approvals on the PR ✅
- Multiple test reports the PR resolves the issue ✅
- Confidence is high ✅ (though more test reports are encouraged and welcomed)
Marking the patch for commit
and, to expedite, dev-reviewed
for backport to 6.4 branch.
#29
@
16 months ago
- Keywords fixed-major added
- Resolution fixed deleted
- Status changed from closed to reopened
Reopening for the back porting to the 6.4 branch once tests have passed, given @hellofromTonya has already marked this as dev-reviewed
.
I have created a sample repository containing a minimal theme setup to reproduce the issue: https://github.com/fabiankaegy/wp-core-59723-sample-theme