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

insert_img_dimensions not detecting existing dimensions #7

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

Comments

Projects
None yet
1 participant
@GoogleCodeExporter

GoogleCodeExporter commented Apr 6, 2015

What steps will reproduce the problem?
1. use default config with core filters insert_img_dimensions is enabled in the 
core filters

What is the expected output? What do you see instead?
added width and height for images when those attributes are missing.
instead width and height are added even when they already exist.

What version of the product are you using? On what operating system?
ubuntu 10.04.1 LTS 32bit

Please provide any additional information below.


Original issue reported on code.google.com by hcrisw...@gmail.com on 3 Nov 2010 at 10:58

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Can you please post an example HTML file?

Original comment by sligocki@google.com on 3 Nov 2010 at 11:27

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Here is the HTML with insert_img_dimensions enabled.  In studying the HTML, I 
see my existing height and width attributes contain units the generated 
attributes don't.

Original comment by hcrisw...@gmail.com on 4 Nov 2010 at 6:53

Attachments:

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

@hcriswell, my guess is that they are parsing the page and looking for a valid 
width and height. The values for width and height can not contain "px" as you 
specify in your pre-mod_pagespeed file, so it probably thinks that there isn't 
a width or height specified and appends it anyway.

Original comment by roya...@gmail.com on 5 Nov 2010 at 3:49

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Thanks for the diagnoses @royanee, Jan, could this be the reason?

Original comment by sligocki@google.com on 5 Nov 2010 at 4:22

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

@royanee, that's absolutely correct.  We are deliberately conservative here, 
but we weren't quite conservative enough: we shouldn't be adding the redundant 
image dimensions if we don't understand the units they're expressed in (and as 
noted, no units => pixels).  I've fixed the code and added a test; what should 
happen in your case is that we'll leave the width and height in px as you 
specified.

Original comment by jmaes...@google.com on 5 Nov 2010 at 6:32

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

This should be fixed in the next push; it just got done testing.

Original comment by jmaes...@google.com on 5 Nov 2010 at 7:58

  • Changed state: Fixed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment