mirror of
https://github.com/angular/angular
synced 2026-05-24 09:28:37 +00:00
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 #53278
(cherry picked from commit 38749698d0)
This commit is contained in:
parent
87372893ea
commit
2eeeabb760
6 changed files with 141 additions and 13 deletions
|
|
@ -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() {
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
@ -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 {}
|
||||
|
|
@ -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';
|
||||
|
|
|
|||
|
|
@ -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},
|
||||
|
|
|
|||
Loading…
Reference in a new issue