Skip to content

Commit

Permalink
core(image-elements): make naturalWidth/Height property optional (#11703
Browse files Browse the repository at this point in the history
)
  • Loading branch information
adrianaixba authored Dec 10, 2020
1 parent 6ad1591 commit 1ca0e7f
Show file tree
Hide file tree
Showing 58 changed files with 48 additions and 205 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class UsesOptimizedImages extends ByteEfficiencyAudit {
}

/**
* @param {LH.Artifacts.ImageElement} imageElement
* @param {{naturalWidth: number, naturalHeight: number}} imageElement
* @return {number}
*/
static estimateJPEGSizeFromDimensions(imageElement) {
Expand Down Expand Up @@ -96,7 +96,12 @@ class UsesOptimizedImages extends ByteEfficiencyAudit {
continue;
}

jpegSize = UsesOptimizedImages.estimateJPEGSizeFromDimensions(imageElement);
// If naturalHeight or naturalWidth are falsy, information is not valid, skip.
const naturalHeight = imageElement.naturalHeight;
const naturalWidth = imageElement.naturalWidth;
if (!naturalHeight || !naturalWidth) continue;
jpegSize =
UsesOptimizedImages.estimateJPEGSizeFromDimensions({naturalHeight, naturalWidth});
fromProtocol = false;
}

Expand Down
30 changes: 11 additions & 19 deletions lighthouse-core/audits/byte-efficiency/uses-responsive-images.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
'use strict';

const ByteEfficiencyAudit = require('./byte-efficiency-audit.js');
const Sentry = require('../../lib/sentry.js');
const URL = require('../../lib/url-shim.js');
const i18n = require('../../lib/i18n/i18n.js');

Expand Down Expand Up @@ -47,10 +46,10 @@ class UsesResponsiveImages extends ByteEfficiencyAudit {
}

/**
* @param {LH.Artifacts.ImageElement} image
* @param {LH.Artifacts.ImageElement & {naturalWidth: number, naturalHeight: number}} image
* @param {LH.Artifacts.ViewportDimensions} ViewportDimensions
* @param {Array<LH.Artifacts.NetworkRequest>} networkRecords
* @return {null|Error|LH.Audit.ByteEfficiencyItem};
* @return {null|LH.Audit.ByteEfficiencyItem};
*/
static computeWaste(image, ViewportDimensions, networkRecords) {
const networkRecord = networkRecords.find(record => record.url === image.src);
Expand Down Expand Up @@ -92,10 +91,6 @@ class UsesResponsiveImages extends ByteEfficiencyAudit {
const totalBytes = Math.min(resourceSize, transferSize);
const wastedBytes = Math.round(totalBytes * wastedRatio);

if (!Number.isFinite(wastedRatio)) {
return new Error(`Invalid image sizing information ${url}`);
}

return {
url,
totalBytes,
Expand All @@ -112,9 +107,6 @@ class UsesResponsiveImages extends ByteEfficiencyAudit {
static audit_(artifacts, networkRecords) {
const images = artifacts.ImageElements;
const ViewportDimensions = artifacts.ViewportDimensions;

/** @type {string[]} */
const warnings = [];
/** @type {Map<string, LH.Audit.ByteEfficiencyItem>} */
const resultsMap = new Map();
for (const image of images) {
Expand All @@ -125,16 +117,17 @@ class UsesResponsiveImages extends ByteEfficiencyAudit {
continue;
}

/* eslint-disable max-len */
const processed = UsesResponsiveImages.computeWaste(image, ViewportDimensions, networkRecords);
const naturalHeight = image.naturalHeight;
const naturalWidth = image.naturalWidth;
// If naturalHeight or naturalWidth are falsy, information is not valid, skip.
if (!naturalWidth || !naturalHeight) continue;
const processed =
UsesResponsiveImages.computeWaste(
{...image, naturalHeight, naturalWidth},
ViewportDimensions, networkRecords
);
if (!processed) continue;

if (processed instanceof Error) {
warnings.push(processed.message);
Sentry.captureException(processed, {tags: {audit: this.meta.id}, level: 'warning'});
continue;
}

// Don't warn about an image that was later used appropriately
const existing = resultsMap.get(processed.url);
if (!existing || existing.wastedBytes > processed.wastedBytes) {
Expand All @@ -154,7 +147,6 @@ class UsesResponsiveImages extends ByteEfficiencyAudit {
];

return {
warnings,
items,
headings,
};
Expand Down
9 changes: 7 additions & 2 deletions lighthouse-core/audits/byte-efficiency/uses-webp-images.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class UsesWebPImages extends ByteEfficiencyAudit {
}

/**
* @param {LH.Artifacts.ImageElement} imageElement
* @param {{naturalWidth: number, naturalHeight: number}} imageElement
* @return {number}
*/
static estimateWebPSizeFromDimensions(imageElement) {
Expand Down Expand Up @@ -95,7 +95,12 @@ class UsesWebPImages extends ByteEfficiencyAudit {
continue;
}

webpSize = UsesWebPImages.estimateWebPSizeFromDimensions(imageElement);
const naturalHeight = imageElement.naturalHeight;
const naturalWidth = imageElement.naturalWidth;
// If naturalHeight or naturalWidth are falsy, information is not valid, skip.
if (!naturalWidth || !naturalHeight) continue;

webpSize = UsesWebPImages.estimateWebPSizeFromDimensions({naturalHeight, naturalWidth});
fromProtocol = false;
}

Expand Down
25 changes: 5 additions & 20 deletions lighthouse-core/audits/image-aspect-ratio.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,6 @@ const UIStrings = {
/** Description of a Lighthouse audit that tells the user why they should maintain the correct aspect ratios for all images. This is displayed after a user expands the section to see more. No character length limits. 'Learn More' becomes link text to additional documentation. */
description: 'Image display dimensions should match natural aspect ratio. ' +
'[Learn more](https://web.dev/image-aspect-ratio/).',
/**
* @description Warning that the size information for an image was nonsensical.
* @example {https://image.cdn.com/} url
*/
warningCompute: 'Invalid image sizing information {url}',
/** Label for a column in a data table; entries in the column will be the numeric aspect ratio of an image as displayed in a web page. */
columnDisplayed: 'Aspect Ratio (Displayed)',
/** Label for a column in a data table; entries in the column will be the numeric aspect ratio of the raw (actual) image. */
Expand Down Expand Up @@ -56,7 +51,7 @@ class ImageAspectRatio extends Audit {

/**
* @param {WellDefinedImage} image
* @return {LH.IcuMessage|{url: string, displayedAspectRatio: string, actualAspectRatio: string, doRatiosMatch: boolean}}
* @return {{url: string, displayedAspectRatio: string, actualAspectRatio: string, doRatiosMatch: boolean}}
*/
static computeAspectRatios(image) {
const url = URL.elideDataURI(image.src);
Expand All @@ -66,11 +61,6 @@ class ImageAspectRatio extends Audit {
const targetDisplayHeight = image.displayedWidth / actualAspectRatio;
const doRatiosMatch = Math.abs(targetDisplayHeight - image.displayedHeight) < THRESHOLD_PX;

if (!Number.isFinite(actualAspectRatio) ||
!Number.isFinite(displayedAspectRatio)) {
return str_(UIStrings.warningCompute, {url});
}

return {
url,
displayedAspectRatio: `${image.displayedWidth} x ${image.displayedHeight}
Expand All @@ -87,8 +77,7 @@ class ImageAspectRatio extends Audit {
*/
static audit(artifacts) {
const images = artifacts.ImageElements;
/** @type {LH.IcuMessage[]} */
const warnings = [];

/** @type {Array<{url: string, displayedAspectRatio: string, actualAspectRatio: string, doRatiosMatch: boolean}>} */
const results = [];
images.filter(image => {
Expand All @@ -97,21 +86,18 @@ class ImageAspectRatio extends Audit {
// - filter out images that don't have following properties:
// networkRecord, width, height, `object-fit` property
// - filter all svgs as they have no natural dimensions to audit
// - filter out images that have falsy naturalWidth or naturalHeight
return !image.isCss &&
image.mimeType &&
image.mimeType !== 'image/svg+xml' &&
image.naturalHeight > 5 &&
image.naturalWidth > 5 &&
image.naturalHeight && image.naturalHeight > 5 &&
image.naturalWidth && image.naturalWidth > 5 &&
image.displayedWidth &&
image.displayedHeight &&
image.cssComputedObjectFit === 'fill';
}).forEach(image => {
const wellDefinedImage = /** @type {WellDefinedImage} */ (image);
const processed = ImageAspectRatio.computeAspectRatios(wellDefinedImage);
if (i18n.isIcuMessage(processed)) {
warnings.push(processed);
return;
}

if (!processed.doRatiosMatch) results.push(processed);
});
Expand All @@ -126,7 +112,6 @@ class ImageAspectRatio extends Audit {

return {
score: Number(results.length === 0),
warnings,
details: Audit.makeTableDetails(headings, results),
};
}
Expand Down
16 changes: 14 additions & 2 deletions lighthouse-core/audits/image-size-responsive.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ function isCandidate(image) {
if (image.displayedWidth <= 1 || image.displayedHeight <= 1) {
return false;
}
if (image.naturalWidth === 0 || image.naturalHeight === 0) {
if (!image.naturalWidth || !image.naturalHeight) {
return false;
}
if (image.mimeType === 'image/svg+xml') {
Expand All @@ -98,7 +98,17 @@ function isCandidate(image) {
}

/**
* Type check to ensure that the ImageElement has natural dimensions.
*
* @param {LH.Artifacts.ImageElement} image
* @return {image is LH.Artifacts.ImageElement & {naturalWidth: number, naturalHeight: number}}
*/
function imageHasNaturalDimensions(image) {
return image.naturalHeight !== undefined && image.naturalWidth !== undefined;
}

/**
* @param {LH.Artifacts.ImageElement & {naturalHeight: number, naturalWidth: number}} image
* @param {number} DPR
* @return {boolean}
*/
Expand All @@ -109,7 +119,7 @@ function imageHasRightSize(image, DPR) {
}

/**
* @param {LH.Artifacts.ImageElement} image
* @param {LH.Artifacts.ImageElement & {naturalWidth: number, naturalHeight: number}} image
* @param {number} DPR
* @return {Result}
*/
Expand Down Expand Up @@ -222,9 +232,11 @@ class ImageSizeResponsive extends Audit {
*/
static audit(artifacts) {
const DPR = artifacts.ViewportDimensions.devicePixelRatio;

const results = Array
.from(artifacts.ImageElements)
.filter(isCandidate)
.filter(imageHasNaturalDimensions)
.filter(image => !imageHasRightSize(image, DPR))
.filter(image => isVisible(image.clientRect, artifacts.ViewportDimensions))
.map(image => getResult(image, DPR));
Expand Down
7 changes: 2 additions & 5 deletions lighthouse-core/gather/gatherers/image-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ function getHTMLImages(allElements) {
displayedWidth: element.width,
displayedHeight: element.height,
clientRect: getClientRect(element),
naturalWidth: canTrustNaturalDimensions ? element.naturalWidth : 0,
naturalHeight: canTrustNaturalDimensions ? element.naturalHeight : 0,
naturalWidth: canTrustNaturalDimensions ? element.naturalWidth : undefined,
naturalHeight: canTrustNaturalDimensions ? element.naturalHeight : undefined,
attributeWidth: element.getAttribute('width') || '',
attributeHeight: element.getAttribute('height') || '',
cssWidth: undefined, // this will get overwritten below
Expand Down Expand Up @@ -113,9 +113,6 @@ function getCSSImages(allElements) {
displayedWidth: element.clientWidth,
displayedHeight: element.clientHeight,
clientRect: getClientRect(element),
// CSS Images do not expose natural size, we'll determine the size later
naturalWidth: 0,
naturalHeight: 0,
attributeWidth: '',
attributeHeight: '',
cssWidth: undefined,
Expand Down
3 changes: 0 additions & 3 deletions lighthouse-core/lib/i18n/locales/ar-XB.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions lighthouse-core/lib/i18n/locales/ar.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions lighthouse-core/lib/i18n/locales/bg.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions lighthouse-core/lib/i18n/locales/ca.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions lighthouse-core/lib/i18n/locales/cs.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions lighthouse-core/lib/i18n/locales/da.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions lighthouse-core/lib/i18n/locales/de.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions lighthouse-core/lib/i18n/locales/el.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions lighthouse-core/lib/i18n/locales/en-GB.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions lighthouse-core/lib/i18n/locales/en-US.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions lighthouse-core/lib/i18n/locales/en-XA.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions lighthouse-core/lib/i18n/locales/en-XL.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions lighthouse-core/lib/i18n/locales/es-419.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 1ca0e7f

Please sign in to comment.