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

Repeat image inlined at low resolution when local_storage_cache is enabled. #773

Closed
GoogleCodeExporter opened this Issue Apr 6, 2015 · 18 comments

Comments

Projects
None yet
1 participant
@GoogleCodeExporter
Copy link

GoogleCodeExporter commented Apr 6, 2015

From Wesley Ripley:

Hello,
I am using google mod_pagespeed with a very image heavy webpage. As a result I 
would like to have google pagespeed to do as much as possible to help the 
images load faster. One of the things that was helping was inlining the images 
but I have run into a problem. It is possible (and sometimes likely) for an 
image to be displayed on one page twice with different dimensions and I have 
found that inlining images does not work in this case. The first time it is 
used the image gets resized and inlined correctly but for every other 
occurrence it uses the same data string to inline the image so if the first one 
was tiny, the larger ones look very blurry. 

Is there any way to make pagespeed handle this correctly (besides resizing the 
images on my own and making them have different urls)? Also if you have any 
suggestions about the best way to handle many pictures per page that would be 
great.

Thank You

Original issue reported on code.google.com by sligocki@google.com on 3 Sep 2013 at 1:38

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Wesley, can you give us a link to a webpage that demonstrates this bug?

Original comment by sligocki@google.com on 3 Sep 2013 at 1:38

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Yes, it'd be really helpful to see a page that exhibits the bug.  I've 
attempted to reproduce this without success on the latest version from HEAD, 
and our cache keys have always included the dimensions of the underlying image.

Also, can you say what version of mod_pagespeed you're using (should be in the 
headers when you fetch a page)?

Original comment by jmaes...@google.com on 3 Sep 2013 at 2:04

  • Changed state: RequestClarification
@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

The version is 1.4.26.4-3396.  I will try and give you a sample page when I 
have time. I am just trying to figure out the best way to show it to you. Right 
not I have forbid inline_images in order to make sure the page works even for 
those who have filters cached (I assume this is how it works because when I 
simply disabled the filter it did not fix it correct me if I am wrong) If such 
a cache exists would it be reasonable to assume that anyone that had it cached 
no longer does so I can simply disable the filter and send you a url with the 
filter specifically requested? Its been forbidden for days now.

Thank You

Original comment by techw...@gmail.com on 4 Sep 2013 at 11:56

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

You should be able to explicitly enable image inlining for a page using query 
parameters:

http://www.example.com/my_page.html?PageSpeedFilters=+inline_images

You may need to shift-reload this url a couple of times to see the completely 
rewritten page.

Note that unless you're caching the rewritten html produced by mod_pagespeed, 
you should see filter changes as soon as your server re-reads the updated 
configuration.  Cached data includes flags identifying the configuration so 
that we *shouldn't* serve the wrong data when that configuration changes.  Let 
us know if you come up with a case where you can verify that this is happening, 
as that's definitely a bug.

In particular, if adding a query parameter as I suggested above changes the 
behavior of the same page without the query parameter, we'd like to know about 
that.

Original comment by jmaes...@google.com on 5 Sep 2013 at 12:49

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

For some reason the query parameters don't seem to be having any effect on what 
is generated. I even tried PageSpeed=off and it didn't seem to do anything.

I tried turning inline_images back on to see if the problem reoccured and it 
did. Here is exactly what I did:
1. Removed ModPageSpeedDisableFilters inline_images line in pagespeed.conf
2. Refreshed page multiple times until problem occurs. Two images on the page 
with different dimensions are inlined with the same data string (that of the 
smaller image) making the bigger image look blurry.
3. Refresh some more times to make sure the problem is persistent. It is.
4. Add back disable line
5. Refresh many more times so problem goes away (it does not go away 
immediately)

I have attached my exact configuration file if it helps. It may also help to 
know that I am using Apache 2.2.14 on Ubuntu with the Django web framework. 
While I am using Django to generate the pages, neither image's dimensions are 
dynamic in any way and are in the width and height attributes of the image tag.

Forgive me while I have a fair amount of experience as a web developer I have 
little experience in web server administration which is one of the reasons I 
choose PageSpeed in the first place. Besides this one problem it has been a 
life saving tool.


Original comment by techw...@gmail.com on 10 Sep 2013 at 2:13

Attachments:

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Forgot to add that between each change to the configuration (i.e. after steps 1 
and 4) I restarted apache

Original comment by techw...@gmail.com on 10 Sep 2013 at 2:17

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Thanks for the detailed writeup!

I fear I may have led you astray on the query params front by suggesting 
?PageSpeedFilters=+inline_images as that syntax is only supported in the beta.  
Try using the older syntax, ?ModPagespeedFilters=+inline_images and 
?ModPagespeed=off instead.  Sorry about that.

It'd still be helpful to have a page which exhibits the problem, as my attempts 
to gin something up haven't managed to reproduce it.  I understand if the page 
in question is something internal.  Absent a failing page, a few questions to 
help me try to reproduce this:

* You say the two images have different dimensions.  How are you specifying the 
dimensions?  Using width= and height= tags in the HTML?  Using a style="..." 
tag?  Using CSS?  Or some combination of the three?  (I'd been assuming the 
first case.)  If you only specify one of the two dimensions, which one are you 
specifying?

* When you turn off inlining and reload your page several times, you should see 
rewritten .pagespeed. image urls for the two images.  They'll look something 
like:
  http://www.example.com/my/path/100x50xMyimage.pagespeed.ic.hash.png
or
  http://www.example.com/my/path/xMyimage.ic.hash.png
What are you getting as the prefix of the url for these two images?  Do the 
dimensions match the ones you expect?

* If you inspect the images in your favorite browser debugger, do the 
dimensions match the ones you were expecting?  Sometimes these may look like 
they're off by 1 or 2, and it'd be useful to know if that's the case here – 
it generally happens for a reason, but takes some exploring.  (In Chrome, 
Developer Tools > Elements, highlight the image, look at metrics)

Original comment by jmaes...@google.com on 10 Sep 2013 at 1:56

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

I will still work on getting that sample page to you but for now here are the 
answers to you questions:
* I am using the width and height attributes on the image tag and I always 
specify both
* The bigger image is 150x150 and the url is this: 
http://mydomain.com/media/profile_pics/150x150x1_2013_05_06_17.jpg.pagespeed.ic.
mVf0_3g60Q.jpg
and the smaller picture is 30x30 and the url is this: 
http://mydomain.com/media/profile_pics/30x30x1_2013_05_06_17.jpg.pagespeed.ic.0k
mlNyd61r.jpg
The dimensions do seem correct and the images at these urls are of the right 
dimensions
* The dimensions of the smaller one are exactly right and I am pretty sure they 
are for the bigger one too. In Chrome dev tools when I hover over the bigger 
image url is says "152x152pixels (Natural 150 x 150 pixels)" However if I open 
the url, look at it in the Resources tab, or download the image the image is 
always 150 x 150 I am not sure what the hover thing is saying

Hope some of this helps though I have a feeling it wont since I feel as if all 
my answers were what you would have expected. If I made the error happen for a 
short period of time and saved the HTML of the page would that help? Or do you 
need the live page?

Original comment by techw...@gmail.com on 14 Sep 2013 at 9:37

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

In case it does help, I have attached the saved version of the page with the 
error occurring. I was also able to reproduce the error with the query 
parameters you gave. In order to get to the part of the site with the error you 
need to log in but I have created a test account so you can see what is going 
on. The website is peachplex.com and here is the account info:
Username: example
Password: pagespeedtestexample
Once you log in you will see the main page that has your profile picture (the 
google pagespeed logo) in both the main part of the screen and small in the 
upper right hand corner. If you then use this url: 
http://peachplex.com/?ModPagespeedFilters=+inline_images and refresh a few 
times you should see the big version become blury as described in the bug. When 
you have found the bug and no longer need this page let me know and I will 
delete the account.

I apologize for the speed of our site in general. We are currently not only 
having problems with pagespeed but possibly also our server provider. I am in 
the progress of trying to figure out where the performance issues are but for 
now it is very slow. I just wanted you to know that I realize the speed issue 
will not be completley resolved when you guys figure out how to fix this.

Original comment by techw...@gmail.com on 15 Sep 2013 at 7:16

Attachments:

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Has there been any progress on this? The other speed issue I mentioned in my 
last post has been resolved, so any improvements modpagespeed can make would 
really help out site.

Original comment by techw...@gmail.com on 23 Sep 2013 at 11:54

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

I've reproduced the bug using your test account, but haven't produced a local 
test case that'll yield the same behavior yet.  Sorry about the delay, didn't 
get to this last week.  Will give a shout when I can repro this locally; would 
appreciate leaving the test account up in the mean time.

Have changed the account password so it's not sitting open in our bug tracker.

Original comment by jmaes...@google.com on 24 Sep 2013 at 3:05

  • Changed state: Accepted
  • Added labels: Priority-High
  • Removed labels: Priority-Medium
@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

The problem is with local_storage_cache; I've reproduced it locally, and you 
can take down the temporary account if you like.  Thanks very much for working 
with us on this, it was a bit tricky to diagnose and looking at the live site 
and matching it with your config helped a great deal.

Your best short-term fix is to disable local_storage_cache in your 
configuration until this bug has been fixed and you've upgraded to the fixed 
version.

The important stimuli for reproducing this bug seem to be:
* local_storage_cache and rewrite_images on.
* Image on the page twice at different sizes, the smaller of which is below the 
inlining threshold.
* reload until local storage cache replaces all copies of the image with the 
low-res inlined version.

To fix this we need to include image dimension information in the local storage 
cache key.

Original comment by jmaes...@google.com on 24 Sep 2013 at 6:29

  • Changed title: Repeat image inlined at low resolution when local_storage_cache is enabled.
@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

To reproduce, put this in htdocs/mod_pagespeed_example/bad_image_inline.html:

<html>
  <head>
    <title>bad image inlining example</title>
  </head>
  <body>
    <img src="images/Puzzle.jpg" width="30" height="30"
         title="30x30 thumbnail, should resize OK."/><br/>
    <img src="images/Puzzle.jpg" width="150" height="150"
         title="150x150 version, should also resize and inline."/><br/>
  </body>
</html>

Then visit 
http://my.host/mod_pagespeed_example/bad_image_inline.html?PageSpeedFilters=rewr
ite_images,local_storage_cache&PageSpeedImageInlineMaxBytes=4000

This is broken in HEAD.

Original comment by jmaes...@google.com on 24 Sep 2013 at 6:32

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Oh, yes, and it's tricky to debug this if there's already full-resolution 
entries in your local storage cache (this happened to me).  Flush them using 
Chrome Dev Tools > Resources > Local Storage.  Interestingly it looks like 
we're adding two entries for the inlined image with different hash values.

Original comment by jmaes...@google.com on 24 Sep 2013 at 6:35

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Any progress on this?

Original comment by techw...@gmail.com on 30 Oct 2013 at 11:45

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

I think Matt's fixed this in HEAD.  If you can build from source, give it a 
whirl (re-enable local_storage_cache as well) and see if it fixes the problem.

Original comment by jmaes...@google.com on 31 Oct 2013 at 2:27

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Sorry, yes, it's fixed in head. If you can wait a few days/weeks we're prepping 
a new beta release,otherwise you can build from source.

Original comment by matterb...@google.com on 31 Oct 2013 at 12:09

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Original comment by sligocki@google.com on 31 Oct 2013 at 3:07

  • Changed state: Fixed
  • Added labels: Milestone-v30, release-note
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment