Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core(uses-responsive-images): higher threshold with breakpoints #13853

Merged
merged 11 commits into from
Jun 29, 2022
35 changes: 30 additions & 5 deletions lighthouse-core/audits/byte-efficiency/uses-responsive-images.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);

const IGNORE_THRESHOLD_IN_BYTES = 4096;

// Ignore up to 12KB of waste if an effort was made with breakpoints.
const IGNORE_THRESHOLD_IN_BYTES_BREAKPOINTS_PRESENT = 12288;

class UsesResponsiveImages extends ByteEfficiencyAudit {
/**
* @return {LH.Audit.Meta}
Expand Down Expand Up @@ -112,6 +115,18 @@ class UsesResponsiveImages extends ByteEfficiencyAudit {
};
}


/**
* @param {LH.Artifacts.ImageElement} image
* @return {number};
*/
static determineAllowableWaste(image) {
if (image.srcset || image.isPicture) {
return IGNORE_THRESHOLD_IN_BYTES_BREAKPOINTS_PRESENT;
}
return IGNORE_THRESHOLD_IN_BYTES;
}

/**
* @param {LH.Artifacts} artifacts
* @param {Array<LH.Artifacts.NetworkRequest>} networkRecords
Expand All @@ -126,6 +141,8 @@ class UsesResponsiveImages extends ByteEfficiencyAudit {
const ViewportDimensions = artifacts.ViewportDimensions;
/** @type {Map<string, LH.Audit.ByteEfficiencyItem>} */
const resultsMap = new Map();
/** @type {Array<string>} */
const passedImageList = [];
for (const image of images) {
// Give SVG a free pass because creating a "responsive" SVG is of questionable value.
// Ignore CSS images because it's difficult to determine what is a spritesheet,
Expand All @@ -147,15 +164,23 @@ class UsesResponsiveImages extends ByteEfficiencyAudit {
);
if (!processed) continue;

// Don't warn about an image that was later used appropriately
// Verify the image wastes more than the minimum.
const exceedsAllowableWaste = processed.wastedBytes > this.determineAllowableWaste(image);

const existing = resultsMap.get(processed.url);
if (!existing || existing.wastedBytes > processed.wastedBytes) {
resultsMap.set(processed.url, processed);
// Don't warn about an image that was later used appropriately, or wastes a trivial amount of data.
if (exceedsAllowableWaste && !passedImageList.includes(processed.url)) {
if ((!existing || existing.wastedBytes > processed.wastedBytes)) {
resultsMap.set(processed.url, processed);
}
} else {
// Ensure this url passes for future tests.
resultsMap.delete(processed.url);
passedImageList.push(processed.url);
TripleEquals marked this conversation as resolved.
Show resolved Hide resolved
}
}

const items = Array.from(resultsMap.values())
.filter(item => item.wastedBytes > IGNORE_THRESHOLD_IN_BYTES);
const items = Array.from(resultsMap.values());

/** @type {LH.Audit.Details.Opportunity['headings']} */
const headings = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,15 @@ function generateSize(width, height, prefix = 'displayed') {
return size;
}

function generateImage(clientSize, naturalDimensions, src = 'https://google.com/logo.png') {
return {src, ...clientSize, naturalDimensions, node: {devtoolsNodePath: '1,HTML,1,IMG'}};
function generateImage(clientSize, naturalDimensions, src = 'https://google.com/logo.png', srcset = '', isPicture = false) {
return {
src,
srcset,
...clientSize,
naturalDimensions,
isPicture,
node: {devtoolsNodePath: '1,HTML,1,IMG'},
};
}

describe('Page uses responsive images', () => {
Expand Down Expand Up @@ -132,7 +139,13 @@ describe('Page uses responsive images', () => {
});

it('identifies when images are not wasteful', async () => {
const networkRecords = [generateRecord(100, 300, 'https://google.com/logo.png'), generateRecord(90, 500, 'https://google.com/logo2.png'), generateRecord(20, 100, 'data:image/jpeg;base64,foobar')];
const networkRecords = [
generateRecord(100, 300, 'https://google.com/logo.png'),
generateRecord(90, 500, 'https://google.com/logo2.png'),
generateRecord(100, 300, 'https://google.com/logo3a.png'),
generateRecord(100, 300, 'https://google.com/logo3b.png'),
generateRecord(100, 300, 'https://google.com/logo3c.png'),
generateRecord(20, 100, 'data:image/jpeg;base64,foobar')];
const auditResult = await UsesResponsiveImagesAudit.audit_({
ViewportDimensions: {innerWidth: 1000, innerHeight: 1000, devicePixelRatio: 2},
ImageElements: [
Expand All @@ -146,6 +159,26 @@ describe('Page uses responsive images', () => {
{width: 210, height: 210},
'https://google.com/logo2.png'
),
generateImage(
generateSize(100, 100),
{width: 210, height: 210},
'https://google.com/logo3a.png',
'',
false
),
generateImage(
generateSize(100, 100),
{width: 210, height: 210},
'https://google.com/logo3b.png',
'',
true
),
generateImage(
generateSize(100, 100),
{width: 210, height: 210},
'https://google.com/logo3c.png',
'https://google.com/logo3c.png https://google.com/logo3c-2x.png 2x'
),
generateImage(
generateSize(100, 100),
{width: 80, height: 80},
Expand All @@ -157,7 +190,10 @@ describe('Page uses responsive images', () => {
{computedCache: new Map()}
);

assert.equal(auditResult.items.length, 2);
assert.equal(auditResult.items.length, 3);
TripleEquals marked this conversation as resolved.
Show resolved Hide resolved
assert.equal(auditResult.items[0].url, 'https://google.com/logo.png');
assert.equal(auditResult.items[1].url, 'https://google.com/logo3a.png');
assert.equal(auditResult.items[2].url, 'https://google.com/logo2.png');
TripleEquals marked this conversation as resolved.
Show resolved Hide resolved
});

it('ignores vectors', async () => {
Expand Down