WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 5 months ago

#17916 reopened defect (bug)

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

Reported by: Cimmo Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.2
Component: Script Loader Keywords: needs-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 3 years ago.
before.png (167.7 KB) - added by Cimmo 3 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 3 years ago.
17916.patch (630 bytes) - added by SergeyBiryukov 3 years ago.

Download all attachments as: .zip

Change History (45)

comment:1 Cimmo3 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.

comment:2 Cimmo3 years ago

  • Cc cimmino.marco@… added

comment:3 Cimmo3 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 3 years ago by Cimmo (previous) (diff)

comment:4 kawauso3 years ago

I can't reproduce on trunk.

comment:5 Cimmo3 years ago

Checked out trunk from today, perfectly reproducible.

comment:6 dd323 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.

Cimmo3 years ago

comment:7 Cimmo3 years ago

  • Keywords reporter-feedback removed

comment:8 dd323 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);

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

Cimmo3 years ago

comment:9 albertolau3 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');

comment:10 SergeyBiryukov3 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.

comment:11 Cimmo3 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 3 years ago by Cimmo (previous) (diff)

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

comment:13 Cimmo3 years ago

Adding
wp_print_styles();

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

comment:14 SergeyBiryukov3 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.

comment:15 Cimmo3 years ago

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

comment:16 follow-up: nacin3 years ago

  • Milestone set to 3.3

Since we have wp_enqueue_scripts, we should do the printing.

comment:17 nacin3 years ago

  • Resolution duplicate deleted
  • Status changed from closed to reopened

SergeyBiryukov3 years ago

comment:18 in reply to: ↑ 16 SergeyBiryukov3 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.

comment:19 SergeyBiryukov3 years ago

  • Keywords has-patch added

comment:20 Cimmo3 years ago

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

comment:21 Cimmo3 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 3 years ago by Cimmo (previous) (diff)

comment:22 SergeyBiryukov3 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 3 years ago by SergeyBiryukov (previous) (diff)

comment:23 nacin3 years ago

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

comment:24 Cimmo3 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 3 years ago by Cimmo (previous) (diff)

comment:25 follow-up: Cimmo3 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

comment:26 in reply to: ↑ 25 ; follow-up: Cimmo3 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.

comment:27 in reply to: ↑ 26 Cimmo3 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

comment:28 Cimmo3 years ago

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

comment:29 Cimmo3 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.

comment:30 azaozz3 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.

comment:31 SergeyBiryukov2 years ago

  • Keywords 3.5-early added; 3.3-early removed

comment:32 husobj2 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?

comment:33 follow-up: husobj2 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?

comment:34 in reply to: ↑ 33 ; follow-up: SergeyBiryukov2 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().

comment:35 in reply to: ↑ 34 ; follow-up: husobj2 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.

comment:36 in reply to: ↑ 35 ; follow-up: SergeyBiryukov2 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).

comment:37 in reply to: ↑ 36 Cimmo2 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?

comment:38 SergeyBiryukov23 months 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

comment:39 nacin22 months ago

  • Keywords commit added; 3.5-early removed

This seems good.

comment:40 nacin20 months 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.

comment:41 nacin5 months ago

  • Component changed from Administration to Script Loader
Note: See TracTickets for help on using tickets.