Opened 13 years ago
Closed 13 years ago
#19510 closed defect (bug) (fixed)
wp_print_styles() causing mid-page scripts/styles to bleed into admin
Reported by: | AdamBackstrom | Owned by: | |
---|---|---|---|
Milestone: | 3.3.1 | Priority: | normal |
Severity: | major | Version: | 3.3 |
Component: | Editor | Keywords: | has-patch commit |
Focuses: | Cc: |
Description
I've been calling wp_enqueue_style() from the wp_print_styles action, as the wp_enqueue_style Codex page seemed to recommend. With the new wp_editor(), this causes styles intended for the template to bleed over into the admin. Example:
function trunktest_styles() { wp_enqueue_style( 'trunktest', site_url('?trunktest=1') ); } add_action( 'wp_print_styles', 'trunktest_styles' ); function trunktest_init() { if( isset( $_GET['trunktest'] ) ) { header( 'Content-type: text/css' ); echo 'body { background-color: red !important; }'; die(); } } add_action( 'init', 'trunktest_init' );
In WordPress 3.3 RC3, wp_editor() causes this style tag to be printed mid-way down the Dashboard page and various Comments pages. This behavior does not exist in WordPress 3.2.1, or on the majority of 3.3 RC3 pages.
Apologies if this is just an error in my usage of wp_enqueue_style().
Attachments (1)
Change History (22)
#2
@
13 years ago
- Keywords dev-feedback added
- Milestone changed from Awaiting Review to 3.3
- Severity changed from normal to blocker
- Summary changed from wp_editor() causing wp_print_styles to bleed into admin to wp_print_styles() causing mid-page scripts/styles to bleed into admin
The issue here is that we use wp_print_styles( $handle ) in wp_editor() and elsewhere to print scripts/styles directly.
This fires the wp_print_styles hook, which is incorrectly listed on the Codex as the best hook (that'd be wp_enqueue_scripts) and is referenced 965 times in the plugins directory.
Combine this with mid-page styles/scripts, and you have frontend styles leaking.
Possible solution: Re-tool print_admin_styles() to accept (and print) a handle, and change our wp_print_styles( $handle ) calls to print_admin_styles( $handle ).
#3
@
13 years ago
Or:
Only fire the wp_print_styles hook (which is in wp_print_styles()) for ! is_admin(). Unsure what that will break, though since we don't use wp_print_styles() (without a handle) in the admin, it should be pretty safe. Going forward, though, we need an admin version of this function.
#5
follow-ups:
↓ 6
↓ 8
@
13 years ago
Or we could make 'wp_print_styles' pass the handle as a parameter. Same for 'wp_print_scripts'.
#6
in reply to:
↑ 5
@
13 years ago
Replying to scribu:
Or we could make 'wp_print_styles' pass the handle as a parameter. Same for 'wp_print_scripts'.
That doesn't help the current regression, no?
#7
@
13 years ago
Or:
Avoid print_late_styles() for is_admin(). May want to consider the same for scripts.
#8
in reply to:
↑ 5
@
13 years ago
Replying to scribu:
Or we could make 'wp_print_styles' pass the handle as a parameter.
You mean if there's a style handle, we don't run the action? That may work:
function wp_print_styles( $handles = false ) { if ( empty($handles) ) do_action( 'wp_print_styles' ); ...
Thinking that even if we have a separate function for the admin, a plugin may still use wp_print_styles() in the admin. Seems choices are either to disallow enqueueing mid-page for styles or not run the hook in the admin.
#9
@
13 years ago
Avoid print_late_styles() for is_admin(). May want to consider the same for scripts.
Mid-page enqueues are new in 3.3 - if we can safely neuter it for 3.3 admin without affecting any core functionality, then this gets my vote.
We can revisit it in 3.4 if need be.
#10
follow-up:
↓ 12
@
13 years ago
Replying to azaozz:
function wp_print_styles( $handles = false ) { if ( empty($handles) ) do_action( 'wp_print_styles' ); ...
Yes, that would be a better fix for the problem I was thinking about:
We really use wp_print_scripts() in two ways:
a) to output all the styles enqueued so far, in which case the 'wp_print_styles' hook makes sense without a parameter.
b) as a wrapper for $wp_styles->do_items( $handle )
, in which it doesn't make sense to fire the hook, even with a parameter, due to the way it's already used in the wild.
+1.
#12
in reply to:
↑ 10
@
13 years ago
Replying to scribu:
We really use wp_print_scripts() in two ways:
a) to output all the styles enqueued so far, in which case the 'wp_print_styles' hook makes sense without a parameter.
b) as a wrapper for
$wp_styles->do_items( $handle )
, in which it doesn't make sense to fire the hook, even with a parameter, due to the way it's already used in the wild.
Right. The same is true for both wp_print_scripts()
and wp_print_styles()
. It doesn't make sense to run the hooks when used as shortcuts to $wp_styles->do_items()
and $wp_scripts->do_items()
. Until 3.3 this wasn't causing problems as these hooks didn't matter when run mid-page.
#13
@
13 years ago
Taking a step back here, This really only affects plugins/themes that don't use specific selectors.
I'd be willing to bet that most of the plugins using this hook, will be enqueuing CSS for specific classes - otherwise they'll be breaking more than just a few things on the front end.
A quick hotfix which should solve any problems, whilst hopefully not causing any others, would be something like this:
add_action( 'admin_init', function() { remove_all_actions('wp_print_styles'); }, 1);
that'll catch and remove any hooks that were probably meant for the front end.
#14
@
13 years ago
- Keywords regression removed
I don't think this is a critical / blocker issue for 3.3
We will (however much we hope we won't) find one or more similar issues once we release due to the extreme extensibility of WordPress and the 10000's of plugins that exist.
We should look to address this issue along with any others that come up in 3.3.1 and communicate the incorrect hook usage through codex updates and a wpdevel post.
We could also email plugin authors we know are making this mistake a heads up as the fix for them is simple.
Removing regression because as dd32 points out the bug was possible to trigger in previous releases we have just made it much easier to trigger.
#15
follow-up:
↓ 17
@
13 years ago
While the Hotfix plugin could solve things, this problem is very difficult to identify without a bit of skill. A user will need to inspect styles to figure out that they are being printed by an errant stylesheet. The blind "Re-activate plugins one by one" will often be needed to find the culprit.
I've seen a lot of situations where "Just install Hotfix" has been recommended even though the plugin will not address the problem. It muddies the plugin's usefulness -- it's best for when there is a targeted, specific error message that clearly shows that Hotfix needs to be used. And I'm not sure I like us relying on it to hit a deadline.
I think the best and most targeted fix here is to simply not fire the wp_print_styles hook in the admin. Then come 3.4, we stop using wp_print_styles() in the admin all together, and if it is called in the admin, we return print_admin_styles( $handle );
.
I've posted to wpdevel: http://wpdevel.wordpress.com/2011/12/12/use-wp_enqueue_scripts-not-wp_print_styles-to-enqueue-scripts-and-styles-for-the-frontend/. *We* know that *_enqueue_scripts is proper, but most good developers — and anyone following the Codex — will be finding that they've been doing it wrong. If we punt this to 3.3.1, I won't strongly object, but I think we will be making a mistake.
#17
in reply to:
↑ 15
@
13 years ago
19510.diff looks good for 3.3.1.
Replying to nacin:
...Then come 3.4, we stop using wp_print_styles() in the admin all together, and if it is called in the admin, we
return print_admin_styles( $handle );
.
Agreed, that seems the best solution for 3.4 (for both styles and scripts).
You're almost correct, in that wp_print_styles was usually only called on the front end in 3.2, it could be called from within the admin, Since WordPress 2.6 the function has been called in the admin if a CSS stylesheet is enqueued after the other stylesheets have already been printed, in 3.3 though, this happens when WP_Editor() is used anywhere too.
'wp_enqueue_scripts'
is a better hook for adding/enqueueing Styles/Javascript on the front end, whilst'admin_enqueue_scripts'
is available on the back-end.As you've noted though, It's quite common to suggest using
'wp_print_styles'
to enqueue CSS, this might be due to there being no'wp_enqueue_style'
action.. The codex examples all use this too.