Make WordPress Core

Opened 9 years ago

Closed 4 years ago

#30579 closed enhancement (wontfix)

wp_enqueue_style in footer

Reported by: spacedmonkey's profile spacedmonkey Owned by: kevdotbadger's profile kevdotbadger
Milestone: Priority: normal
Severity: normal Version: 2.6
Component: Script Loader Keywords: has-patch needs-testing
Focuses: template, performance Cc:

Description

If you call the wp_enqueue_style function after wp_head ( in a shortcode for example ) has been called, it puts the css in the footer. This is means a link tag is outputted out the buttom of the body tag. This is invalid html, as link tag can only be used in the head tag. To make it valid html, a style tag should be used.

Something like this

<style id='$handle-inline-css' type='text/css' media="$media">
     @import url('$src');
</style>

Attachments (2)

bug-fix-30579.diff (1.7 KB) - added by kevdotbadger 9 years ago.
Fixes 30579
bug-fix-30579.2.diff (1.7 KB) - added by JulienMelissas 9 years ago.
Added space around else statement using @kevdotbadger's patch.

Download all attachments as: .zip

Change History (26)

#1 @dcondrey
9 years ago

Just for reference because I wanted to look it up..

HTML 4.01 Specification
12.1.3 Specifying anchors and links
Although several HTML elements and attributes create links to other resources (e.g., the IMG element, the FORM element, etc.), this chapter discusses links and anchors created by the LINK and A elements. The LINK element may only appear in the head of a document. The A element may only appear in the body.

#2 @johnbillion
9 years ago

  • Focuses template removed
  • Keywords needs-patch good-first-bug added
  • Version changed from 4.0.1 to 2.6

#3 @kevdotbadger
9 years ago

The HTML5 spec says that <link> IS allowed in the body, but only if it contains both the rel and property attributes.

This line fails the HTML5 validator test (when placed in the body)
<link type="text/css" rel="stylesheet" href="style.css" />

However, this line passes
<link type="text/css" rel="stylesheet" property="stylesheet" href="style.css" />

HOWEVER, HTML4 doesn't have the property attribute. So I agree with the OP's proposal, using the more traditional <style> tag.

Last edited 9 years ago by kevdotbadger (previous) (diff)

@kevdotbadger
9 years ago

Fixes 30579

#4 @kevdotbadger
9 years ago

  • Focuses template added
  • Keywords has-patch needs-testing added; needs-patch removed

Actually, having dug into this a little more it seems that HTML5 doesn't allow the <style> tag outside the <head> (without using the scoped attribute, which isn't Xbrowser supported at the moment), so you need to use the <link> tag. However, as mentioned above, the <link> tag can only be used if it has both the rel and property attributes defined. HTML4 doesn't have a property attribute, so you need to use the <style> tag outside the <head>. We can use current_theme_supports('html5') to determine if we should use either <style> or <link>. This is what the patch uses.

TD:RD; HTML5 uses <link>, HTML4 uses <style>.

#5 @evasivesoftware
9 years ago

Is there any reason why we couldn't create a new <link> element in DOM and append it into <head>?

A <noscript> fallback could function as a method using kevdotbadger's patch.

#6 follow-up: @kevdotbadger
9 years ago

I think the point is that you're allowed <link>s in the body, so there might be a reason why a template author wants to load it in the <body>. It's another great WP inconsistency, you have the in-footer option when loading scripts, but that option is missing from this loader, dispute there being a method in the loader to do just that.

Another solution would be to ignore anything that's queued after wp_head(), and actually use an in-footer option to place <link>s in the footer. But I think that'll break backwards compatibility.

#7 in reply to: ↑ 6 ; follow-up: @evasivesoftware
9 years ago

Replying to kevdotbadger:

I think the point is that you're allowed <link>s in the body, so there might be a reason why a template author wants to load it in the <body>. It's another great WP inconsistency, you have the in-footer option when loading scripts, but that option is missing from this loader, dispute there being a method in the loader to do just that.

Another solution would be to ignore anything that's queued after wp_head(), and actually use an in-footer option to place <link>s in the footer. But I think that'll break backwards compatibility.

Good point, I just think it sounds slightly messy. Utilizing DOM to create the link and inject into <head> is valid with either HTML4/5 and seems more robust (still hacky, none-the-less)

As you mentioned, rejecting things queued after wp_head() would be ideal. I don't see any situation where a stylesheet cannot be registered before wp_head() in cases that they are required before in-footer. How much cleanliness should be sacrificed to support lower level developers?

#8 in reply to: ↑ 7 ; follow-up: @spacedmonkey
9 years ago

Replying to evasivesoftware:

Good point, I just think it sounds slightly messy. Utilizing DOM to create the link and inject into <head> is valid with either HTML4/5 and seems more robust (still hacky, none-the-less)

As you mentioned, rejecting things queued after wp_head() would be ideal. I don't see any situation where a stylesheet cannot be registered before wp_head() in cases that they are required before in-footer. How much cleanliness should be sacrificed to support lower level developers?

I completely disagree with your point here. There are lots of reason to add css into the footer of a page. If for example, you are using a shortcode and you only want to output the css files if the shortcode is loaded. Removing styles that are loaded after wp_head, removes functionality and breaks compatibility with plugins and themes that may already be doing this. It is impractical to remove it now, it could break so much stuff.

wp enqueue style and wp register style functions should be have a in footer option, to bring them inline with wp enqueue script. If you call wp enqueue script after wp_head it just puts it in the footer, even if in footer isn't set to true.

kevdotbadger patch seems workable to me.

#9 in reply to: ↑ 8 ; follow-up: @evasivesoftware
9 years ago

Replying to spacedmonkey:

Replying to evasivesoftware:

Good point, I just think it sounds slightly messy. Utilizing DOM to create the link and inject into <head> is valid with either HTML4/5 and seems more robust (still hacky, none-the-less)

As you mentioned, rejecting things queued after wp_head() would be ideal. I don't see any situation where a stylesheet cannot be registered before wp_head() in cases that they are required before in-footer. How much cleanliness should be sacrificed to support lower level developers?

I completely disagree with your point here. There are lots of reason to add css into the footer of a page. If for example, you are using a shortcode and you only want to output the css files if the shortcode is loaded. Removing styles that are loaded after wp_head, removes functionality and breaks compatibility with plugins and themes that may already be doing this. It is impractical to remove it now, it could break so much stuff.

wp enqueue style and wp register style functions should be have a in footer option, to bring them inline with wp enqueue script. If you call wp enqueue script after wp_head it just puts it in the footer, even if in footer isn't set to true.

kevdotbadger patch seems workable to me.

You'r

Last edited 9 years ago by evasivesoftware (previous) (diff)

#10 in reply to: ↑ 9 @spacedmonkey
9 years ago

Replying to evasivesoftware:

Firstly, evasivesoftware There is no reason to be rude in comments here. Saying things like I am completely wrong is unhelp and unconstrutive.

In reply to your comments, yes wordpress code does support "lower level developers" whatever that means. It supports everyone and makes sure it is as backwards compatible as possible. Considering this functionality is already in core and has been for a long time, removing it isn't an option. Your reply regarding the shortcode is unclear, but seem to imply that the developer should change how they call the style not fix this issue. Getting every developer to change their behaviour isn't a workable solution, as it requires to much eduction and we would always miss some developers.

If I am wrong and it is a change to the core, can you please supply a patch.

#11 @kevdotbadger
9 years ago

  • Keywords dev-feedback added

This ticket was mentioned in Slack in #core by julienmelissas. View the logs.


9 years ago

@JulienMelissas
9 years ago

Added space around else statement using @kevdotbadger's patch.

#13 @JulienMelissas
9 years ago

Added patch to add space around else statement.

#14 @DrewAPicture
9 years ago

  • Owner set to kevdotbadger
  • Status changed from new to assigned

#15 @spacedmonkey
9 years ago

Is there any chance of this getting into 4.3?

#16 follow-up: @johnbillion
9 years ago

  • Keywords reporter-feedback added; good-first-bug dev-feedback removed

Can someone point me to the HTML5 spec where it states a link tag in the body needs a property attribute? Can't find it myself, and it seems weird that an arbitrary value like that implemented in bug-fix-30579.2.diff is of any use.

#18 @spacedmonkey
9 years ago

This is still an issue. Is there anyway of getting a fix put into 4.4? Has anyone tested the patch?

#19 @joehoyle
9 years ago

Looking at the nightly draft, the <link> is only allowed with the itemprop attribute as per http://www.w3.org/html/wg/drafts/html/master/single-page.html#the-link-element

#20 in reply to: ↑ 16 @afercia
8 years ago

Replying to johnbillion:

Can someone point me to the HTML5 spec where it states a link tag in the body needs a property attribute?

There's no such a thing :) Just to clarify, HTML 5.1 is still a Working Draft, not a Recommendation, so at the moment the only valid reference is HTML5 and there's a clear difference about the link element in the two specs:

HTML 5
http://www.w3.org/TR/html5/document-metadata.html#the-link-element

HTML 5.1
http://www.w3.org/TR/html51/semantics.html#the-link-element

#21 @peterwilsoncc
8 years ago

Related ticket for Chrome: Allow the page to render before <link> tags in body

We block rendering the entire page until the CSS has loaded. IE will allow content before the <link> to render, and block afterwards. Firefox is similar to IE, but overshoots slightly (the "Hulk Hogan" heading is after the <link>) revealing unstyled content.

IE is winning this one, can we copy them?

Developers are currently having to hack around our behaviour here, eg https://github.com/filamentgroup/loadCSS

Regardless of the spec, browsers can deal with CSS in the body as WP currently supplies it and are improving their behaviour. It's not pretty but letting devs learn is important.

#22 @cyman
8 years ago

#35195 was marked as a duplicate.

#23 @westonruter
4 years ago

  • Keywords close added

A lot changes in 6 years.

Adding an external stylesheet via link in the body is not invalid HTML anymore. Refer to MDN docs:

A <link> element can occur either in the <head> or <body> element, depending on whether it has a link type that is body-ok. For example, the stylesheet link type is body-ok, and therefore <link rel="stylesheet"> is permitted in the body. […]

Refer also to the HTML spec which states that stylesheet links are body-ok.

Try also running this HTML document through the HTML validator:

<!DOCTYPE html>
<html lang="en">
  <head>
    <title>Foo</title>
  </head>
  <body>
    <link rel="stylesheet" href="/foo.css">
  </body>
</html>

This results in:

Document checking completed. No errors or warnings to show.

Putting stylesheet links in the body has also been called the future of loading CSS.

I suggest closing this as being now invalid.

Last edited 4 years ago by westonruter (previous) (diff)

#24 @ocean90
4 years ago

  • Keywords reporter-feedback close removed
  • Resolution set to wontfix
  • Status changed from assigned to closed
Note: See TracTickets for help on using tickets.