Opened 12 months ago
Last modified 2 months ago
#59365 new defect (bug)
Deprecated notice when calling get_admin_page_title() on some dashboard pages.
Reported by: | apedog | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | minor | Version: | |
Component: | Administration | Keywords: | php81 |
Focuses: | php-compatibility | Cc: |
Description
Deprecated preg_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated wp-admin/includes/plugin.php:2092 preg_replace() wp-admin/includes/plugin.php:2092 get_plugin_page_hookname() wp-admin/includes/plugin.php:2056 get_plugin_page_hook() wp-admin/includes/plugin.php:1980 get_admin_page_title()
This occurs on all pages where global $plugin_page
is null (Plugins, Tools, etc.)
Change History (9)
This ticket was mentioned in PR #5231 on WordPress/wordpress-develop by nasimcoderex.
12 months ago
#1
- Keywords has-patch added
#2
@
2 months ago
- Focuses coding-standards removed
- Keywords reporter-feedback php81 added
Hello @apedog,
Welcome back to WordPress Core Trac.
Can you please provide step-by-step instructions on how to reproduce the deprecation notice? Other information that can also help:
- What version of WordPress?
- What plugin(s) are in your stack?
- Are you seeing this issue with any plugin activated? If yes, are there specific URLs where it's happening?
- "Dashboard" are you meaning the plugin's landing page or the default WordPress Dashboard page?
Why am I asking for all of this information?
I'm not able to reproduce this in trying various plugins (BuddyPress, bbPress, Yoast SEO, Query Monitor, etc.) and navigating to their settings page. In each case, the ?page=[plugin_hookname]
is present in the URL.
Having the information requested helps contributors (like me) reproduce the issue as the starting point to understanding where to start with diagnosing where the root cause might be.
#3
follow-up:
↓ 6
@
2 months ago
Diving into the issue ...
The root case is not in the get_plugin_page_hookname()
function.
Why? It requires a non-nullable string
to be passed to its $plugin_page
parameter (see the documentation for more information). Passing null
to its $plugin_page
parameter is incorrect and the deprecation notice is correct if this happens.
This means, the root cause is elsewhere in the call stack and needs to be fixed upstream of this function. That could be in how a plugin is registering its settings page, something is unsetting or setting the global variable to null
, etc.
Are there clues in the call stack?
Possibly. The call stack (thank you for providing it):
get_admin_page_title()
is invoked.get_admin_page_title()
invokedget_plugin_page_hook()
and passed the global$plugin_page
variable to it.get_plugin_page_hook()
invokedget_plugin_page_hookname()
and passed$plugin_page
to it.
get_plugin_page_hook()
has no function parameters. Instead, it uses the global variables $title
and $plugin_page
. If $title
is not empty, then the title is returned, exiting out of the function. Else, get_plugin_page_hook()
is invoked, passing the global $plugin_page
variable to it. (More on this in a moment.)
get_plugin_page_hook()
also requires a non-nullable string
to be passed to its $plugin_page
parameter (see the documentation for more information).
So how would null
be passed to the functions?
Both of these need to be true:
- The global variable
$title
is empty or not set. - The URL does not have a
?page
query parameter.
As noted in the description, it happens when the global variable is null
.
This occurs on all pages where global
$plugin_page
is null (Plugins, Tools, etc.)
The Tools and Plugins admin page are built-in Core and have dedicated PHP files:
- Tools admin page is
admin/tools.php
and has the global$title
variable set to "Tools" within that file. - Plugins admin page is
admin/plugins.php
and has the global$title
variable set to "Plugins" within that file.
If the deprecation notice is happening on those admin pages, then it means some code somewhere incorrectly setting (to null
) or unsetting the global $title
variable.
@apedog Can you please confirm if the deprecation notice is being thrown on the Tools and Plugins admin page please?
@hellofromTonya commented on PR #5231:
2 months ago
#4
Thank you @nasimcoderex for the patch. However, it's not fixing the root cause. Let me explain (a more detailed explanation is here in the Trac ticket).
get_plugin_page_hookname()
requires a non-nullable string
to be passed to its $plugin_page
parameter (see the documentation).
Passing null
is incorrect. If null
is passed to it, then the PHP native preg_replace()
will throw the deprecation notice. The deprecation notice is informing of the incorrect data type received. (Currently) Core itself does not do incoming parameter data checks to ensure the incoming data is of the required type. Rather, it leans into it's documentation with the expectation of what's documented being the required data type(s) the function requires.
As this function is receiving a null
instead of a string
, the root cause is elsewhere. Thus, the fix is also elsewhere, i.e. where the root cause is (yet to be discovered).
I'm closing this PR. Once the root cause is found and if a Core fix needed, please consider opening a new PR to resolve it. Thank you 😄
#6
in reply to:
↑ 3
;
follow-ups:
↓ 7
↓ 8
@
2 months ago
Replying to hellofromTonya:
This means, the root cause is elsewhere in the call stack and needs to be fixed upstream of this function. That could be in how a plugin is registering its settings page, something is unsetting or setting the global variable to
null
, etc.
Hello Tonya,
The issue indeed was caused by a plugin registering a settings page.
More specifically,
I had used get_admin_page_title()
as the $title
parameter in add_settings_section()
. In hindsight, this is obviously wrong, as it would register a different title depending on the page being visited. However it would only print the title when I was on the correct settings page, so I thought nothing of it - less static strings for me to keep track of.
This worked fine up until I migrated to PHP 8+. I promptly fixed the "upstream" problem in my own plugin - and submitted the ticket at that time. I couldn't tell if this was indicative of a deeper issue or not.
@apedog Can you please confirm if the deprecation notice is being thrown on the Tools and Plugins admin page please?
Yes, the notice would appear on built-in core pages such as Tools and Plugins.
#7
in reply to:
↑ 6
@
2 months ago
Replying to apedog:
The issue indeed was caused by a plugin registering a settings page.
More specifically,
I had usedget_admin_page_title()
as the$title
parameter inadd_settings_section()
. In hindsight, this is obviously wrong, as it would register a different title depending on the page being visited. However it would only print the title when I was on the correct settings page, so I thought nothing of it - less static strings for me to keep track of.
Aha, that makes sense. add_settings_section()
needs the title you want to appear in that settings page section. Pass it as a string
or translatable string (e.g. through __()
).
Can you please confirm if the deprecation notice is being thrown on the Tools and Plugins admin page please?
Yes, the notice would appear on built-in core pages such as Tools and Plugins.
That makes now too. Thank you for confirming.
This worked fine up until I migrated to PHP 8+. I promptly fixed the "upstream" problem in my own plugin - and submitted the ticket at that time.
It should still work fine with PHP 8, as it's a deprecation notice rather than a Warning or Fatal Error. Glad you got it fixed in your plugin.
#8
in reply to:
↑ 6
;
follow-up:
↓ 9
@
2 months ago
- Keywords reporter-feedback removed
Replying to apedog:
I couldn't tell if this was indicative of a deeper issue or not.
While this specific issue is not a deeper issue within Core itself, it is an opportunity to improve the robustness and helpfulness of the code.
I'm thinking about if there's value in adding a defensive guard and a notice or deprecation message within get_admin_page_title()
for when both of the $title
and $plugin_page
global variables are not correct (see the code from 6.5.5). Maybe something like this:
if ( ! is_string( $plugin_page ) ) { wp_trigger_error( __FUNCTION__, 'Requires the $plugin_page global variable to be a non-nullable string data type. '. 'This global variable is automatically set when the admin URL contains a `page` query parameter.', E_USER_DEPRECATED ); $plugin_page = ''; } $hook = get_plugin_page_hook( $plugin_page, $pagenow );
This kind of defensive guard moves the detection further into the stack call, closer to the root of the problem. As in your case @apedog, it's within the Core function used that triggered the issue.
@apedog would a check have helped you to more quickly identify where the plugin was doing it wrong?
#9
in reply to:
↑ 8
@
2 months ago
I'm sorry, but I need to revise my previous reply.
While I did stumble across this issue when registering a settings page, the issue has nothing to do with registering settings.
The issue can be easily reproduced with this line of code:
add_action( 'admin_init', 'get_admin_page_title' );
So this is an issue in core. Especially since get_admin_page_title()
does return the correct string for the page being visited - even on core pages where said globals are null/not set.
ie.
add_action( 'admin_init', fn() => wp_die( get_admin_page_title() ) );
or (less destructively if using Query Monitor):
add_action( 'admin_init', fn() => QM::debug( get_admin_page_title() ) );
will return the string "Available Tools" when visiting the Tools page.
So function get_admin_page_title()
works fine on core pages (there is no "doing it wrong" here). However core throws deprecated notices. I don't think this is an upstream issue after all.
Ticket : https://core.trac.wordpress.org/ticket/59365
### Description:
Check the string before using preg_replace method in wp-admin/includes/plugin.php file
Trac ticket: