feat(common): add warnings re: image distortion (#47082)

Checks whether image is visually distorted. Also adds a check to verify that width and height are set to a non-zero number.

PR Close #47082

packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts

@@ -36,6 +36,11 @@ const VALID_WIDTH_DESCRIPTOR_SRCSET = /^((\s*\d+w\s*(,|$)){1,})$/;
  */
 const VALID_DENSITY_DESCRIPTOR_SRCSET = /^((\s*\d(\.\d)?x\s*(,|$)){1,})$/;
 
+/**
+ * Used to determine whether two aspect ratios are similar in value.
+ */
+const ASPECT_RATIO_TOLERANCE = .1;
+
 /**
  * ** EXPERIMENTAL **
  *
@@ -86,7 +91,7 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
    */
   @Input()
   set width(value: string|number|undefined) {
-    ngDevMode && assertValidNumberInput(value, 'width');
+    ngDevMode && assertGreaterThanZeroNumber(value, 'width');
     this._width = inputToInteger(value);
   }
   get width(): number|undefined {
@@ -98,7 +103,7 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
    */
   @Input()
   set height(value: string|number|undefined) {
-    ngDevMode && assertValidNumberInput(value, 'height');
+    ngDevMode && assertGreaterThanZeroNumber(value, 'height');
     this._height = inputToInteger(value);
   }
   get height(): number|undefined {
@@ -146,6 +151,7 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
       assertRequiredNumberInput(this, this.width, 'width');
       assertRequiredNumberInput(this, this.height, 'height');
       assertValidLoadingInput(this);
+      assertNoImageDistortion(this, this.imgElement, this.renderer);
       if (this.priority) {
         const checker = this.injector.get(PreconnectLinkChecker);
         checker.check(this.getRewrittenSrc(), this.rawSrc);
@@ -400,11 +406,12 @@ function assertNoPostInitInputChange(
   });
 }
 
-// Verifies that a specified input has a correct type (number).
-function assertValidNumberInput(inputValue: unknown, inputName: string) {
-  const isValid = typeof inputValue === 'number' ||
-      (typeof inputValue === 'string' && /^\d+$/.test(inputValue.trim()));
-  if (!isValid) {
+// Verifies that a specified input is a number greater than 0.
+function assertGreaterThanZeroNumber(inputValue: unknown, inputName: string) {
+  const validNumber = typeof inputValue === 'number' && inputValue > 0;
+  const validString =
+      typeof inputValue === 'string' && /^\d+$/.test(inputValue.trim()) && parseInt(inputValue) > 0;
+  if (!validNumber && !validString) {
     throw new RuntimeError(
         RuntimeErrorCode.INVALID_INPUT,
         `The NgOptimizedImage directive has detected that the \`${inputName}\` has an invalid ` +
@@ -413,6 +420,64 @@ function assertValidNumberInput(inputValue: unknown, inputName: string) {
   }
 }
 
+// Verifies that the rendered image is not visually distorted. Effectively this is checking:
+// - Whether the "width" and "height" attributes reflect the actual dimensions of the image.
+// - Whether image styling is "correct" (see below for a longer explanation).
+function assertNoImageDistortion(
+    dir: NgOptimizedImage, element: ElementRef<any>, renderer: Renderer2) {
+  const img = element.nativeElement;
+  const removeListenerFn = renderer.listen(img, 'load', () => {
+    removeListenerFn();
+    const renderedWidth = parseFloat(img.clientWidth);
+    const renderedHeight = parseFloat(img.clientHeight);
+    const renderedAspectRatio = renderedWidth / renderedHeight;
+    const nonZeroRenderedDimensions = renderedWidth !== 0 && renderedHeight !== 0;
+
+    const intrinsicWidth = parseFloat(img.naturalWidth);
+    const intrinsicHeight = parseFloat(img.naturalHeight);
+    const intrinsicAspectRatio = intrinsicWidth / intrinsicHeight;
+
+    const suppliedWidth = dir.width!;
+    const suppliedHeight = dir.height!;
+    const suppliedAspectRatio = suppliedWidth / suppliedHeight;
+
+    // Tolerance is used to account for the impact of subpixel rendering.
+    // Due to subpixel rendering, the rendered, intrinsic, and supplied
+    // aspect ratios of a correctly configured image may not exactly match.
+    // For example, a `width=4030 height=3020` image might have a rendered
+    // size of "1062w, 796.48h". (An aspect ratio of 1.334... vs. 1.333...)
+    const inaccurateDimensions =
+        Math.abs(suppliedAspectRatio - intrinsicAspectRatio) > ASPECT_RATIO_TOLERANCE;
+    const stylingDistortion = nonZeroRenderedDimensions &&
+        Math.abs(intrinsicAspectRatio - renderedAspectRatio) > ASPECT_RATIO_TOLERANCE;
+    if (inaccurateDimensions) {
+      console.warn(
+          RuntimeErrorCode.INVALID_INPUT,
+          `${imgDirectiveDetails(dir.rawSrc)} has detected that the aspect ratio of the ` +
+              `image does not match the aspect ratio indicated by the width and height attributes. ` +
+              `Intrinsic image size: ${intrinsicWidth}w x ${intrinsicHeight}h (aspect-ratio: ${
+                  intrinsicAspectRatio}). ` +
+              `Supplied width and height attributes: ${suppliedWidth}w x ${
+                  suppliedHeight}h (aspect-ratio: ${suppliedAspectRatio}). ` +
+              `To fix this, update the width and height attributes.`);
+    } else {
+      if (stylingDistortion) {
+        console.warn(
+            RuntimeErrorCode.INVALID_INPUT,
+            `${imgDirectiveDetails(dir.rawSrc)} has detected that the aspect ratio of the ` +
+                `rendered image does not match the image's intrinsic aspect ratio. ` +
+                `Intrinsic image size: ${intrinsicWidth}w x ${intrinsicHeight}h (aspect-ratio: ${
+                    intrinsicAspectRatio}). ` +
+                `Rendered image size: ${renderedWidth}w x ${renderedHeight}h (aspect-ratio: ${
+                    renderedAspectRatio}). ` +
+                `This issue can occur if "width" and "height" attributes are added to an image ` +
+                `without updating the corresponding image styling. In most cases, ` +
+                `adding "height: auto" or "width: auto" to the image styling will fix this issue.`);
+      }
+    }
+  });
+}
+
 // Verifies that a specified input is set.
 function assertRequiredNumberInput(dir: NgOptimizedImage, inputValue: unknown, inputName: string) {
   if (typeof inputValue === 'undefined') {

packages/common/test/directives/ng_optimized_image_spec.ts

@@ -167,6 +167,22 @@ describe('Image directive', () => {
               'on the mentioned element.');
     });
 
+    it('should throw if `width` is 0', () => {
+      setupTestingModule();
+
+      const template = '<img rawSrc="img.png" width="0" height="10">';
+      expect(() => {
+        const fixture = createTestComponent(template);
+        fixture.detectChanges();
+      })
+          .toThrowError(
+              `NG0${
+                  RuntimeErrorCode
+                      .INVALID_INPUT}: The NgOptimizedImage directive has detected that the \`width\` ` +
+              'has an invalid value: expecting a number that represents the width ' +
+              'in pixels, but got: `0`.');
+    });
+
     it('should throw if `width` has an invalid value', () => {
       setupTestingModule();
 
@@ -200,6 +216,22 @@ describe('Image directive', () => {
               'on the mentioned element.');
     });
 
+    it('should throw if `height` is 0', () => {
+      setupTestingModule();
+
+      const template = '<img rawSrc="img.png" width="10" height="0">';
+      expect(() => {
+        const fixture = createTestComponent(template);
+        fixture.detectChanges();
+      })
+          .toThrowError(
+              `NG0${
+                  RuntimeErrorCode
+                      .INVALID_INPUT}: The NgOptimizedImage directive has detected that the \`height\` ` +
+              'has an invalid value: expecting a number that represents the height ' +
+              'in pixels, but got: `0`.');
+    });
+
     it('should throw if `height` has an invalid value', () => {
       setupTestingModule();
 

packages/core/test/bundling/image-directive/BUILD.bazel

@@ -6,6 +6,7 @@ ng_module(
     name = "image-directive",
     srcs = [
         "e2e/basic/basic.ts",
+        "e2e/image-distortion/image-distortion.ts",
         "e2e/lcp-check/lcp-check.ts",
         "e2e/preconnect-check/preconnect-check.ts",
         "index.ts",
@@ -57,6 +58,7 @@ ts_devserver(
         ":tslib",
         "e2e/a.png",
         "e2e/b.png",
+        "e2e/logo-500w.jpg",
     ],
     deps = [":image-directive"],
 )

packages/core/test/bundling/image-directive/e2e/image-distortion/image-distortion.e2e-spec.ts

@@ -0,0 +1,88 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+/* tslint:disable:no-console  */
+import {browser, by, element, ExpectedConditions} from 'protractor';
+import {logging} from 'selenium-webdriver';
+
+import {collectBrowserLogs} from '../util';
+
+describe('NgOptimizedImage directive', () => {
+  it('should not warn if there is no image distortion', async () => {
+    await browser.get('/e2e/image-distortion-passing');
+    const logs = await collectBrowserLogs(logging.Level.WARNING);
+    expect(logs.length).toEqual(0);
+  });
+  it('should warn if there is image distortion', async () => {
+    await browser.get('/e2e/image-distortion-failing');
+    const logs = await collectBrowserLogs(logging.Level.WARNING);
+
+    expect(logs.length).toEqual(5);
+    // Image loading order is not gaurenteed, so all logs, rather than single entry
+    // needs to be checked in order to test whether a given error message is present.
+    const expectErrorMessageInLogs = (logs: logging.Entry[], message: string) => {
+      expect(logs.some((log) => {
+        return log.message.includes(message);
+      })).toBeTruthy();
+    };
+
+    // Images with incorrect width/height attributes
+    expectErrorMessageInLogs(
+        logs,
+        'The NgOptimizedImage directive (activated on an \\u003Cimg> element ' +
+            'with the \`rawSrc=\\"/e2e/b.png\\"`) has detected that ' +
+            'the aspect ratio of the image does not match the aspect ratio indicated by the width and height attributes. ' +
+            'Intrinsic image size: 250w x 250h (aspect-ratio: 1). ' +
+            'Supplied width and height attributes: 26w x 30h (aspect-ratio: 0.8666666666666667). ' +
+            'To fix this, update the width and height attributes.');
+
+    expectErrorMessageInLogs(
+        logs,
+        'The NgOptimizedImage directive (activated on an \\u003Cimg> element ' +
+            'with the \`rawSrc=\\"/e2e/b.png\\"`) has detected that ' +
+            'the aspect ratio of the image does not match the aspect ratio indicated by the width and height attributes. ' +
+            'Intrinsic image size: 250w x 250h (aspect-ratio: 1). ' +
+            'Supplied width and height attributes: 24w x 240h (aspect-ratio: 0.1). ' +
+            'To fix this, update the width and height attributes.');
+
+    // Images with incorrect styling
+    expectErrorMessageInLogs(
+        logs,
+        'The NgOptimizedImage directive (activated on an \\u003Cimg> element ' +
+            'with the \`rawSrc=\\"/e2e/b.png\\"`) has detected that ' +
+            'the aspect ratio of the rendered image does not match the image\'s intrinsic aspect ratio. ' +
+            'Intrinsic image size: 250w x 250h (aspect-ratio: 1). ' +
+            'Rendered image size: 250w x 30h (aspect-ratio: 8.333333333333334). ' +
+            'This issue can occur if \\"width\\" and \\"height\\" attributes are added to an image ' +
+            'without updating the corresponding image styling. In most cases, ' +
+            'adding \\"height: auto\\" or \\"width: auto\\" to the image styling will fix this issue.');
+
+    expectErrorMessageInLogs(
+        logs,
+        'The NgOptimizedImage directive (activated on an \\u003Cimg> element ' +
+            'with the \`rawSrc=\\"/e2e/b.png\\"`) has detected that ' +
+            'the aspect ratio of the rendered image does not match the image\'s intrinsic aspect ratio. ' +
+            'Intrinsic image size: 250w x 250h (aspect-ratio: 1). ' +
+            'Rendered image size: 30w x 250h (aspect-ratio: 0.12). ' +
+            'This issue can occur if \\"width\\" and \\"height\\" attributes are added to an image ' +
+            'without updating the corresponding image styling. In most cases, ' +
+            'adding \\"height: auto\\" or \\"width: auto\\" to the image styling will fix this issue.');
+
+    // Image with incorrect width/height attributes AND incorrect styling
+    // This only generate only one error to ensure that users first fix the width and height
+    // attributes.
+    expectErrorMessageInLogs(
+        logs,
+        'The NgOptimizedImage directive (activated on an \\u003Cimg> element ' +
+            'with the \`rawSrc=\\"/e2e/b.png\\"`) has detected that ' +
+            'the aspect ratio of the image does not match the aspect ratio indicated by the width and height attributes. ' +
+            'Intrinsic image size: 250w x 250h (aspect-ratio: 1). ' +
+            'Supplied width and height attributes: 150w x 250h (aspect-ratio: 0.6). ' +
+            'To fix this, update the width and height attributes.');
+  });
+});

packages/core/test/bundling/image-directive/e2e/image-distortion/image-distortion.ts

@@ -0,0 +1,79 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import {ɵNgOptimizedImageModule as NgOptimizedImageModule} from '@angular/common';
+import {Component} from '@angular/core';
+
+@Component({
+  selector: 'image-distortion-passing',
+  standalone: true,
+  imports: [NgOptimizedImageModule],
+  template: `
+     <!-- All the images in this template should not throw -->
+     <!-- This image is here for the sake of making sure the "LCP image is priority" assertion is passed -->
+     <img rawSrc="/e2e/logo-500w.jpg" width="500" height="500" priority>
+     <br>
+     <!-- width and height attributes exacly match the intrinsic size of image -->
+     <img rawSrc="/e2e/a.png" width="25" height="25">
+     <br>
+     <!-- supplied aspect ratio exactly matches intrinsic aspect ratio-->
+     <img rawSrc="/e2e/a.png" width="250" height="250">
+     <img rawSrc="/e2e/b.png" width="40" height="40">
+     <img rawSrc="/e2e/b.png" width="240" height="240">
+     <br>
+     <!-- supplied aspect ratio is similar to intrinsic aspect ratio -->
+     <!-- Aspect-ratio: 0.93333333333 -->
+     <img rawSrc="/e2e/b.png" width="28" height="30">
+     <!-- Aspect-ratio: 0.9 -->
+     <img rawSrc="/e2e/b.png" width="27" height="30">
+     <!-- Aspect-ratio: 1.09375 -->
+     <img rawSrc="/e2e/b.png" width="350" height="320">
+     <!-- Aspect-ratio: 1.0652173913 -->
+     <img rawSrc="/e2e/b.png" width="245" height="230">
+     <br>
+     <!-- Supplied aspect ratio is correct & image has 0x0 rendered size -->
+     <img rawSrc="/e2e/a.png" width="25" height="25" style="display: none">
+     <br>
+     <!-- styling is correct -->
+     <img rawSrc="/e2e/a.png" width="25" height="25" style="width: 100%; height: 100%">
+     <img rawSrc="/e2e/a.png" width="250" height="250" style="max-width: 100%; height: 100%">
+     <img rawSrc="/e2e/a.png" width="25" height="25" style="height: 25%; width: 25%;">
+     <br>
+    `,
+})
+export class ImageDistortionPassingComponent {
+}
+@Component({
+  selector: 'image-distortion-failing',
+  standalone: true,
+  imports: [NgOptimizedImageModule],
+  template: `
+     <!-- With the exception of the priority image, all the images in this template should throw -->
+     <!-- This image is here for the sake of making sure the "LCP image is priority" assertion is passed -->
+     <img rawSrc="/e2e/logo-500w.jpg" width="500" height="500" priority>
+     <br>
+     <!-- These images should throw -->
+     <!-- Supplied aspect ratio differs from intrinsic aspect ratio by > .1 -->
+     <!-- Aspect-ratio: 0.86666666666 -->
+     <img rawSrc="/e2e/b.png" width="26" height="30">
+     <!-- Aspect-ratio: 0.1 -->
+     <img rawSrc="/e2e/b.png" width="24" height="240">
+     <!-- Supplied aspect ratio is incorrect & image has 0x0 rendered size -->
+     <img rawSrc="/e2e/a.png" width="222" height="25" style="display: none">
+     <br>
+     <!-- Image styling is causing distortion -->
+     <div style="width: 300px; height: 300px">
+       <img rawSrc="/e2e/b.png" width="250" height="250" style="width: 10%">
+       <img rawSrc="/e2e/b.png" width="250" height="250" style="max-height: 10%">
+       <!-- Images dimensions are incorrect AND image styling is incorrect -->
+       <img rawSrc="/e2e/b.png" width="150" height="250" style="max-height: 10%">
+     </div>
+     `,
+})
+export class ImageDistortionFailingComponent {
+}

packages/core/test/bundling/image-directive/e2e/lcp-check/lcp-check.ts

@@ -23,7 +23,7 @@ import {Component} from '@angular/core';
     <br>
 
     <!-- 'a.png' should be treated as an LCP element -->
-    <img rawSrc="/e2e/a.png" width="1500" height="2500">
+    <img rawSrc="/e2e/a.png" width="2500" height="2500">
 
     <br>
 

packages/core/test/bundling/image-directive/index.ts

@@ -11,6 +11,7 @@ import {bootstrapApplication, provideProtractorTestingSupport} from '@angular/pl
 import {RouterModule} from '@angular/router';
 
 import {BasicComponent} from './e2e/basic/basic';
+import {ImageDistortionFailingComponent, ImageDistortionPassingComponent} from './e2e/image-distortion/image-distortion';
 import {LcpCheckComponent} from './e2e/lcp-check/lcp-check';
 import {PreconnectCheckComponent} from './e2e/preconnect-check/preconnect-check';
 import {PlaygroundComponent} from './playground';
@@ -31,7 +32,9 @@ const ROUTES = [
   // Paths below are used for e2e testing:
   {path: 'e2e/basic', component: BasicComponent},
   {path: 'e2e/lcp-check', component: LcpCheckComponent},
-  {path: 'e2e/preconnect-check', component: PreconnectCheckComponent}
+  {path: 'e2e/preconnect-check', component: PreconnectCheckComponent},
+  {path: 'e2e/image-distortion-passing', component: ImageDistortionPassingComponent},
+  {path: 'e2e/image-distortion-failing', component: ImageDistortionFailingComponent},
 ];
 
 bootstrapApplication(RootComponent, {