Opened 2 years ago

Last modified 7 months ago

#17916 reopened defect (bug)

Enqueued styles are only printed on login_footer in wp-login.php

Reported by: Cimmo Owned by:
Priority: normal Milestone: Future Release
Component: Administration Version: 3.2
Severity: normal Keywords: needs-patch
Cc: cimmino.marco@…, ben@…

Description

In my plugin I have this include:
wp_register_style("cimy_uef_register", $cuef_css_webpath."/cimy_uef_register.css", false, false);

now independently where and when I use the above css, even if I never enqueue or print it... it basically forces the admin panel to be always left instead of the right.

In particular it changes the admin panel menu #adminmenu to stick to the left, also other little things.
The question is:
why this happens even if the CSS is not included at all? Browsing the documentation quickly on that function I didn't find anything useful, please help me.

Attachments (4)

screw_rtl_plugin.php (341 bytes) - added by Cimmo 23 months ago.
before.png (167.7 KB) - added by Cimmo 23 months ago.
The freaking example works and you have also to add in the wp-config.php: $text_direction = 'rtl'; as said in the comments, if you don't believe me check these screenshots, before and after enabling the plugin I attached.
after.png (177.6 KB) - added by Cimmo 23 months ago.
17916.patch (630 bytes) - added by SergeyBiryukov 21 months ago.

Download all attachments as: .zip

Change History (44)

  • Version set to 3.2

Forgot to say that obviously I forced to be RTL in wp-config.php
$text_direction = 'rtl';

so I wouldn't expect that a simple wp_register_style screws up things.

  • Cc cimmino.marco@… added

One more funny thing:
even if I register a non existent css file, bug occurs, so I think is something in wp code :P

Last edited 2 years ago by Cimmo (previous) (diff)

I can't reproduce on trunk.

Checked out trunk from today, perfectly reproducible.

  • Keywords reporter-feedback added

Please attach a plugin which shows can be used to reproduce this on a stock installation, with no other plugins active.

Cimmo23 months ago

  • Keywords reporter-feedback removed

I'm closing this as worksforme for one reason: It works, and that example doesn't do anything at all (other than create a PHP notice due to the mis-use of the $cuef_css_webpath variable).

If you can provide an example which, on a stock install, ie. a clean install, can reproduce the problem, please post a followup comment

As a way of testing the php-notice impacts upon your install, you can test with this (which will do exactly as your attachment - nothing)

wp_register_style("cimy_uef_register", "404-style.css", false, false);

Cimmo23 months ago

The freaking example works and you have also to add in the wp-config.php: $text_direction = 'rtl'; as said in the comments, if you don't believe me check these screenshots, before and after enabling the plugin I attached.

Cimmo23 months ago

Cimmo, i have the same problem. until they figure out a solution you can try doing all your wp_register_style in a hook that is after load_textdomain

function proof_of_concept_init(){
	wp_register_style( 'debug-handle', 'doesntmatters/if/it/exists.css', array(),'1.0.0');
} 

add_action('after_setup_theme','proof_of_concept_init');
  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Like wp_register_script(), wp_register_style() also shouldn't be called before init, since locale isn't initialized yet (see #11526).

I've updated the patch in #11526 to include _doing_it_wrong() for $wp_styles.

I've also added a note in Codex with this ticket as a reference.

Ok my bad but still I don't get it when to use exactly wp_register_style and wp_enqueue_style
I need the css file in the login/registration pages only and I tried this code:

function cimy_uef_register_init_css() {
	global $cuef_css_webpath;
	wp_register_style("cimy_uef_register", $cuef_css_webpath."/cimy_uef_register.css", false, false);
	wp_enqueue_style("cimy_uef_register");
}

add_action('wp_print_styles', 'cimy_uef_register_init_css');

but works only for WordPress MS, not for non-MS. Using 'after_setup_theme' hook has same effect.

Last edited 22 months ago by Cimmo (previous) (diff)

For login/registration pages, I'd try login_head or login_enqueue_scripts action:

function cimy_uef_register_init_css() {
	global $cuef_css_webpath;
	wp_register_style("cimy_uef_register", $cuef_css_webpath."/cimy_uef_register.css", false, false);
	wp_enqueue_style("cimy_uef_register");
	wp_print_styles();
}

add_action('login_enqueue_scripts', 'cimy_uef_register_init_css');

Someone may correct me if I'm wrong, but it looks like we should call wp_print_styles() or wp_print_scripts() ourselves in this case.

Last edited 13 months ago by SergeyBiryukov (previous) (diff)

Adding
wp_print_styles();

Fixes the issue, but afaik that is not needed elsewhere, how is that?

wp_print_styles() is only called automatically for wp_head, not for login_head.

I guess that's for a reason, since most of the time custom styles should only be included on frontend.

but registration page is part of the front-end, anyways not a big deal, I fixed my issue, thank you!

comment:16 follow-up: ↓ 18   nacin21 months ago

  • Milestone set to 3.3

Since we have wp_enqueue_scripts, we should do the printing.

  • Resolution duplicate deleted
  • Status changed from closed to reopened

comment:18 in reply to: ↑ 16   SergeyBiryukov21 months ago

Replying to nacin:

Since we have wp_enqueue_scripts, we should do the printing.

17916.patch adds wp_print_styles() on login_head.

  • Keywords has-patch added

I applied the patch to WP 3.2.1 and does not fix the issue for me, still needs wp_print_styles();

Problem is in the patch:
add_action( 'login_head', 'wp_print_styles', 8 );

and when adding an action to login_head without priority this is set by default to 10 that means will be runned _after_ that.
So either the patch should have a higher number or I have to setup a lower one in my code.

Problem is: how authors can understand they have to use priority < 8 to have this working properly? You should document it somewhere.

Last edited 20 months ago by Cimmo (previous) (diff)

With the patch applied, the right hook for enqueuing styles would be wp_print_styles, not login_head.

I used the same priority as in wp_head:
http://core.trac.wordpress.org/browser/tags/3.2.1/wp-includes/default-filters.php#L211

Last edited 20 months ago by SergeyBiryukov (previous) (diff)

Actually, the best hook is wp_enqueue_scripts. There are equivalent login_ and admin_ versions as well.

With the patch applied, the right hook for enqueuing styles would be wp_print_styles, not login_head.

Not really if you want to add the css only in the registration page for example instead of spamming everything.

Actually, the best hook is wp_enqueue_scripts. There are equivalent login_ and admin_ versions as well.

Exactly! 'login_enqueue_scripts' is the right one, thanks.
I think patch can be applied then and documentation you'll find the best way to mention this I guess.

Last edited 20 months ago by Cimmo (previous) (diff)

comment:25 follow-up: ↓ 26   Cimmo20 months ago

nacin can we have the go for this patch? This bug is causing me issues
http://wordpress.org/support/topic/plugin-cimy-user-extra-fields-wrong-background-for-login-page

comment:26 in reply to: ↑ 25 ; follow-up: ↓ 27   Cimmo20 months ago

Replying to Cimmo:

nacin can we have the go for this patch? This bug is causing me issues
http://wordpress.org/support/topic/plugin-cimy-user-extra-fields-wrong-background-for-login-page

Actually a side effect of the patch is with Suffusion theme installed the login/registration page is totally screwed.
I think that the problem is on theme side. Will contact the author.

comment:27 in reply to: ↑ 26   Cimmo20 months ago

Replying to Cimmo:

Actually a side effect of the patch is with Suffusion theme installed the login/registration page is totally screwed.
I think that the problem is on theme side. Will contact the author.

Filed a ticket on their side:
http://aquoid.com/forum/viewtopic.php?f=2&t=6417

Suffusion issue fixed in version 3.8.9
Please apply this patch before 3.3 release!

Sorry to bother, but patch is not yet applied WordPress 3.3 beta3 any chance to get this before release candidates?

Thank you.

  • Keywords 3.3-early added
  • Milestone changed from 3.3 to Future Release

Still not clear what would be best there, the login screen is "special", not a part of the admin, not really a part of the front-end... And we don't want many scripts loading there for better security.

Think we need to talk/clarify the concept first before adding more auto-run stuff.

  • Keywords 3.5-early added; 3.3-early removed
  • Cc ben@… added

+1 for this patch

Works fine for me if correct hooks are used for queuing: login_enqueue_scripts / admin_enqueue_scripts / wp_enqueue_scripts

I guess it's just older themes/plugins using the wrong hooks that could potential affect styling?

comment:33 follow-up: ↓ 34   husobj10 months ago

Also note that wp_print_footer_scripts() is called via filter in the login footer anyway so why not load the scripts in the header like everywhere else?

comment:34 in reply to: ↑ 33 ; follow-up: ↓ 35   SergeyBiryukov10 months ago

Replying to husobj:

Also note that wp_print_footer_scripts() is called via filter in the login footer anyway so why not load the scripts in the header like everywhere else?

wp_print_head_scripts() is called as well, the patch adds wp_print_styles().

comment:35 in reply to: ↑ 34 ; follow-up: ↓ 36   husobj10 months ago

Replying to SergeyBiryukov:

Replying to husobj:

Also note that wp_print_footer_scripts() is called via filter in the login footer anyway so why not load the scripts in the header like everywhere else?

wp_print_head_scripts() is called as well, the patch adds wp_print_styles().

wp_print_footer_scripts() triggers an action
which calls _wp_footer_scripts()
which calls print_late_styles()

...so the styles get loaded in the footer of the login page anyway.

comment:36 in reply to: ↑ 35 ; follow-up: ↓ 37   SergeyBiryukov10 months ago

  • Milestone changed from Future Release to 3.5

Replying to husobj:

...so the styles get loaded in the footer of the login page anyway.

Indeed, since [18446].

So 17916.patch just makes it consistent with wp_print_head_scripts(), no new styles are added (which I guess was the concern in comment:30).

comment:37 in reply to: ↑ 36   Cimmo10 months ago

Replying to SergeyBiryukov:

So 17916.patch just makes it consistent with wp_print_head_scripts(), no new styles are added (which I guess was the concern in comment:30).

Also:
how a CSS could be of any security threat?

  • Summary changed from wp_register_style makes the RTL to be lost in some CSS files to Enqueued styles are only printed on login_footer in wp-login.php
  • Keywords commit added; 3.5-early removed

This seems good.

  • Keywords needs-patch added; has-patch commit removed
  • Milestone changed from 3.5 to Future Release

I think this patch revives http://make.wordpress.org/core/2011/12/12/use-wp_enqueue_scripts-not-wp_print_styles-to-enqueue-scripts-and-styles-for-the-frontend/. This is probably time to create a new function, such as wp_print_head_styles() (like wp_print_head_scripts), that does not accept a set of handles, and simply does the printing. Otherwise, calling wp_print_styles() in this case will fire the wp_print_styles hook.

Note: See TracTickets for help on using tickets.