Make WordPress Core

Opened 13 years ago

Last modified 6 years ago

#17916 reopened defect (bug)

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

Reported by: cimmo's profile Cimmo Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.2
Component: Script Loader Keywords: dev-feedback has-patch
Focuses: Cc:

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 13 years ago.
before.png (167.7 KB) - added by Cimmo 13 years 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 13 years ago.
17916.patch (630 bytes) - added by SergeyBiryukov 13 years ago.

Download all attachments as: .zip

Change History (50)

#1 @Cimmo
13 years ago

  • 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.

#2 @Cimmo
13 years ago

  • Cc cimmino.marco@… added

#3 @Cimmo
13 years ago

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 13 years ago by Cimmo (previous) (diff)

#4 @kawauso
13 years ago

I can't reproduce on trunk.

#5 @Cimmo
13 years ago

Checked out trunk from today, perfectly reproducible.

#6 @dd32
13 years ago

  • 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.

#7 @Cimmo
13 years ago

  • Keywords reporter-feedback removed

#8 @dd32
13 years ago

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);

@Cimmo
13 years 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.

@Cimmo
13 years ago

#9 @albertolau
13 years 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');

#10 @SergeyBiryukov
13 years ago

  • 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.

#11 @Cimmo
13 years ago

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 13 years ago by Cimmo (previous) (diff)

#12 @SergeyBiryukov
13 years ago

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 12 years ago by SergeyBiryukov (previous) (diff)

#13 @Cimmo
13 years ago

Adding
wp_print_styles();

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

#14 @SergeyBiryukov
13 years ago

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.

#15 @Cimmo
13 years ago

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

#16 follow-up: @nacin
13 years ago

  • Milestone set to 3.3

Since we have wp_enqueue_scripts, we should do the printing.

#17 @nacin
13 years ago

  • Resolution duplicate deleted
  • Status changed from closed to reopened

#18 in reply to: ↑ 16 @SergeyBiryukov
13 years ago

Replying to nacin:

Since we have wp_enqueue_scripts, we should do the printing.

17916.patch adds wp_print_styles() on login_head.

#19 @SergeyBiryukov
13 years ago

  • Keywords has-patch added

#20 @Cimmo
13 years ago

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

#21 @Cimmo
13 years ago

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 13 years ago by Cimmo (previous) (diff)

#22 @SergeyBiryukov
13 years ago

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 13 years ago by SergeyBiryukov (previous) (diff)

#23 @nacin
13 years ago

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

#24 @Cimmo
13 years ago

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 13 years ago by Cimmo (previous) (diff)

#25 follow-up: @Cimmo
13 years 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

#26 in reply to: ↑ 25 ; follow-up: @Cimmo
13 years 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.

#27 in reply to: ↑ 26 @Cimmo
13 years 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

#28 @Cimmo
12 years ago

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

#29 @Cimmo
12 years ago

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

Thank you.

#30 @azaozz
12 years ago

  • 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.

#31 @SergeyBiryukov
12 years ago

  • Keywords 3.5-early added; 3.3-early removed

#32 @husobj
12 years ago

  • 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?

#33 follow-up: @husobj
12 years 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?

#34 in reply to: ↑ 33 ; follow-up: @SergeyBiryukov
12 years 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().

#35 in reply to: ↑ 34 ; follow-up: @husobj
12 years 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.

#36 in reply to: ↑ 35 ; follow-up: @SergeyBiryukov
12 years 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).

#37 in reply to: ↑ 36 @Cimmo
12 years 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?

#38 @SergeyBiryukov
12 years ago

  • 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

#39 @nacin
12 years ago

  • Keywords commit added; 3.5-early removed

This seems good.

#40 @nacin
11 years ago

  • 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.

#41 @nacin
10 years ago

  • Component changed from Administration to Script Loader

#42 @chriscct7
8 years ago

  • Keywords dev-feedback added

#43 @SergeyBiryukov
8 years ago

#33922 was marked as a duplicate.

#44 @diddledani
8 years ago

  • Keywords has-patch added; needs-patch removed

I independently came to the same conclusion as @SergeyBiryukov, and provided semantically the same patch in the duplicate issue #33922. I believe that this ticket should be marked as has-patch, so I've done that with this comment :-)

#45 @ocean90
8 years ago

#35896 was marked as a duplicate.

#46 @SergeyBiryukov
6 years ago

Note: login_head triggers print_admin_styles() since [36341].

Note: See TracTickets for help on using tickets.