Random Redirect: only declare the redirect function when nothing else has - #51037
Conversation
… has The module was removed from the plugin in 13.6 (#38310) and restored in 16.1 (#50940). While it was gone, some themes and plugins started shipping their own copy of jetpack_matt_random_redirect(). A theme's functions.php runs before after_setup_theme, where Jetpack loads module-extras.php, so the theme declares the function first and the plugin's copy fatals. Wrap the declaration and its hook in a function_exists() check. The declaration has to live inside the conditional rather than behind an early return: PHP binds unconditional top-level function declarations when the file is compiled, so a runtime check before one never gets the chance to prevent it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U3VxzdSXMePRtoxepyNGYZ
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Jetpack plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Code Coverage SummaryCoverage changed in 1 file.
|
chrisbliss18
left a comment
There was a problem hiding this comment.
The changes look good. Approved.
I added unit test coverage for this issue and other potential issues in Random Redirect in #51041. The unit test finds the issue in the current code and your fix satisfies the test. Please give it a review when you have a chance.
… has (#51037) The module was removed from the plugin in 13.6 (#38310) and restored in 16.1 (#50940). While it was gone, some themes and plugins started shipping their own copy of jetpack_matt_random_redirect(). A theme's functions.php runs before after_setup_theme, where Jetpack loads module-extras.php, so the theme declares the function first and the plugin's copy fatals. Wrap the declaration and its hook in a function_exists() check. The declaration has to live inside the conditional rather than behind an early return: PHP binds unconditional top-level function declarations when the file is compiled, so a runtime check before one never gets the chance to prevent it. Claude-Session: https://claude.ai/code/session_01U3VxzdSXMePRtoxepyNGYZ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Cherry-picked to |
Proposed changes
jetpack_matt_random_redirect()when nothing else already has, so the Random Redirect module no longer fatals on sites where a theme or plugin ships its own copy of the function.The module was removed from the plugin in 13.6 (#38310) and restored in 16.1 (#50940). While it was gone, some themes started carrying their own copy. A theme's
functions.phpruns beforeafter_setup_theme, where Jetpack loadsmodule-extras.php, so the theme declares the function first and the plugin's copy hitsCannot redeclare jetpack_matt_random_redirect(). This was reported against 16.1-beta.2 with thepaneltheme active.The declaration has to sit inside the conditional rather than behind an early
return— PHP binds unconditional top-level function declarations when the file is compiled, so a runtime check placed before one never gets the chance to run.Reviewing with whitespace hidden (
?w=1) shows the real change: 8 added lines. Everything else is the existing function being indented one level.Related product discussion/links
p1785868165462669-slack-C01U2KGS2PQ
Does this pull request change what data or activity we track or use?
No.
Testing instructions
The redeclare only happens when something else declares the function first, so you need to simulate that.
On a test site running this branch, confirm Random Redirect still works normally: visit
https://<site>/?randomon the front end. You should be redirected to a random published post.Drop this into
wp-content/mu-plugins/random-redirect-conflict.phpto stand in for a theme that ships its own copy:Reload any front-end page. On trunk this fatals with
Cannot redeclare jetpack_matt_random_redirect(). On this branch the site loads normally.Visit
https://<site>/?randomagain — you should seeconflicting copy ran, confirming Jetpack stepped aside and the other copy is the one running.Delete the mu-plugin and confirm
/?randomgoes back to redirecting to a random post.Note that when another copy wins, the
jetpack_random_redirect_enabledfilter added in #50940 no longer applies, since that filter lives in Jetpack's implementation. That is the intended trade-off: not fataling matters more than owning the behaviour on those sites.