#49824 closed defect (bug) (fixed)
Site Health instantiation prevents use of some hooks by plugins.
| Reported by: | peterwilsoncc | Owned by: | whyisjake |
|---|---|---|---|
| Priority: | normal | Milestone: | 5.4.1 |
| Component: | Site Health | Version: | 5.4 |
| Severity: | normal | Keywords: | commit |
| Cc: | Focuses: |
Description
As the WP_Site_Health class is instantiated prior to plugins being required and the plugins_loaded hook being fired, it prevents plugins from using the following hooks in the functions called by maybe_create_scheduled_event().
That is, each of the hooks in these functions:
update_option(relating to cron array)get_option(cron array)wp_get_scheduled_eventwp_schedule_eventwp_load_alloptions
Scanning the code, it looks like this has the potential to lead to false reports in WP_Site_Health::wp_cron_scheduled_check().
Is it possible to move the instantiation so it follows plugins_loaded (or even after_setup_theme to account for themes using features)?
I've moved this to the 5.4.1 milestone for consideration and visibility, sorry I didn't catch this during the release cycle.
Attachments (2)
Change History (14)
This ticket was mentioned in PR #216 on WordPress/wordpress-develop by peterwilsoncc.
7 years ago
#2
As the WP_Site_Health class is instantiated prior to plugins being required and the plugins_loaded hook being fired, it prevents plugins from using the following hooks in the functions called by maybe_create_scheduled_event().
That is, each of the hooks in these functions:
update_option(relating to cron array)get_option(cron array)wp_get_scheduled_eventwp_schedule_eventwp_load_alloptions
Scanning the code, it looks like this has the potential to lead to false reports in WP_Site_Health::wp_cron_scheduled_check().
Is it possible to move the instantiation so it follows plugins_loaded (or even after_setup_theme to account for themes using features)?
---
Initially PR moves to after after_theme_setup (needs testing though).
#3
in reply to: ↑ 1
@
7 years ago
Replying to Clorith:
Hmm, I don't think there's any problem with re-prioritizing it so that it instantiates later, did you have anything specific in mind?
Nothing specific. In the linked pull request, GH-216, I've moved it to immediately following the after_setup_theme hook. Largely at random but it now has access to pluggable functions, follows wp_cache_postload() if it's defined.
This ticket was mentioned in Slack in #core by whyisjake. View the logs.
7 years ago
#5
@
7 years ago
- Owner set to
- Status new → accepted
Not sure how to test this effectively, code looks good?
#6
@
7 years ago
49824.diff to bring the PR to the trac.
@whyisjake I tested as with the following and it worked as expected:
- Deleted transients from options table
wp transient delete --all - Emptied the Cron option in the options table
wp option delete cron - Logged in and hit the WP Dashboard
- Confirmed the cron job was added to the cron option
wp option get cron - Hit the site health page, waited for completion
- Back to the WP Dashboard to confirm the widget was operating as expected.
I also added this to a plugin to confirm the hook was being hit, it's rather blunt:
<?php add_filter( 'pre_get_scheduled_event', function ( $pre, $hook ) { if ( 'wp_site_health_scheduled_check' === $hook ) { var_dump( 'hehehe' ); exit; } return $pre; }, 10, 2 );
As always a logic check from another committer or a component maintainer would be dandy.
peterwilsoncc commented on PR #216:
7 years ago
#10
Merged in 47568(https://core.trac.wordpress.org/changeset/47568), 3a2e24bab4b77dccfa6c7ff425b739d9eaf53885
#11
in reply to: ↑ 9
@
7 years ago
Just noting the commit looks good to me too.
Site Health instantiation prevents use of some hooks by plugins.
As the WP_Site_Health class is instantiated prior to plugins being required and the
plugins_loadedhook being fired, it prevents plugins from using the following hooks in the functions called bymaybe_create_scheduled_event().
In the future though, I think it would be better if the commit message highlighted the solution and the changes made, rather than just the problem. The Commit Messages handbook article might be helpful.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
Hmm, I don't think there's any problem with re-prioritizing it so that it instantiates later, did you have anything specific in mind?