fix(common): fix LCP image detection with duplicate URLs

Addresses an issue where the LCP image observer incorrectly identified LCP elements when the same image URL was used multiple times on a page

Fixes angular#53278

Author: SkyZeroZx

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

@@ -25,6 +25,7 @@ interface ObservedImageState {
   modified: boolean;
   alreadyWarnedPriority: boolean;
   alreadyWarnedModified: boolean;
+  count: number;
 }
 
 /**
@@ -94,29 +95,77 @@ export class LCPImageObserver implements OnDestroy {
 
   registerImage(rewrittenSrc: string, isPriority: boolean) {
     if (!this.observer) return;
-    const newObservedImageState: ObservedImageState = {
-      priority: isPriority,
-      modified: false,
-      alreadyWarnedModified: false,
-      alreadyWarnedPriority: false,
-    };
-    this.images.set(getUrl(rewrittenSrc, this.window!).href, newObservedImageState);
+    const url = getUrl(rewrittenSrc, this.window!).href;
+    const existingState = this.images.get(url);
+
+    if (existingState) {
+      // If any instance has priority, the URL is considered to have priority
+      existingState.priority = existingState.priority || isPriority;
+      existingState.count++;
+    } else {
+      const newObservedImageState: ObservedImageState = {
+        priority: isPriority,
+        modified: false,
+        alreadyWarnedModified: false,
+        alreadyWarnedPriority: false,
+        count: 1,
+      };
+      this.images.set(url, newObservedImageState);
+    }
   }
 
   unregisterImage(rewrittenSrc: string) {
     if (!this.observer) return;
-    this.images.delete(getUrl(rewrittenSrc, this.window!).href);
+    const url = getUrl(rewrittenSrc, this.window!).href;
+    const existingState = this.images.get(url);
+
+    if (existingState) {
+      existingState.count--;
+      if (existingState.count <= 0) {
+        this.images.delete(url);
+      }
+    }
   }
 
   updateImage(originalSrc: string, newSrc: string) {
     if (!this.observer) return;
     const originalUrl = getUrl(originalSrc, this.window!).href;
-    const img = this.images.get(originalUrl);
-    if (img) {
-      img.modified = true;
-      this.images.set(getUrl(newSrc, this.window!).href, img);
+    const newUrl = getUrl(newSrc, this.window!).href;
+
+    // URL hasn't changed
+    if (originalUrl === newUrl) return;
+
+    const originalState = this.images.get(originalUrl);
+    if (!originalState) return;
+
+    // Decrement count for original URL
+    originalState.count--;
+    if (originalState.count <= 0) {
       this.images.delete(originalUrl);
     }
+
+    // Add or update entry for new URL
+    const newState = this.images.get(newUrl);
+    if (newState) {
+      // Merge if original had priority, new should too
+      newState.priority = newState.priority || originalState.priority;
+      newState.modified = true;
+      // Preserve warning flags from the original state to avoid duplicate warnings
+      newState.alreadyWarnedPriority =
+        newState.alreadyWarnedPriority || originalState.alreadyWarnedPriority;
+      newState.alreadyWarnedModified =
+        newState.alreadyWarnedModified || originalState.alreadyWarnedModified;
+      newState.count++;
+    } else {
+      // Create new entry, preserving state from the image that moved
+      this.images.set(newUrl, {
+        priority: originalState.priority,
+        modified: true,
+        alreadyWarnedModified: originalState.alreadyWarnedModified,
+        alreadyWarnedPriority: originalState.alreadyWarnedPriority,
+        count: 1,
+      });
+    }
   }
 
   ngOnDestroy() {

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

@@ -12,6 +12,7 @@ ng_project(
         "e2e/image-perf-warnings-lazy/image-perf-warnings-lazy.ts",
         "e2e/image-perf-warnings-oversized/image-perf-warnings-oversized.ts",
         "e2e/image-perf-warnings-oversized/svg-no-perf-oversized-warnings.ts",
+        "e2e/lcp-check-duplicate/lcp-check-duplicate.ts",
         "e2e/lcp-check/lcp-check.ts",
         "e2e/oversized-image/oversized-image.ts",
         "e2e/preconnect-check/preconnect-check.ts",

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

@@ -0,0 +1,35 @@
+/**
+ * @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.dev/license
+ */
+
+/* tslint:disable:no-console  */
+import {browser, by, element} from 'protractor';
+import {logging} from 'selenium-webdriver';
+
+import {collectBrowserLogs} from '../browser-logs-util';
+
+describe('NgOptimizedImage directive', () => {
+  it('should log a warning when a `priority` is missing on an LCP image', async () => {
+    await browser.get('/e2e/lcp-check-duplicate');
+    // Verify that both images were rendered.
+    const imgs = element.all(by.css('img'));
+    let srcB = await imgs.get(0).getAttribute('src');
+    expect(srcB.endsWith('b.png')).toBe(true);
+    let srcA = await imgs.get(1).getAttribute('src');
+    expect(srcA.endsWith('a.png')).toBe(true);
+    // The `b.png` and `a.png` images are used twice in a template.
+    srcB = await imgs.get(2).getAttribute('src');
+    expect(srcB.endsWith('b.png')).toBe(true);
+    srcA = await imgs.get(3).getAttribute('src');
+    expect(srcA.endsWith('a.png')).toBe(true);
+
+    // Make sure that no warnings are in the console for image `a.png`,
+    // since the first instance has the `priority` attribute, and is the LCP element.
+    const logs = await collectBrowserLogs(logging.Level.SEVERE);
+    expect(logs.length).toEqual(0);
+  });
+});

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

@@ -0,0 +1,41 @@
+/**
+ * @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 {NgOptimizedImage} from '@angular/common';
+import {Component} from '@angular/core';
+
+@Component({
+  selector: 'lcp-check',
+  imports: [NgOptimizedImage],
+  template: `
+    <!--
+      'b.png' should *not* be treated as an LCP element,
+      since there is a bigger one right below it
+    -->
+    <img ngSrc="/e2e/b.png" width="5" height="5" />
+
+    <br />
+
+    <!-- 'a.png' should be treated as an LCP element, and has priority -->
+    <img ngSrc="/e2e/a.png" width="2500" height="2500" priority />
+
+    <br />
+
+    <!--
+      'b.png' should *not* be treated as an LCP element here
+      as well, since it's below the fold
+    -->
+    <img ngSrc="/e2e/b.png" width="10" height="10" />
+
+    <!-- 'a.png' doesn't have priority, and is not the LCP element -->
+    <img ngSrc="/e2e/a.png" width="1000" height="1000" />
+
+    <br />
+  `,
+})
+export class LcpCheckDuplicate {}

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

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
-import {browser, by, element, ExpectedConditions} from 'protractor';
+import {browser} from 'protractor';
 import {logging} from 'selenium-webdriver';
 
 import {collectBrowserLogs} from '../browser-logs-util';

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

@@ -27,6 +27,7 @@ import {
 } from './e2e/oversized-image/oversized-image';
 import {PreconnectCheckComponent} from './e2e/preconnect-check/preconnect-check';
 import {PlaygroundComponent} from './playground';
+import {LcpCheckDuplicate} from './e2e/lcp-check-duplicate/lcp-check-duplicate';
 
 @Component({
   selector: 'app-root',
@@ -42,6 +43,7 @@ const ROUTES = [
   // Paths below are used for e2e testing:
   {path: 'e2e/basic', component: BasicComponent},
   {path: 'e2e/lcp-check', component: LcpCheckComponent},
+  {path: 'e2e/lcp-check-duplicate', component: LcpCheckDuplicate},
   {path: 'e2e/image-perf-warnings-lazy', component: ImagePerfWarningsLazyComponent},
   {path: 'e2e/image-perf-warnings-oversized', component: ImagePerfWarningsOversizedComponent},
   {path: 'e2e/svg-no-perf-oversized-warnings', component: SvgNoOversizedPerfWarningsComponent},