apache / incubator-pagespeed-ngx

Automatic PageSpeed optimization module for Nginx
http://ngxpagespeed.com/
Apache License 2.0
4.37k stars 362 forks source link

Override webp lossless decision logic #1660

Open igmdvc opened 5 years ago

igmdvc commented 5 years ago

Hi,

I have been doing some tests on IPRO to be able to convert from PNG to webp lossless, but looking at the documentation it seems it is using the user agent to determine that capability. As I plan to have a CDN, it is not recommended to have Vary on User-Agent due to poor cache hit ratio.

I was wondering if there is a way to configure pagespeed to force the conversion to webp (lossless) just by detecting the webp support in the accept header, as some of the test images I am using are not converted when I use pagespeed AllowVaryOn Accept;. I know that this is not accurate enough as older versions of certain browsers don´t support lossless.

Thanks in advance.

jmarantz commented 5 years ago

Are you caching html in your cdn, out of curiosity?

I think the behavior you desire is what pagespeed does, iirc. And you are right as well that we cannot determine which features of webp are available with just the accept header so we need to compromise between exploiting new features and breaking images on older browsers that do not support them, but send the accept header anyway.

Probably the support level for various webp features has changed since this code was last updated, so we could possibly be more aggressive.

Can you say explicitly what you are requesting in this issue?

On Sun, Jul 28, 2019, 1:27 PM igmdvc notifications@github.com wrote:

Hi,

I have been doing some tests on IPRO to be able to convert from PNG to webp lossless, but looking at the documentation it seems it is using the user agent to determine that capability. As I plan to have a CDN, it is not recommended to have Vary on User-Agent due to poor cache hit ratio.

I was wondering if there is a way to configure pagespeed to force the conversion to webp (lossless) just by detecting the webp support in the accept header, as some of the test images I am using are not converted when I use pagespeed AllowVaryOn Accept;. I know that this is not accurate enough as older versions of certain browsers don´t support lossless.

Thanks in advance.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/apache/incubator-pagespeed-ngx/issues/1660?email_source=notifications&email_token=AAO2IPKDEZGHCCSK4JWGPODQBXJGXA5CNFSM4IHNAKJKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HB5DQXQ, or mute the thread https://github.com/notifications/unsubscribe-auth/AAO2IPJTP665E5TJKSPKHHDQBXJGXANCNFSM4IHNAKJA .

igmdvc commented 5 years ago

Nothing to do with HTML as all the images are fetched using ajax requests and are used for WebGL applications. For diverse reasons, we cannot modify the applications build process to generate webp on build time and that is why we want to use pagespeed.

I am requesting a specific configuration option to have lossless webp (if applicable) based on accept header without checking user-agent, so I can end up with response in which the vary header is Accept and not User-Agent. By enabling this option the user accepts as a side-effect, that there could be a request from an old version browser which receives a webp lossless and is not able to render properly due to not support it.

Hope it makes sense.

Thanks again

igmdvc commented 5 years ago

I am thinking about possible workarounds. Would it be possible to override the Vary: User-Agent header that pagespeed is adding into response with Vary: Accept within the NGINX configuration?

jmarantz commented 5 years ago

I guess the theme of this bug is to allow site owners to make their own judgement balancing their ability to exploit modern web features vs support of old browsers. Seems fair.

igmdvc commented 5 years ago

I am thinking about possible workarounds. Would it be possible to override the Vary: User-Agent header that pagespeed is adding into response with Vary: Accept within the NGINX configuration?

Just answering myself and probably others https://github.com/apache/incubator-pagespeed-ngx/issues/1107 and https://github.com/apache/incubator-pagespeed-ngx/issues/1149.

Instead of trying to force a specific order of different modules, the idea is to create a self proxy based on server block

server {
    server_name pagespeed.local; 
    listen 80;

    access_log off; 

    allow 127.0.0.1;
    deny all;

    # Pagespeed configuration
    pagespeed on;
    ...
}

Then you can proxy the files you want to be processed by pagespeed

proxy_pass http://127.0.0.1:80;
proxy_pass_request_headers on;
proxy_set_header Host pagespeed.local;

Finally you can rely on map blocks to process the response returned by the self proxy and use $upstreamhttp variables. For example, in my particular case:

map $upstream_http_Vary $custom_vary {
    default '';
    '~*User-Agent' 'Accept';
}