apache / incubator-pagespeed-mod

Apache module for rewriting web pages to reduce latency and bandwidth.
http://modpagespeed.com
Apache License 2.0
696 stars 158 forks source link

Optimize page with no-transform header #1355

Open JoyceBabu opened 8 years ago

JoyceBabu commented 8 years ago

When a Google Search Engine detects that a user's connection speed is low, it automatically optimize the result pages by switching to Google Web Light proxy. In order to disable this feature, we have to add the Cache-Control:no-transform header.

We are detecting the user connection speed and automatically optimizing the pages for such users, by inlining the critical CSS and lazy loading the main CSS file. Unfortunately, when we add the no-transform header, PageSpeed also stops optimizing the page.

Is it possible to add a directive to stop PageSpeed from honouring the no-transform header? It would still be possible to disable pagespeed for a particular page by using the PageSpeed: off header.

oschaaf commented 8 years ago

There is the DisableRewriteOnNoTransform option [1] but it seems that it does not apply to html:

Please note that PageSpeed will always respect Cache-Control: no-transform headers on HTML files irrespective of the setting. 

So right now I think you can only achieve what you want here by somehow adding the no-transform header after mod_pagespeed runs. I don't know if it is possible to get mod_headers to run after mod_pagespeed does?

[1] https://developers.google.com/speed/pagespeed/module/configuration#notransform

jeffkaufman commented 8 years ago

This is a terrible hack, but you could use mod_proxy to loop back on yourself and add the no-transform header in the proxy.

JoyceBabu commented 8 years ago

Thank you @oschaaf, @jeffkaufman.

In both solutions, I have to add the header in apache configuration. Which means it is not easy to add complex logic for determining whether or not to add the no-transform header.

Also, I think I have read a post by Joshua that MPS should run after mod_headers.

I believe there is a valid use case for enabling MPS support, even when no-transform header is present.

jeffkaufman commented 8 years ago

It's possible we should just fully ignore no-transform if DisableRewriteOnNoTransform is off? It's in the doc, though, so it seems like this was a conscious choice when the feature was added. @jmarantz do you remember?

jmarantz commented 8 years ago

I do not remember. That was my first thought as well reading this thread... Why do we treat html differently?

I am curious though, what is the problem with web lite on slow connections for your site? Does it not improve things enough? Or break functionality?

On Jul 21, 2016 8:55 AM, "Jeff Kaufman" notifications@github.com wrote:

It's possible we should just fully ignore no-transform if DisableRewriteOnNoTransform is off? It's in the doc, though, so it seems like this was a conscious choice when the feature was added. @jmarantz https://github.com/jmarantz do you remember?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pagespeed/mod_pagespeed/issues/1355#issuecomment-234244736, or mute the thread https://github.com/notifications/unsubscribe-auth/AB2kPf18Ev2puDYYxypCYvEvTakG__8Vks5qX2xTgaJpZM4JRiJP .

JoyceBabu commented 8 years ago

For article pages, web lite is fine. But it breaks pages that depends on JS. For example, the following calculator requires user's location, which is auto detected from client IP. Changing the default requires JS. A web lite user is stuck with Mountain View as the location, and cannot change it.

http://googleweblight.com/?lite_url=http://m.prokerala.com/astrology/panchang/

Also, it removes all ads and re-inserts them at locations that sometimes severely degrade user experience.

Which is why I need the option to disable web lite (add no-transform header) on a case by case basis.

jmarantz commented 8 years ago

Looking at the change from 3 years ago that implemented this...prior to that change, we respected no-transform unconditionally. The change was to override no-transform specifically for resources, so I think HTML was simply not considered in the change.

I think we should update the doc & make DisableRewriteOnNoTransform apply to HTML.

On Thu, Jul 21, 2016 at 8:59 AM, Joshua Marantz jmarantz@google.com wrote:

I do not remember. That was my first thought as well reading this thread... Why do we treat html differently?

I am curious though, what is the problem with web lite on slow connections for your site? Does it not improve things enough? Or break functionality?

On Jul 21, 2016 8:55 AM, "Jeff Kaufman" notifications@github.com wrote:

It's possible we should just fully ignore no-transform if DisableRewriteOnNoTransform is off? It's in the doc, though, so it seems like this was a conscious choice when the feature was added. @jmarantz https://github.com/jmarantz do you remember?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pagespeed/mod_pagespeed/issues/1355#issuecomment-234244736, or mute the thread https://github.com/notifications/unsubscribe-auth/AB2kPf18Ev2puDYYxypCYvEvTakG__8Vks5qX2xTgaJpZM4JRiJP .

jeffkaufman commented 8 years ago

I wonder if there are people who are using DisableRewriteOnNoTransform on sites that have no-transform on both their html and resources, and are expecting the current behavior of respecting it on html?

JoyceBabu commented 8 years ago

If I comment out lines 132-135 of resouce.cc and recompile mod_pagespeed, would that disable the no-transform check?

https://github.com/pagespeed/mod_pagespeed/blob/99efdd19c1989bc10f20c68e7fdf6b30cdb284b1/net/instaweb/rewriter/resource.cc#L132-L135

crowell commented 8 years ago

yes, that looks like it should disable the check.

JoyceBabu commented 7 years ago

Are you planning on implementing this?