Skip to content

Commit 19fbdba

Browse files
sebastianbenzsebastianbenz
authored
Don't remove sizes for hero images (#1142)
* Don't remove sizes for hero images Workaround for ampproject/amphtml#32644. * generate default sizes attribute for hero images
1 parent 972aa9a commit 19fbdba

8 files changed

Lines changed: 62 additions & 4 deletions

File tree

‎packages/optimizer/lib/transformers/ApplyCommonAttributes.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,15 @@ class ApplyCommonAttributes {
249249
customStyles.children[0].data += styles;
250250
for (const node of this.transformedNodes) {
251251
for (const attribute of Object.keys(this.attributeTransformations)) {
252+
// Don't remove sizes from amp-img
253+
// workaround for https://github.com/ampproject/amphtml/issues/32644
254+
if (
255+
node.tagName === 'amp-img' &&
256+
attribute === 'sizes' &&
257+
hasAttribute(node, 'data-hero')
258+
) {
259+
continue;
260+
}
252261
delete node.attribs[attribute];
253262
}
254263
}

‎packages/optimizer/lib/transformers/OptimizeHeroImages.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,11 @@ class OptimizeHeroImage {
316316
if (!node) {
317317
return;
318318
}
319+
// Generate a default sizes element to avoid that the runtime generate it
320+
// See https://github.com/ampproject/amphtml/issues/32644
321+
if (hasAttribute(node, 'srcset') && !hasAttribute(node, 'sizes')) {
322+
node.attribs.sizes = '100vw';
323+
}
319324
node.attribs['i-amphtml-ssr'] = '';
320325
node.attribs['data-hero'] = '';
321326
// Create img node

‎packages/optimizer/spec/transformers/valid/OptimizeHeroImages/amp-img-sizes/expected_output.html‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<html>
2+
<head>
3+
<script async src="https://cdn.ampproject.org/v0.js"></script>
4+
</head>
5+
<body>
6+
<amp-img data-hero width="500" height="400" src="/foo1.png" srcset="elva-fairy-480w.jpg 480w, elva-fairy-800w.jpg 800w" sizes="(max-width: 600px) 480px, 800px" i-amphtml-ssr><img class="i-amphtml-fill-content i-amphtml-replaced-content" decoding="async" src="/foo1.png" srcset="elva-fairy-480w.jpg 480w, elva-fairy-800w.jpg 800w" sizes="(max-width: 600px) 480px, 800px"></amp-img>
7+
<amp-img data-hero width="500" height="400" src="/foo2.png" srcset="elva-fairy-480w.jpg 480w, elva-fairy-800w.jpg 800w" sizes="100vw" i-amphtml-ssr><img class="i-amphtml-fill-content i-amphtml-replaced-content" decoding="async" src="/foo2.png" srcset="elva-fairy-480w.jpg 480w, elva-fairy-800w.jpg 800w" sizes="100vw"></amp-img>
8+
</body>
9+
</html>

‎packages/optimizer/spec/transformers/valid/OptimizeHeroImages/amp-img-sizes/input.html‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<html>
2+
<head>
3+
<script async src="https://cdn.ampproject.org/v0.js"></script>
4+
</head>
5+
<body>
6+
<amp-img data-hero width="500" height="400" src="/foo1.png" srcset="elva-fairy-480w.jpg 480w, elva-fairy-800w.jpg 800w" sizes="(max-width: 600px) 480px, 800px"></amp-img>
7+
<amp-img data-hero width="500" height="400" src="/foo2.png" srcset="elva-fairy-480w.jpg 480w, elva-fairy-800w.jpg 800w"></amp-img>
8+
</body>
9+
</html>

‎packages/optimizer/spec/transformers/valid/OptimizeHeroImages/srcset_cannot_be_preloaded/expected_output.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
<head></head>
33
<body>
44
<!-- srcset is not supported by all browsers -->
5-
<amp-img width="500" height="400" src="https://example-com.cdn.ampproject.org/foo.png" srcset="test 100w test2 3dpr" i-amphtml-ssr data-hero><img class="i-amphtml-fill-content i-amphtml-replaced-content" decoding="async" src="https://example-com.cdn.ampproject.org/foo.png" srcset="test 100w test2 3dpr"></amp-img>
5+
<amp-img width="500" height="400" src="https://example-com.cdn.ampproject.org/foo.png" srcset="test 100w test2 3dpr" sizes="100vw" i-amphtml-ssr data-hero><img class="i-amphtml-fill-content i-amphtml-replaced-content" decoding="async" src="https://example-com.cdn.ampproject.org/foo.png" srcset="test 100w test2 3dpr" sizes="100vw"></amp-img>
66
<!-- not preloaded as it's not a hero image -->
77
<amp-img width="500" height="400" src="https://example-com.cdn.ampproject.org/foo.png"></amp-img>
88
</body>

‎packages/optimizer/spec/transformers/valid/OptimizeHeroImages/srcset_validity_empty_srcset/expected_output.html‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
<html>
22
<head></head>
33
<body>
4-
<amp-img width="500" height="400" src="https://example-com.cdn.ampproject.org/foo.png" srcset i-amphtml-ssr data-hero>
5-
<img class="i-amphtml-fill-content i-amphtml-replaced-content" decoding="async" src="https://example-com.cdn.ampproject.org/foo.png" srcset>
4+
<amp-img width="500" height="400" src="https://example-com.cdn.ampproject.org/foo.png" srcset sizes="100vw" i-amphtml-ssr data-hero>
5+
<img class="i-amphtml-fill-content i-amphtml-replaced-content" decoding="async" src="https://example-com.cdn.ampproject.org/foo.png" srcset sizes="100vw">
66
</amp-img>
77
</body>
88
</html>

‎packages/optimizer/spec/transformers/valid/ServerSideRendering/converts_sizes_attribute_to_css/expected_output.html‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,24 @@
55

66

77
<link href="https://example.com/favicon.ico" rel="icon">
8-
<style amp-custom>#i-amp-0{width:100vw}@media (min-width: 320px){#i-amp-0{width:320px}}</style>
8+
<style amp-custom>#i-amp-0{width:100vw}@media (min-width: 320px){#i-amp-0{width:320px}}#i-amp-1{width:100vw}@media (min-width: 320px){#i-amp-1{width:320px}}</style>
99
</head>
1010
<body>
1111
<!-- requires srcset attribute -->
1212
<amp-img height="300" layout="responsive" src="https://acme.org/image1.png" width="400" class="i-amphtml-layout-responsive i-amphtml-layout-size-defined" i-amphtml-layout="responsive">
1313
<i-amphtml-sizer style="display:block;padding-top:75%"></i-amphtml-sizer>
1414
</amp-img>
1515
<!-- sizes -->
16+
<amp-video height="300" layout="responsive" src="https://acme.org/videot" .mp4 width="400" class="i-amphtml-layout-responsive i-amphtml-layout-size-defined" i-amphtml-layout="responsive">
17+
<i-amphtml-sizer style="display:block;padding-top:75%"></i-amphtml-sizer>
18+
</amp-video>
19+
<!-- sizes in amp-img-->
1620
<amp-img height="300" layout="responsive" srcset="img-480w.jpg 480w,img-800w.jpg 800w" src="https://acme.org/image1.png" width="400" class="i-amphtml-layout-responsive i-amphtml-layout-size-defined" i-amphtml-layout="responsive" id="i-amp-0">
1721
<i-amphtml-sizer style="display:block;padding-top:75%"></i-amphtml-sizer>
1822
</amp-img>
23+
<!-- sizes in hero img is not removed -->
24+
<amp-img data-hero height="300" layout="responsive" sizes="(min-width: 320px) 320px, 100vw" srcset="img-480w.jpg 480w,img-800w.jpg 800w" src="https://acme.org/image1.png" width="400" class="i-amphtml-layout-responsive i-amphtml-layout-size-defined" i-amphtml-layout="responsive" id="i-amp-1">
25+
<i-amphtml-sizer style="display:block;padding-top:75%"></i-amphtml-sizer>
26+
</amp-img>
1927
</body>
2028
</html>

‎packages/optimizer/spec/transformers/valid/ServerSideRendering/converts_sizes_attribute_to_css/input.html‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,25 @@
1717
width=400>
1818
</amp-img>
1919
<!-- sizes -->
20+
<amp-video
21+
height=300
22+
layout=responsive
23+
sizes="(min-width: 320px) 320px, 100vw"
24+
src=https://acme.org/videot .mp4
25+
width=400>
26+
</amp-video>
27+
<!-- sizes in amp-img-->
28+
<amp-img
29+
height=300
30+
layout=responsive
31+
sizes="(min-width: 320px) 320px, 100vw"
32+
srcset="img-480w.jpg 480w,img-800w.jpg 800w"
33+
src=https://acme.org/image1.png
34+
width=400>
35+
</amp-img>
36+
<!-- sizes in hero img is not removed -->
2037
<amp-img
38+
data-hero
2139
height=300
2240
layout=responsive
2341
sizes="(min-width: 320px) 320px, 100vw"

0 commit comments

Comments
 (0)