Jetpack settings: override module list & debugger page titles#46283
Jetpack settings: override module list & debugger page titles#46283
Conversation
Fixes JETPACK-1008 When registering the Module list page and the Jetpack Debugger page, we do not want the page to appear in the admin menu. This causes us to run into this issue: - https://core.trac.wordpress.org/ticket/57579 - #46214 ``` Deprecated: strip_tags(): Passing null to parameter #1 ($string) of type string is deprecated in …/wp-admin/admin-header.php on line 41 ``` Let's override the title when we're on those pages to avoid the deprecation warning and have a proper page title on those 2 pages. Co-authored-by: Weston Ruter <weston@xwp.co>
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
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: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage SummaryCoverage changed in 1 file.
Full summary · PHP report · JS report If appropriate, add one of these labels to override the failing coverage check:
Covered by non-unit tests
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a deprecation warning that occurs when loading the Jetpack Settings screen (module list page) and the Debugging Center page. These pages are registered as "hidden" admin pages (with empty parent slug) to prevent them from appearing in the admin menu, which causes WordPress's strip_tags() function to receive a null title parameter. The fix manually overrides the page titles using the current_screen action hook to provide proper titles for both pages.
Key Changes
- Added a new
override_page_title()method to set explicit page titles for hidden admin pages - Hooked this method to the
current_screenaction in theadd_actions()method - Added changelog entry documenting the bugfix
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| projects/plugins/jetpack/changelog/fix-missing-title-wp-admin-settings | Adds changelog entry documenting the deprecation notice fix |
| projects/plugins/jetpack/_inc/lib/admin-pages/class.jetpack-admin-page.php | Adds override_page_title() method and registers it on current_screen hook to set titles for jetpack_modules and jetpack-debugger pages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
projects/plugins/jetpack/_inc/lib/admin-pages/class.jetpack-admin-page.php
Outdated
Show resolved
Hide resolved
| global $title; | ||
| if ( 'admin_page_jetpack_modules' === $screen->id ) { | ||
| $title = __( 'Jetpack Settings', 'jetpack' ); // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited -- we override the title global only on this page. | ||
| } elseif ( 'admin_page_jetpack-debugger' === $screen->id ) { | ||
| $title = __( 'Debugging Center', 'jetpack' ); // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited -- we override the title global only on this page. | ||
| } |
There was a problem hiding this comment.
Can we use the admin_title filter instead of modifying the global? Or is that different than the title we are trying to modify here?
There was a problem hiding this comment.
We could, but since the filter is declared later (see https://github.com/WordPress/wordpress-develop/blob/48b2b65eb7f719936260b8ece17b8847d264eec5/src/wp-admin/admin-header.php#L89 ), we would still get the PHP warning.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
manzoorwanijk
left a comment
There was a problem hiding this comment.
This looks good to me now. Thanks
* Jetpack settings: override module list & debugger page titles Fixes JETPACK-1008 When registering the Module list page and the Jetpack Debugger page, we do not want the page to appear in the admin menu. This causes us to run into this issue: - https://core.trac.wordpress.org/ticket/57579 - Automattic/jetpack#46214 ``` Deprecated: strip_tags(): Passing null to parameter #1 ($string) of type string is deprecated in …/wp-admin/admin-header.php on line 41 ``` Let's override the title when we're on those pages to avoid the deprecation warning and have a proper page title on those 2 pages. Co-authored-by: Weston Ruter <weston@xwp.co> * changelog * Change hook registration to match static declaration See Automattic/jetpack#46283 (comment) --------- Co-authored-by: Weston Ruter <weston@xwp.co> Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/20262979158 Upstream-Ref: Automattic/jetpack@cab13aa
* Jetpack settings: override module list & debugger page titles Fixes JETPACK-1008 When registering the Module list page and the Jetpack Debugger page, we do not want the page to appear in the admin menu. This causes us to run into this issue: - https://core.trac.wordpress.org/ticket/57579 - Automattic/jetpack#46214 ``` Deprecated: strip_tags(): Passing null to parameter #1 ($string) of type string is deprecated in …/wp-admin/admin-header.php on line 41 ``` Let's override the title when we're on those pages to avoid the deprecation warning and have a proper page title on those 2 pages. Co-authored-by: Weston Ruter <weston@xwp.co> * changelog * Change hook registration to match static declaration See Automattic/jetpack#46283 (comment) --------- Co-authored-by: Weston Ruter <weston@xwp.co> Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/20262979158 Upstream-Ref: Automattic/jetpack@cab13aa
* Jetpack settings: override module list & debugger page titles Fixes JETPACK-1008 When registering the Module list page and the Jetpack Debugger page, we do not want the page to appear in the admin menu. This causes us to run into this issue: - https://core.trac.wordpress.org/ticket/57579 - Automattic/jetpack#46214 ``` Deprecated: strip_tags(): Passing null to parameter #1 ($string) of type string is deprecated in …/wp-admin/admin-header.php on line 41 ``` Let's override the title when we're on those pages to avoid the deprecation warning and have a proper page title on those 2 pages. Co-authored-by: Weston Ruter <weston@xwp.co> * changelog * Change hook registration to match static declaration See Automattic/jetpack#46283 (comment) --------- Co-authored-by: Weston Ruter <weston@xwp.co> Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/20262979158 Upstream-Ref: Automattic/jetpack@cab13aa
Fixes JETPACK-1008
Proposed changes:
When registering the Module list page and the Jetpack Debugger page, we do not want the page to appear in the admin menu. This causes us to run into this issue:
strip_tags(): Passing null #46214Let's override the title when we're on those pages to avoid the deprecation warning and have a proper page title on those 2 pages.
Other information:
Jetpack product discussion
Does this pull request change what data or activity we track or use?
Testing instructions:
mysite.com/wp-admin/admin.php?page=jetpack-debuggermysite.com/wp-admin/admin.php?page=jetpack_modulestitle: it should match the titles set in this PR.