Skip to content
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

Update class-amp-img-sanitizer.php #575

Closed
wants to merge 1 commit into from

Conversation

silverma
Copy link

If only one image dimension is set, the img sanitizer would ignore it and use the actual image sizes instead. This proposed change checks if only one dimension was set, and if so, calculates the other.

If only one image dimension is set, the img sanitizer would ignore it and use the actual image sizes instead. This proposed change checks if only one dimension was set, and if so, calculates the other.
@westonruter
Copy link
Member

See also #695.

@westonruter
Copy link
Member

@silverma Is this still relevant after #979?

@silverma
Copy link
Author

@silverma Is this still relevant after #979?

@westonruter for some reason I'm not able to reply to the comment above so replying here. I haven't looked at this in quite a while, so I'm not sure. If I remember correctly the specific problem was when the attribute wasn't included, not that it was there and set to 0. Also, the proposed change suggested to auto calculate the missing attribute. If those cases are covered, I'm guessing it should be good!

Copy link
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need unit tests prior to merging.

@westonruter westonruter closed this Jun 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants