Resource regeneration doesn't respect custom options

[GoogleCodeExporter]

Setup:
 - Set jpeg recompression quality to 
   default to 10.
 - Set jpeg recompression quality to 50
   for an images directory.
 - Set jpeg recompression quality to 100
   for an html directory.
 - Reference a jpeg in that images
   directory from an html file in that
   html directory.

When you load the html file it will rewrite the jpeg with the options from the 
html directory, quality 100.  If the image falls out of cache or, in a 
multi-server environment, is requested from different server, it will be 
regenerated using the specified default quality 10.

If there were another html directory with jpeg-quality set to 75, for example, 
this would be even messier.

At no point will the recompression quality of 50 be used.

The problem is that there are several options that affect filter application 
without changing the url, so mod_pagespeed can't always generate the resource 
correctly from just the url.  Making the url contain all of this information 
would be possible, but it would also mean that any time we added a filter than 
could be adjusted on a per-directory basis we would need to expand the urls and 
that they might become too long.

Because this is not a common configuration for mod_pagespeed and even then this 
bug would rarely surface in a single-server setup, it may not be very serious.  
But if you're using mod_pagespeed for multiple sites and you want to have 
different filter-options for different ones this might be a real problem.

One fix would be, as described above, expanding the urls to contain all 
information necessary to generate a resource.  Another would be to modify the 
resource flow to support custom options, in which case this bug would stop 
being a problem for a setup where mod_pagespeed is used for multiple sites with 
different filter-options.

(Warning: none of this has been experimentally tested; it's all based on 
reading the code for handle_as_resource which does not support custom options 
(unlike InstawebContext::InstawebContext(), which does.).)

Original issue reported on code.google.com by jefftk@google.com on 3 Jul 2012 at 8:33

[GoogleCodeExporter]

I recall that we noticed this issue in the past, but decided that it only came 
up in a rather strange use-case (changing image quality config per directory on 
a multi-server setup) and the result was not too bad (just used inconsistent 
options, rather than breaking the page, etc.)

However, this may be more significant for a user who can only configure 
mod_pagespeed through .htaccess. In this case, they may have no way to 
configure the default behavior and so it could be drastically different. 
Furthermore, with CSS files many options affect the output.

For example, if the system default is to have all filters turned off 
(passthrough mode) then and request for rewritten CSS files (that must be 
regenerated) will not have images cache extended or rewritten even if the user 
turned those options on in their .htaccess file.

It seems to me that we should at least be using custom options for the 
directory of the resource on regeneration. That could be inconsistent as well, 
but I think it's more likely to be what's expected than the current behavior. 
Especially for people who can only configure their sites via .htaccess files.

Original comment by sligocki@google.com on 3 Jul 2012 at 9:11

[GoogleCodeExporter]

The other thing that came up in offline discussion was that, however we end up 
addressing this, we need to be able to explain the behavior to our users in 
some cohesive fashion.  If we can't do that, we probably have a bad solution.

In Shawn's example above, it would be something like "make sure your resources 
have the same configuration as the page that references them."  Nice and 
simple.  We can describe "Or else what" if we like, but laying out the intended 
rules first is a good plan.

Original comment by jmaes...@google.com on 8 Jul 2012 at 1:20

[GoogleCodeExporter]

Requiring resources to have the same configuration as the page that references 
them, and then respecting directory-specific options in resource generation 
sounds good to me.  Working on this now.

Original comment by jefftk@google.com on 20 Jul 2012 at 3:21

[GoogleCodeExporter]

This issue was closed by revision r1706.

Original comment by jefftk@google.com on 23 Jul 2012 at 3:23

• Changed state: Fixed

[GoogleCodeExporter]

Original comment by j...@google.com on 26 Oct 2012 at 6:05

• Added labels: Milestone-v23