Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

<head> elements reordering #45

Closed
joeldbirch opened this issue Sep 20, 2014 · 13 comments
Closed

<head> elements reordering #45

joeldbirch opened this issue Sep 20, 2014 · 13 comments
Labels
bug tested/reproduced

Comments

@joeldbirch
Copy link

Hello,

I think this is a bug; it certainly seems counterintuitive.

Using Quick Cache Pro I have all the "HTML Compression Options" set to "yes" except for "remote resources" and "final HTML code". I am also listing exclusions for files that I don't want moving to the footer or compressing. These exclusions are being honoured correctly, except that the files they match are being moved into an order I do not want. I'm not sure if this is only related to the Quick Cache Pro plugin (if so please let me know and I'll post an issue there instead).

CSS Exclusion Patterns:
style.css

JavaScript Exclusion Patterns:
Typekit
modernizr.js

Prior to compression my <head> section is similar to this (simplified pseudo-code) and I'd like the element order to remain this way (hence the exclusions above):

<head>
    <script> /* inline Typekit loading JS, early as possible in source order */ (function(d){var config={kitId:'xxxxx... </script>
    <link rel="stylesheet" href="style.css">
    <script src="modernizr.js"></script>
</head>

Once the plugin has done its magic the<head> elements are ordered thusly:

<head>
    <script src="modernizr.js"></script>
    <link rel="stylesheet" href="style.css">
    <script> /* inline Typekit loading JS, early as possible in source order */ (function(d){var config={kitId:'xxxxx... </script>
</head>

Other files (not shown) are of course compressed and moved to the footer correctly. However, the head elements that I excluded have been reordered in a way I don't like. My expectation is that by excluding them from compression they should remain untouched in the order I left them.

Is this a bug? If not, what is the reason for the reordering and is there a workaround?

Thank you very much for your time.
@joeldbirch
Copy link
Author

I should note that those script and stylesheet elements are hardcoded in the HTML template. I just tested loading Modernizr via wp_enqueue_script and that caused the file to output in the original, correct position.

The inline Typekit script can't be loaded via wp_enqueue_script of course.

@jaswrks
Copy link

jaswrks commented Sep 20, 2014

@joeldbirch Thanks for the detailed report! Yes, it does seem like a bug. I have not been able to reproduce this yet, but those elements should simply be skipped over, the order should not be changing.

Do you have a URL where I can run the HTML Compressor on the HTML to see this happening? I don't need Dashboard access or anything, just a link to a page that contains the HTML that I can compress to see the order out of place.

@joeldbirch
Copy link
Author

Here is the html prior to compression. Obviously, none of the asset references will work, but it sounds like you don't need any of that. Hope it helps. Just let me know if you need anything else.

@joeldbirch
Copy link
Author

An additional note: I thought I could work around this by changing to the option "No, do not combine JS from <head> into fewer files", but the Modernizr script tag was still rendered above the CSS references. Then I added another change "No, do not combine CSS from <head> and <body> into fewer files." and finally the order was correct. Just some more data to consider.

@joeldbirch
Copy link
Author

Sorry. Previous comment edited to encode angle brackets.

@jaswrks
Copy link

jaswrks commented Sep 22, 2014

Issue identified. A <script> tag with only an src attribute may not always get picked up, causing the order to seem wrong, because the HTML Compressor is passing over some script tags without warning. Taking a closer look at these two methods now.

@jaswrks
Copy link

jaswrks commented Sep 22, 2014

A script tag without a language or type attribute should be assumed to contain JavaScript. The HTML Compressor is not getting this right in all scenarios.

@jaswrks
Copy link

jaswrks commented Sep 22, 2014

cc @raamdev This is now a known issue. Just a quick heads up :-)

@joeldbirch
Copy link
Author

Nice work! I'll add some type attributes until a fix is added. Thanks!

jaswrks pushed a commit that referenced this issue Sep 22, 2014
Bug fix and refactoring. See: #45
7aa44ab
@jaswrks
Copy link

jaswrks commented Sep 22, 2014

@joeldbirch Great, thanks again for reporting :-)

This bug has been fixed in the dev branch. @raamdev Will likely pull this into Quick Cache Pro soon. When he does (i.e. after you upgrade), here is something else to be aware of, just to help avoid confusion.

Given the example you provided above, with this input...

<head>
    <script> /* inline Typekit loading JS, early as possible in source order */ (function(d){var config={kitId:'xxxxx... </script>
    <link rel="stylesheet" href="style.css">
    <script src="modernizr.js"></script>
</head>

The expected output would be...

<head>
    <link rel="stylesheet" href="style.css">
    <script> /* inline Typekit loading JS, early as possible in source order */ (function(d){var config={kitId:'xxxxx... </script>
    <script src="modernizr.js"></script>
</head>

At first glance, this might still seem to be out of order. However, it is to be expected. The order of CSS and JS tags is preserved, but they are also grouped by the HTML Compressor to ensure that CSS (as a data classification) always comes first. Therefore, if you see your JavaScript (which was at the top), moved under CSS, this is normal. It's a best practice that we follow to improve page speed.

The order of the CSS and JS (within each respective group; i.e. classification) should remain preserved now though, even when a script tag does not include a type|language attribute.

@jaswrks
Copy link

jaswrks commented Sep 22, 2014

cc @raamdev I will push this fix into the next official release of the HTML Compressor. Coming soon.

@jaswrks jaswrks closed this as completed Sep 22, 2014
@jaswrks
Copy link

jaswrks commented Sep 22, 2014

@raamdev https://github.com/websharks/html-compressor/releases/tag/140922

@raamdev
Copy link
Contributor

raamdev commented Sep 22, 2014

@jaswsinc Thanks! The HTML Compressor will be updated with this fix for the next Quick Cache release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug tested/reproduced
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@raamdev @joeldbirch @jaswrks