diff --git a/builtins/amp-img.js b/builtins/amp-img.js index 1593ab8ba730..8cc334756607 100644 --- a/builtins/amp-img.js +++ b/builtins/amp-img.js @@ -27,7 +27,8 @@ import {registerElement} from '../src/service/custom-element-registry'; const TAG = 'amp-img'; /** - * Attributes to propagate to internal image when changed externally. + * Attributes to propagate to internal image when changed externally. The + * `sizes` attribute is handled separately to prioritize `img-sizes`. * @type {!Array} */ const ATTRIBUTES_TO_PROPAGATE = [ @@ -39,7 +40,6 @@ const ATTRIBUTES_TO_PROPAGATE = [ 'aria-labelledby', 'srcset', 'src', - 'sizes', ]; export class AmpImg extends BaseElement { @@ -69,6 +69,30 @@ export class AmpImg extends BaseElement { this.sizesWidth_ = 0; } + /** + * @return {?string} The sizes value to use for the img, preferring + * `img-sizes` and falling back to `sizes`. + */ + getSizesValueToUse_() { + return ( + this.element.getAttribute('img-sizes') || + this.element.getAttribute('sizes') + ); + } + + /** + * If present, propagates the `img-sizes` or `sizes` value to the internal + * img. + */ + propagateSizes() { + const sizes = this.getSizesValueToUse_(); + if (sizes === null) { + this.img_.removeAttribute('sizes'); + } else { + this.img_.setAttribute('sizes', sizes); + } + } + /** @override */ mutatedAttributesCallback(mutations) { if (this.img_) { @@ -97,6 +121,9 @@ export class AmpImg extends BaseElement { this.img_, /* opt_removeMissingAttrs */ true ); + if (mutations['sizes']) { + this.propagateSizes(); + } guaranteeSrcForSrcsetUnsupportedBrowsers(this.img_); } } @@ -176,6 +203,7 @@ export class AmpImg extends BaseElement { } this.propagateAttributes(ATTRIBUTES_TO_PROPAGATE, this.img_); + this.propagateSizes(); guaranteeSrcForSrcsetUnsupportedBrowsers(this.img_); this.maybeGenerateSizes_(); this.applyFillContent(this.img_, true); @@ -194,7 +222,7 @@ export class AmpImg extends BaseElement { return; } // No need to generate sizes if already present. - const sizes = this.element.getAttribute('sizes'); + const sizes = this.getSizesValueToUse_(); if (sizes) { return; } diff --git a/builtins/amp-img.md b/builtins/amp-img.md index fa0b36b6315d..ef0676e79fa2 100644 --- a/builtins/amp-img.md +++ b/builtins/amp-img.md @@ -125,6 +125,10 @@ For the `` tag in `HTML`, the `sizes` attribute is used in conjunction with See [Responsive images with srcset, sizes & heights](https://amp.dev/documentation/guides-and-tutorials/develop/style_and_layout/art_direction) for usage of `sizes` and `srcset`. +**img-sizes** + +Like `sizes` above, sets the [`sizes`](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img#attr-sizes) attribute on the underlying `img`. Unlike `sizes`, this does not change the width of the `amp-img` and is only used by the browser for picking which source to use from the `srcset`. + **alt** A string of alternate text, similar to the `alt` attribute on `img`. diff --git a/test/unit/test-amp-img.js b/test/unit/test-amp-img.js index 0c25f28225f3..0958ae107230 100644 --- a/test/unit/test-amp-img.js +++ b/test/unit/test-amp-img.js @@ -188,6 +188,67 @@ describe('amp-img', () => { }); }); + it('should propagate srcset and img-sizes', async () => { + const ampImg = await getImg({ + src: '/examples/img/sample.jpg', + srcset: SRCSET_STRING, + 'img-sizes': '(max-width: 320px) 640px, 100vw', + width: 320, + height: 240, + }); + const img = ampImg.querySelector('img'); + expect(img.getAttribute('srcset')).to.equal(SRCSET_STRING); + expect(img.getAttribute('sizes')).to.equal( + '(max-width: 320px) 640px, 100vw' + ); + }); + + it('should prefer img-sizes to sizes', async () => { + const ampImg = await getImg({ + src: '/examples/img/sample.jpg', + srcset: SRCSET_STRING, + sizes: '(max-width: 320px) 640px, 100vw', + 'img-sizes': '640px', + width: 320, + height: 240, + }); + const img = ampImg.querySelector('img'); + expect(img.getAttribute('srcset')).to.equal(SRCSET_STRING); + expect(img.getAttribute('sizes')).to.equal('640px'); + }); + + it('should update sizes on mutation', async () => { + const ampImg = await getImg({ + src: '/examples/img/sample.jpg', + srcset: SRCSET_STRING, + sizes: '640px', + width: 300, + height: 200, + }); + const impl = ampImg.implementation_; + + ampImg.setAttribute('sizes', '50vw'); + impl.mutatedAttributesCallback({sizes: '50vw'}); + + expect(impl.img_.getAttribute('sizes')).to.equal('50vw'); + }); + + it('should ignore sizes mutations when img-sizes is used', async () => { + const ampImg = await getImg({ + src: '/examples/img/sample.jpg', + srcset: SRCSET_STRING, + 'img-sizes': '640px', + width: 300, + height: 200, + }); + const impl = ampImg.implementation_; + + ampImg.setAttribute('sizes', '50vw'); + impl.mutatedAttributesCallback({sizes: '50vw'}); + + expect(impl.img_.getAttribute('sizes')).to.equal('640px'); + }); + describe('#fallback on initial load', () => { let el; let impl; @@ -530,6 +591,23 @@ describe('amp-img', () => { }); }); + it('should not generate sizes for amp-imgs that already have img-sizes', async () => { + const ampImg = await getImg({ + src: '/examples/img/sample.jpg', + srcset: SRCSET_STRING, + 'img-sizes': '50vw', + width: 300, + height: 200, + }); + + const impl = ampImg.implementation_; + impl.buildCallback(); + await impl.layoutCallback(); + + const img = impl.img_; + expect(img.getAttribute('sizes')).to.equal('50vw'); + }); + it('should not generate sizes for amp-imgs with layout intrinsic', () => { let impl; return getImg({ diff --git a/validator/validator-main.protoascii b/validator/validator-main.protoascii index 347fce175197..2d526f298df8 100644 --- a/validator/validator-main.protoascii +++ b/validator/validator-main.protoascii @@ -5210,6 +5210,8 @@ tags: { # attrs: { name: "object-fit" } attrs: { name: "object-position" } attrs: { name: "placeholder" } + # Used to set sizes on the without AMP's sizes behavior + attrs: { name: "img-sizes" } # attrs: { name: "[alt]" } attrs: { name: "[attribution]" }