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.

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

rel="preload" usage w/mod_pagespeed #225

Open
cadencelabs-master opened this Issue May 15, 2017 · 18 comments

Comments

Projects
None yet
8 participants
@cadencelabs-master

cadencelabs-master commented May 15, 2017

Newer versions of mod_pagespeed will strip <link rel="preload" hints from the page and turn them into preload headers ... you can disable this feature, however doing so also disables pagespeed's ability to optimize the CSS.

I'm just curious -- have you run into this issue yet? I was thinking of modifying the preload polyfill to load <link tags with some other rel, e.g., rel="pagespeed-preload" -- since the preload header is coming through on the response, it would be fine to add the styles into the page via js since in theory the browser will already be loading the styles due to the preload header

@rsteinwand

This comment has been minimized.

Show comment
Hide comment
@rsteinwand

rsteinwand May 15, 2017

Sounds like it should be bugged with mod_pagespeed.

https://groups.google.com/forum/#!forum/mod-pagespeed-discuss

rsteinwand commented May 15, 2017

Sounds like it should be bugged with mod_pagespeed.

https://groups.google.com/forum/#!forum/mod-pagespeed-discuss

@cadencelabs-master

This comment has been minimized.

Show comment
Hide comment
@cadencelabs-master

cadencelabs-master May 15, 2017

Do you think so? They have explicit notes that this is intended behavior (replacing preload hints with page headers). The problem is that the loadCss recommended implementation uses preload tags in a way which mod_pagespeed doesn't perceive them (mod_pagespeed's opinion is that a preload tag won't later become a stylesheet tag).

cadencelabs-master commented May 15, 2017

Do you think so? They have explicit notes that this is intended behavior (replacing preload hints with page headers). The problem is that the loadCss recommended implementation uses preload tags in a way which mod_pagespeed doesn't perceive them (mod_pagespeed's opinion is that a preload tag won't later become a stylesheet tag).

@scottjehl

This comment has been minimized.

Show comment
Hide comment
@scottjehl

scottjehl May 15, 2017

Member

This is interesting. So does pagespeed replace the link with a rel="stylesheet" link so that it will end up applying to the page?

Member

scottjehl commented May 15, 2017

This is interesting. So does pagespeed replace the link with a rel="stylesheet" link so that it will end up applying to the page?

@cadencelabs-master

This comment has been minimized.

Show comment
Hide comment
@cadencelabs-master

cadencelabs-master May 15, 2017

It does not -- that's the problem. It thinks rel="preload" is just hinting to the browser to load the resource, but that it will be applied by some user action or javascript code.

I went ahead and opened a thread on pagespeed -- considering that onload="" is a valid attribute for <link tags I think their engine ought to look at that tag to see if the "preload" may become something else once it's loaded

cadencelabs-master commented May 15, 2017

It does not -- that's the problem. It thinks rel="preload" is just hinting to the browser to load the resource, but that it will be applied by some user action or javascript code.

I went ahead and opened a thread on pagespeed -- considering that onload="" is a valid attribute for <link tags I think their engine ought to look at that tag to see if the "preload" may become something else once it's loaded

@scottjehl

This comment has been minimized.

Show comment
Hide comment
@scottjehl

scottjehl May 15, 2017

Member

Seems like a good idea Thanks.

Member

scottjehl commented May 15, 2017

Seems like a good idea Thanks.

@scottjehl

This comment has been minimized.

Show comment
Hide comment
@scottjehl

scottjehl Jun 8, 2017

Member

@cadencelabs-master could you post a link to that issue? That'd be helpful to track

Member

scottjehl commented Jun 8, 2017

@cadencelabs-master could you post a link to that issue? That'd be helpful to track

@cadencelabs-master

This comment has been minimized.

Show comment
Hide comment
@cadencelabs-master

cadencelabs-master Jun 10, 2017

This was the post I made: https://groups.google.com/forum/#!topic/mod-pagespeed-discuss/6_JlG7Q8zO8

BTW, I've been using the below script as a work around. It allows you to use rel="pagespeed-preload" if using mod_pagespeed

Below JS supports link tags like this:
<link rel="pagespeed-preload" onload="this.rel='stylesheet'" as="style" href="/path/to/stylesheet.css" />

<script data-pagespeed-no-defer type="text/javascript"> !function (t) { if (t.loadCSS) { var e = loadCSS.relpreload = {}; e.pagespeed = function () { for (var e = t.document.getElementsByTagName("link"), r = 0; r < e.length; r++) { var n = e[r]; "pagespeed-preload" === n.rel && "style" === n.getAttribute("as") && (t.loadCSS(n.href, n, n.getAttribute("media")), n.rel = null) } }; e.pagespeed(); var pagespeedR = t.setInterval(e.pagespeed, 300); t.addEventListener && t.addEventListener("load", function () { e.pagespeed(), t.clearInterval(pagespeedR) }), t.attachEvent && t.attachEvent("onload", function () { t.clearInterval(pagespeedR) }); } }(this); </script>

cadencelabs-master commented Jun 10, 2017

This was the post I made: https://groups.google.com/forum/#!topic/mod-pagespeed-discuss/6_JlG7Q8zO8

BTW, I've been using the below script as a work around. It allows you to use rel="pagespeed-preload" if using mod_pagespeed

Below JS supports link tags like this:
<link rel="pagespeed-preload" onload="this.rel='stylesheet'" as="style" href="/path/to/stylesheet.css" />

<script data-pagespeed-no-defer type="text/javascript"> !function (t) { if (t.loadCSS) { var e = loadCSS.relpreload = {}; e.pagespeed = function () { for (var e = t.document.getElementsByTagName("link"), r = 0; r < e.length; r++) { var n = e[r]; "pagespeed-preload" === n.rel && "style" === n.getAttribute("as") && (t.loadCSS(n.href, n, n.getAttribute("media")), n.rel = null) } }; e.pagespeed(); var pagespeedR = t.setInterval(e.pagespeed, 300); t.addEventListener && t.addEventListener("load", function () { e.pagespeed(), t.clearInterval(pagespeedR) }), t.attachEvent && t.attachEvent("onload", function () { t.clearInterval(pagespeedR) }); } }(this); </script>

@kwisatz

This comment has been minimized.

Show comment
Hide comment
@kwisatz

kwisatz Aug 12, 2017

I ran into this issue, but luckily found your thread.

kwisatz commented Aug 12, 2017

I ran into this issue, but luckily found your thread.

@Enalmada

This comment has been minimized.

Show comment
Hide comment
@Enalmada

Enalmada Aug 24, 2017

I just ran into this too. Is this a bug in pagespeed or loadCSS? Should we make sure mod-pagespeed has a ticket?

Enalmada commented Aug 24, 2017

I just ran into this too. Is this a bug in pagespeed or loadCSS? Should we make sure mod-pagespeed has a ticket?

@cadencelabs-master

This comment has been minimized.

Show comment
Hide comment
@cadencelabs-master

cadencelabs-master Aug 24, 2017

This is a bug in pagespeed, not loadCss

Do you know where to add that ticket? I started a discussion in the google group to no response from the Pagespeed Team

cadencelabs-master commented Aug 24, 2017

This is a bug in pagespeed, not loadCss

Do you know where to add that ticket? I started a discussion in the google group to no response from the Pagespeed Team

@Enalmada

This comment has been minimized.

Show comment
Hide comment

Enalmada commented Aug 24, 2017

@Enalmada

This comment has been minimized.

Show comment
Hide comment
@Enalmada

Enalmada Aug 24, 2017

@cadencelabs-master You mentioned in your first post that you can disable this pagespeed feature. Do you remember how? I'm trying to figure out what filters cause it and while testing...it seems like lots of important filters trigger it.

Enalmada commented Aug 24, 2017

@cadencelabs-master You mentioned in your first post that you can disable this pagespeed feature. Do you remember how? I'm trying to figure out what filters cause it and while testing...it seems like lots of important filters trigger it.

@cadencelabs-master

This comment has been minimized.

Show comment
Hide comment
@cadencelabs-master

cadencelabs-master Aug 24, 2017

@Enalmada Use the hack I posted above and change rel="preload" to rel="pagespeed-preload"

That will load all the deferred css via javascript (so you do sacrifice the native preload functionality) but it's the only way to make it work with Mod Pagespeed -- you'd need to disable any filter that affects CSS resources which would be way more harmful to performance

cadencelabs-master commented Aug 24, 2017

@Enalmada Use the hack I posted above and change rel="preload" to rel="pagespeed-preload"

That will load all the deferred css via javascript (so you do sacrifice the native preload functionality) but it's the only way to make it work with Mod Pagespeed -- you'd need to disable any filter that affects CSS resources which would be way more harmful to performance

@Enalmada

This comment has been minimized.

Show comment
Hide comment
@Enalmada

Enalmada Aug 24, 2017

@cadencelabs-master Ok well that hack seems to work, so +1Billion to you for posting that here for all us loadCSS pagespeeders.

Enalmada commented Aug 24, 2017

@cadencelabs-master Ok well that hack seems to work, so +1Billion to you for posting that here for all us loadCSS pagespeeders.

@oschaaf

This comment has been minimized.

Show comment
Hide comment
@oschaaf

oschaaf Aug 25, 2017

Hi all - I filed apache/incubator-pagespeed-mod#1621 for tracking this on the mod_pagespeed side

oschaaf commented Aug 25, 2017

Hi all - I filed apache/incubator-pagespeed-mod#1621 for tracking this on the mod_pagespeed side

@jmarantz

This comment has been minimized.

Show comment
Hide comment
@jmarantz

jmarantz Aug 25, 2017

Just to clarify: is this a performance issue or does this break observable page behavior? I'm trying to determine if PageSpeed can resolve the issue by learning whether the preloaded source ever got renamed in the recent past, and storing that in a server-side cache.

https://developer.mozilla.org/en-US/docs/Web/HTML/Preloading_content suggests that preload hints must be paired with rel=stylesheet link, and if the preload hint was stripped (eg due to cold server cache) it shouldn't break the page, just make it slower.

Does loadCSS do something to make the preload-link functionally required?

jmarantz commented Aug 25, 2017

Just to clarify: is this a performance issue or does this break observable page behavior? I'm trying to determine if PageSpeed can resolve the issue by learning whether the preloaded source ever got renamed in the recent past, and storing that in a server-side cache.

https://developer.mozilla.org/en-US/docs/Web/HTML/Preloading_content suggests that preload hints must be paired with rel=stylesheet link, and if the preload hint was stripped (eg due to cold server cache) it shouldn't break the page, just make it slower.

Does loadCSS do something to make the preload-link functionally required?

@Enalmada

This comment has been minimized.

Show comment
Hide comment
@Enalmada

Enalmada Sep 1, 2017

@cadencelabs-master I think you are the expert on this, can you comment on the question of performance vs breaking? My use case is a bit nonstandard...I was testing turbolinks plugin locally and found that it loads "rel=preload onload=..." css out of order with inlined css in the head. But I couldn't create a public test case for turbolinks (https://www.gell.com/turbolinks) because this pagespeed issue didn't allow me to reproduce the turbolinks issue on production.

Enalmada commented Sep 1, 2017

@cadencelabs-master I think you are the expert on this, can you comment on the question of performance vs breaking? My use case is a bit nonstandard...I was testing turbolinks plugin locally and found that it loads "rel=preload onload=..." css out of order with inlined css in the head. But I couldn't create a public test case for turbolinks (https://www.gell.com/turbolinks) because this pagespeed issue didn't allow me to reproduce the turbolinks issue on production.

@bekh6ex

This comment has been minimized.

Show comment
Hide comment
@bekh6ex

bekh6ex Dec 13, 2017

For those who ran into this issue and looking for solution, stripping can be disabled with following Apache config:

ModPagespeedPreserveSubresourceHints On
# or
ModPagespeedCssPreserveURLs On

bekh6ex commented Dec 13, 2017

For those who ran into this issue and looking for solution, stripping can be disabled with following Apache config:

ModPagespeedPreserveSubresourceHints On
# or
ModPagespeedCssPreserveURLs On
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment