Fix for issue #1302: "SSR: Duplicated ids in rendered markup, if ther…

…e are nodes with attributes 'media', 'heights' or 'sizes'" (#1303) * fix duplicated ids * Added transformation test and refactored fix. * Adjusted input.html of transformation test with more realistic example
ampproject · Mar 1, 2022 · a1eaef5 · a1eaef5
1 parent 26e113b
commit a1eaef5
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 0 deletions.
diff --git a/packages/optimizer/lib/transformers/ApplyCommonAttributes.js b/packages/optimizer/lib/transformers/ApplyCommonAttributes.js
@@ -219,6 +219,7 @@ class ApplyCommonAttributes {
       }
       if (nodeHasBeenTransformed) {
         node.attribs.id = id;
+        this.ids.add(id);
       } else {
         this.transformedNodesCounter--;
       }

diff --git a/...erates_unique_ids_for_items_with_media_and_ignores_items_with_own_id/expected_output.html b/...erates_unique_ids_for_items_with_media_and_ignores_items_with_own_id/expected_output.html
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html ⚡ i-amphtml-layout i-amphtml-no-boilerplate>
+<head><style amp-runtime></style>
+  <script data-rh async src="https://cdn.ampproject.org/v0.js"></script>
+  <style amp-custom>@media not all and (min-width: 0px) and (max-width: 50px){#i-amp-0{display:none}}@media not all and (min-width: 51px) and (max-width: 129px){#i-amp-1{display:none}}</style>
+</head>
+<body>
+  <amp-img height="100" width="100" layout="responsive" src="img1.png" class="i-amphtml-layout-responsive i-amphtml-layout-size-defined" i-amphtml-layout="responsive" id="i-amp-0">
+    <i-amphtml-sizer slot="i-amphtml-svc" style="display:block;padding-top:100%"></i-amphtml-sizer>
+  </amp-img>
+  <amp-img id="customId" height="100" width="100" layout="responsive" src="img2.png" class="i-amphtml-layout-responsive i-amphtml-layout-size-defined" i-amphtml-layout="responsive">
+    <i-amphtml-sizer slot="i-amphtml-svc" style="display:block;padding-top:100%"></i-amphtml-sizer>
+  </amp-img>
+  <amp-img height="100" width="100" layout="responsive" src="img3.png" class="i-amphtml-layout-responsive i-amphtml-layout-size-defined" i-amphtml-layout="responsive" id="i-amp-1">
+    <i-amphtml-sizer slot="i-amphtml-svc" style="display:block;padding-top:100%"></i-amphtml-sizer>
+  </amp-img>
+</body>
+</html>
diff --git a/...dering/generates_unique_ids_for_items_with_media_and_ignores_items_with_own_id/input.html b/...dering/generates_unique_ids_for_items_with_media_and_ignores_items_with_own_id/input.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html ⚡>
+<head>
+  <script data-rh="" async="" src="https://cdn.ampproject.org/v0.js"></script>
+</head>
+<body>
+<amp-img media="(min-width: 0px) and (max-width: 50px)" height="100" width="100" layout="responsive" src="img1.png"></amp-img>
+<amp-img id="customId" height="100" width="100" layout="responsive" src="img2.png"></amp-img>
+<amp-img media="(min-width: 51px) and (max-width: 129px)" height="100" width="100" layout="responsive" src="img3.png"></amp-img>
+</body>
+</html>