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

For images, don't raise a validation error for loading="lazy" or decoding="async" #3940

Merged
merged 2 commits into from
Dec 17, 2019

Conversation

kienstra
Copy link
Contributor

@kienstra kienstra commented Dec 16, 2019

Summary

For images, this prevents validation errors if loading="lazy" or decoding="async", but raises an error for any other value. Uses Weston's snippet in #3938 (comment).

Fixes #3938

Using Weston's testing snippet in a Custom HTML block:

<img decoding="async" loading="eager" src="https://blog.amp.dev/wp-content/uploads/2019/12/AdobeStock_163856462-e1574813805126-1024x448.jpeg" alt="" class="wp-image-3003" width="1024" height="448">

...there is now only a validation error for loading="eager", not decoding="async":

valid-ealo

And there's no validation error for <img decoding="async" loading="lazy" ...>, though those aren't copied into the <amp-img>:

error-async

Sanitization behavior

<img> attribute value Validation error? Copied into <amp-img>?
loading lazy no no
loading anything other than lazy, like eager yes no
decoding async no no
decoding anything other than async, like sync yes no

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

For any other value, raise a
validation error.
Use Weston's snippet:
#3938 (comment)
@googlebot googlebot added the cla: yes Signed the Google CLA label Dec 16, 2019
@kienstra
Copy link
Contributor Author

Request For Review

Hi @westonruter,
Hope you're doing great.

Could you please review this PR when you have a chance?

Thanks, Weston!

@kienstra kienstra requested a review from westonruter December 17, 2019 00:09
@westonruter westonruter added this to the v1.5 milestone Dec 17, 2019
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.

Good work 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Signed the Google CLA
Projects
None yet
3 participants