Add native `img` to the AMP validator

[rbeckthomas]

Add native img tag to the AMP Validator

This issue falls under the umbrella of changes which are a part of the 'amp-img deprecation': #30442.

We will be adding the native img tag to the validator which will allow for native img tags which require the following attributes...

Standard image:

• alt
• decoding=async
• height
• loading=lazy
• src
• width

Hero image:

• alt
• data-hero
• decoding=async
• height
• src
• width

Image using srcset:

• alt
• decoding=async
• height
• loading=lazy
• sizes
• srcset
• width

Describe alternatives you've considered

These guidelines are fairly strict, we had considered having looser more granular validation rules for images trying to replicate specific amp-layout considerations, however for ease of development and for user understanding we have decided to start with more strict validation rules and allow for loosening of these restrictions in the future.

Additional context

For additional documents see this document about amp-img deprecation, and this document about allowing native img in valid amp documents.

[honeybadgerdontcare]

To be considered:

• do not allow for ancestors that are amp- elements
• do not allow for ancestors of template
• consider limitations regarding amp-bind

[kristoferbaxter]

1. do not allow for ancestors that are amp- elements

For a first revision yes, if an image has a amp- custom element ancestor, it must be a amp-img. This could be loosened in the future but would be a component by component change.

2. do not allow for ancestors of template

Sound reasonable for a first revision, can re-visit after first phase.

3. consider limitations regarding amp-bind

Which limitations are you thinking?

[honeybadgerdontcare]

1. consider limitations regarding amp-bind

Which limitations are you thinking?

We weren't exactly sure and it's an action item to determine if any need to exist. We plan to follow up with @jridgewell or @choumx to see if they thought this would be something to consider.

In all things validation, we prefer to be more strict at first and open up later once we're sure that's the direction we want. The intention here is to confirm that this is okay.

[honeybadgerdontcare]

1. do not allow for ancestors that are amp- elements

For a first revision yes, if an image has a amp- custom element ancestor, it must be a amp-img. This could be loosened in the future but would be a component by component change.

The Validator currently has a disallowed_ancestor [1] feature but not an allowed_ancestor. Is it a requirement to only allow some amp components to have this as a child when the time comes?

[1] https://github.com/ampproject/amphtml/blob/master/validator/validator.proto#L627

[Gregable]

We see a few somewhat common attributes added to img tags which aren't in this list. Do we want to allow any of these @rebeccanthomas / @kristoferbaxter ?

• border
• align
• vspace / hspace
• layout (I'm guessing this just just from the amp layout engine and we don't want it, but figured I'd ask).

[westonruter]

Can we also add importance?

[kristoferbaxter]

None of these seem problematic except for layout.

[westonruter]

There are a few others listed in MDN:

• crossorigin
• ismap
• referrerpolicy
• usemap

While align, hspace, and vspace are all deprecated, there are others which are also deprecated which don't seem harmful to allow:

• name
• longdesc

[banaag]

@kristoferbaxter could you approve the final list below? From the thread, this is what we folks want to add:

• border
• align
• vspace / hspace
• importance
• crossorigin
• ismap
• referrerpolicy
• usemap
• name
• longdesc

[kristoferbaxter]

Approved! Thanks @banaag.

[Gregable]

I'd like to remove longdesc from the list. It is an URL which might be fetched by some useragents, which might represent a leak of the user's ip / query in prefetch. I'm not certain this is the case, but I think the risk combined with very low usage and the fact that it's deprecated means we can safely leave it out.

[kristoferbaxter]

Makes sense @Gregable, no need for caches to implement custom rules for a deprecated attribute.

[banaag]

All the changes are checked-in. Should be uploaded to github during next validation rotation deployment.

[Gregable]

Now that this is done, and as per @honeybadgerdontcare 's mention, we should update external documentation before closing this. @rebeccanthomas, checking that is this on your radar.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.