fix(common): apply fixed_srcset_width value only to fixed srcsets (#52459)

atcastle · alxhub · commit 3bd85fb7b072 · 2023-10-31T14:57:19.000-07:00
add logic to NgOptimizedImage to keep fixed_srcset_width from applying to large responsive images, which is incorrect behavior PR Close #52459
diff --git a/packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts b/packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts
@@ -513,8 +513,13 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
   }
 
   private shouldGenerateAutomaticSrcset(): boolean {
+    let oversizedImage = false;
+    if (!this.sizes) {
+      oversizedImage =
+          this.width! > FIXED_SRCSET_WIDTH_LIMIT || this.height! > FIXED_SRCSET_HEIGHT_LIMIT;
+    }
     return !this.disableOptimizedSrcset && !this.srcset && this.imageLoader !== noopImageLoader &&
-        !(this.width! > FIXED_SRCSET_WIDTH_LIMIT || this.height! > FIXED_SRCSET_HEIGHT_LIMIT);
+        !oversizedImage;
   }
 
   /** @nodoc */
diff --git a/packages/common/test/directives/ng_optimized_image_spec.ts b/packages/common/test/directives/ng_optimized_image_spec.ts
@@ -1797,6 +1797,38 @@ describe('Image directive', () => {
         expect(img.getAttribute('srcset')).toBeNull();
       });
 
+      it('should add a responsive srcset to the img element if height is too large', () => {
+        setupTestingModule({imageLoader});
+
+        const template = `<img ngSrc="img" width="1100" height="2400" sizes="100vw">`;
+        const fixture = createTestComponent(template);
+        fixture.detectChanges();
+
+        const nativeElement = fixture.nativeElement as HTMLElement;
+        const img = nativeElement.querySelector('img')!;
+        expect(img.getAttribute('srcset'))
+            .toBe(`${IMG_BASE_URL}/img?w=640 640w, ${IMG_BASE_URL}/img?w=750 750w, ${
+                IMG_BASE_URL}/img?w=828 828w, ${IMG_BASE_URL}/img?w=1080 1080w, ${
+                IMG_BASE_URL}/img?w=1200 1200w, ${IMG_BASE_URL}/img?w=1920 1920w, ${
+                IMG_BASE_URL}/img?w=2048 2048w, ${IMG_BASE_URL}/img?w=3840 3840w`);
+      });
+
+      it('should add a responsive srcset to the img element if width is too large', () => {
+        setupTestingModule({imageLoader});
+
+        const template = `<img ngSrc="img" width="3000" height="400" sizes="100vw">`;
+        const fixture = createTestComponent(template);
+        fixture.detectChanges();
+
+        const nativeElement = fixture.nativeElement as HTMLElement;
+        const img = nativeElement.querySelector('img')!;
+        expect(img.getAttribute('srcset'))
+            .toBe(`${IMG_BASE_URL}/img?w=640 640w, ${IMG_BASE_URL}/img?w=750 750w, ${
+                IMG_BASE_URL}/img?w=828 828w, ${IMG_BASE_URL}/img?w=1080 1080w, ${
+                IMG_BASE_URL}/img?w=1200 1200w, ${IMG_BASE_URL}/img?w=1920 1920w, ${
+                IMG_BASE_URL}/img?w=2048 2048w, ${IMG_BASE_URL}/img?w=3840 3840w`);
+      });
+
       it('should use a custom breakpoint set if one is provided', () => {
         const imageConfig = {
           breakpoints: [16, 32, 48, 64, 96, 128, 256, 384, 640, 1280, 3840],

Original file line number	Diff line number	Diff line change
`@@ -513,8 +513,13 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {`
`513`	`513`	`}`
`514`	`514`
`515`	`515`	`private shouldGenerateAutomaticSrcset(): boolean {`
	`516`	`+ let oversizedImage = false;`
	`517`	`+ if (!this.sizes) {`
	`518`	`+ oversizedImage =`
	`519`	`+ this.width! > FIXED_SRCSET_WIDTH_LIMIT \|\| this.height! > FIXED_SRCSET_HEIGHT_LIMIT;`
	`520`	`+ }`
`516`	`521`	`return !this.disableOptimizedSrcset && !this.srcset && this.imageLoader !== noopImageLoader &&`
`517`		`- !(this.width! > FIXED_SRCSET_WIDTH_LIMIT \|\| this.height! > FIXED_SRCSET_HEIGHT_LIMIT);`
	`522`	`+ !oversizedImage;`
`518`	`523`	`}`
`519`	`524`
`520`	`525`	`/** @nodoc */`