Admin Titles: avoid PHP warnings in PHP 8.1 #4308
Conversation
This avoids the following warning on pages where a title is not defined: ``` PHP Deprecated: strip_tags(): Passing null to parameter WordPress#1 ($string) of type string is deprecated in /wp-admin/admin-header.php on line 36 ``` This should not normally happen, but some folks may pass null to `add_submenu_page()` when creating new pages, to hide that page from the admin menu. This may not be the best approach, but it is one that is documented in the codex and used in the wild: https://developer.wordpress.org/reference/functions/add_submenu_page/#comment-445 - Related Core trac ticket: https://core.trac.wordpress.org/ticket/57579 - Related PR: WordPress#4084
|
|
||
| if ( ! is_null( $title ) ) { | ||
| $title = wp_strip_all_tags( $title ); // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited | ||
| } |
There was a problem hiding this comment.
Since wp_strip_all_tags() now handles null (see related trac ticket and commit d46dc08), I suppose we could do without the extra check, and only replace strip_tags by wp_strip_all_tags:
| if ( ! is_null( $title ) ) { | |
| $title = wp_strip_all_tags( $title ); // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited | |
| } | |
| $title = wp_strip_all_tags( $title ); // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited |
There was a problem hiding this comment.
HIya @jeherve !
IMO, the pulled fix is not the right approach/fix for the following reason: $title may still be null afterwards.
This means, the problem is now moved to subsequent functions which may be handling $title and using PHP native functions to do so, so this is just setting WP up for the next bug report instead of fixing the actual issue that $title should always be set and set to a string.
As for the second proposal (comment above), which I do think is better, there is one thing I'd like to point out about that:
- Yes, it will handle
nullsilently, but for anything scalar but non-string, that will now yield awarning, which is a significantly higher error level than the originaldeprecation(though justifiable).
In both cases, the // phpcs:ignore ... comment needs to be removed as that WPCS rule does not apply to WP Core and we should not add unnecessary noise like that to the codebase.
2 participants
This avoids the following warning on pages where a title is not defined:
This should not normally happen, but some folks may pass null to
add_submenu_page()when creating new pages, to hide that page from the admin menu. This may not be the best approach, but it is one that is documented in the codex and used in the wild: https://developer.wordpress.org/reference/functions/add_submenu_page/#comment-445