Opened 10 years ago
Closed 9 years ago
#32529 closed enhancement (fixed)
wp-admin/includes files should not contain hooks
Reported by: | wonderboymusic | Owned by: | wonderboymusic |
---|---|---|---|
Milestone: | 4.3 | Priority: | normal |
Severity: | normal | Version: | 4.3 |
Component: | Administration | Keywords: | has-patch |
Focuses: | Cc: |
Description
Following up on #30947
There are random actions and filters littered among files like misc.php
.
These files contain a bunch of functions that won't work outside of admin context and are typically only loaded in files that have already loaded the admin bootstrap.
These hooks should move to wp-admin/includes/admin-filters.php
and wp-admin/includes/ms-admin-filters.php
which can load where appropriate in wp-admin/includes/admin.php`.
A lot of these hooks (old media.php
) are essentially deprecated.
Attachments (2)
Change History (14)
#3
@
10 years ago
A few things I'd like to see here:
- Consolidate actions vs. filters
- Column align parameters to make them easier to identify
- Looks like two
network_admin_notices
need to be moved toms-default-filters.php
#4
@
10 years ago
32529.02.patch suggests the following:
- Alphabetize hooks by
$tag
- One empty line between hook types
- Two empty lines between hook groups
- Column align a few lines for improved readability
Patch also moves network_admin_notices
hooks to ms-default-filters.php
.
I have not tested actual hook priorities; this patch is largely cosmetic.
#7
@
9 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
This sounds good but broke outputting the Add Media button above the editor in non wp-admin context, see #33257.
Looking through the files from where the actions and filters were moved, several more are sometimes included in non wp-admin context. Some plugins may also include them outside wp-admin.
In that terms seems we should revert this to maintain back-compat.
#8
@
9 years ago
Since the Editor class is an edge case where we load an admin file potentially on the front end, wouldn't 32529.diff work?
#9
@
9 years ago
Yeah, that works. Don't even need to include ms-admin-filters.php.
Even simpler:
if ( ! function_exists( 'media_buttons' ) ) { include( ABSPATH . 'wp-admin/includes/media.php' ); add_action( 'media_buttons', 'media_buttons' ); }
The back-compat concern is mostly about plugins that include one of these files outside wp-admin. Also things like: https://core.trac.wordpress.org/browser/trunk/src/wp-includes/update.php#L198.
This ticket was mentioned in Slack in #core by ocean90. View the logs.
9 years ago
#11
@
9 years ago
Thinking that we can close this.
- https://core.trac.wordpress.org/attachment/ticket/33257/33257.diff fixes core and the known plugins use cases.
- Post on make/core and a mention in the email to plugin authors.
In 32653: