diff --git a/lighthouse-cli/test/fixtures/byte-efficiency/tester.html b/lighthouse-cli/test/fixtures/byte-efficiency/tester.html index 17e1372dcde4..15df1431bd00 100644 --- a/lighthouse-cli/test/fixtures/byte-efficiency/tester.html +++ b/lighthouse-cli/test/fixtures/byte-efficiency/tester.html @@ -102,7 +102,7 @@

Byte efficiency tester page

- + @@ -130,15 +130,15 @@

Byte efficiency tester page

- - - + + + -
+
- +
diff --git a/lighthouse-cli/test/smokehouse/byte-efficiency/expectations.js b/lighthouse-cli/test/smokehouse/byte-efficiency/expectations.js index 3da2d9062e2e..649b674e44ae 100644 --- a/lighthouse-cli/test/smokehouse/byte-efficiency/expectations.js +++ b/lighthouse-cli/test/smokehouse/byte-efficiency/expectations.js @@ -83,7 +83,7 @@ module.exports = [ details: { overallSavingsBytes: '>60000', items: { - length: 4, + length: 5, }, }, }, @@ -106,14 +106,14 @@ module.exports = [ }, }, 'uses-responsive-images': { - displayValue: 'Potential savings of 75\xa0KB', + displayValue: 'Potential savings of 69\xa0KB', details: { - overallSavingsBytes: '>75000', - items: [ - {wastedPercent: '<60'}, - {wastedPercent: '<60'}, - {wastedPercent: '<60'}, - ], + overallSavingsBytes: '>65000', + items: { + 0: {wastedPercent: '<60'}, + 1: {wastedPercent: '<60'}, + length: 2, + }, }, }, }, diff --git a/lighthouse-core/audits/byte-efficiency/uses-responsive-images.js b/lighthouse-core/audits/byte-efficiency/uses-responsive-images.js index 139f26c733a2..6bf2bb16a448 100644 --- a/lighthouse-core/audits/byte-efficiency/uses-responsive-images.js +++ b/lighthouse-core/audits/byte-efficiency/uses-responsive-images.js @@ -94,19 +94,22 @@ class UsesResponsiveImages extends ByteEfficiencyAudit { const warnings = []; /** @type {Map} */ const resultsMap = new Map(); - images.forEach(image => { - // TODO: give SVG a free pass until a detail per pixel metric is available - if (!image.resourceSize || image.mimeType === 'image/svg+xml') { - return; + for (const image of images) { + // Ignore images without resource size information. + // 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, + // and the reward-to-effort ratio for responsive CSS images is quite low https://css-tricks.com/responsive-images-css/. + if (!image.resourceSize || image.mimeType === 'image/svg+xml' || image.isCss) { + continue; } const processed = UsesResponsiveImages.computeWaste(image, DPR); - if (!processed) return; + if (!processed) continue; if (processed instanceof Error) { warnings.push(processed.message); Sentry.captureException(processed, {tags: {audit: this.meta.id}, level: 'warning'}); - return; + continue; } // Don't warn about an image that was later used appropriately @@ -114,7 +117,7 @@ class UsesResponsiveImages extends ByteEfficiencyAudit { if (!existing || existing.wastedBytes > processed.wastedBytes) { resultsMap.set(processed.url, processed); } - }); + } const items = Array.from(resultsMap.values()) .filter(item => item.wastedBytes > IGNORE_THRESHOLD_IN_BYTES); diff --git a/lighthouse-core/gather/gatherers/image-elements.js b/lighthouse-core/gather/gatherers/image-elements.js index 3a49b0f241e2..3c321a2b3c90 100644 --- a/lighthouse-core/gather/gatherers/image-elements.js +++ b/lighthouse-core/gather/gatherers/image-elements.js @@ -15,30 +15,31 @@ const Driver = require('../driver.js'); // eslint-disable-line no-unused-vars /* global window, getElementsInDocument, Image */ -/** @return {Array} */ + +/** @param {Element} element */ /* istanbul ignore next */ -function collectImageElementInfo() { - /** @param {Element} element */ - function getClientRect(element) { - const clientRect = element.getBoundingClientRect(); - return { - // Just grab the DOMRect properties we want, excluding x/y/width/height - top: clientRect.top, - bottom: clientRect.bottom, - left: clientRect.left, - right: clientRect.right, - }; - } +function getClientRect(element) { + const clientRect = element.getBoundingClientRect(); + return { + // Just grab the DOMRect properties we want, excluding x/y/width/height + top: clientRect.top, + bottom: clientRect.bottom, + left: clientRect.left, + right: clientRect.right, + }; +} - /** @type {Array} */ - // @ts-ignore - added by getElementsInDocumentFnString - const allElements = getElementsInDocument(); +/** + * @param {Array} allElements + * @return {Array} + */ +/* istanbul ignore next */ +function getHTMLImages(allElements) { const allImageElements = /** @type {Array} */ (allElements.filter(element => { return element.localName === 'img'; })); - /** @type {Array} */ - const htmlImages = allImageElements.map(element => { + return allImageElements.map(element => { const computedStyle = window.getComputedStyle(element); return { // currentSrc used over src to get the url as determined by the browser @@ -57,30 +58,30 @@ function collectImageElementInfo() { ), }; }); +} +/** + * @param {Array} allElements + * @return {Array} + */ +/* istanbul ignore next */ +function getCSSImages(allElements) { // Chrome normalizes background image style from getComputedStyle to be an absolute URL in quotes. // Only match basic background-image: url("http://host/image.jpeg") declarations const CSS_URL_REGEX = /^url\("([^"]+)"\)$/; - // Only find images that aren't specifically scaled - const CSS_SIZE_REGEX = /(auto|contain|cover)/; - const cssImages = allElements.reduce((images, element) => { + /** @type {Array} */ + const images = []; + + for (const element of allElements) { const style = window.getComputedStyle(element); - if (!style.backgroundImage || !CSS_URL_REGEX.test(style.backgroundImage) || - !style.backgroundSize || !CSS_SIZE_REGEX.test(style.backgroundSize)) { - return images; - } + // If the element didn't have a CSS background image, we're not interested. + if (!style.backgroundImage || !CSS_URL_REGEX.test(style.backgroundImage)) continue; const imageMatch = style.backgroundImage.match(CSS_URL_REGEX); // @ts-ignore test() above ensures that there is a match. const url = imageMatch[1]; - // Heuristic to filter out sprite sheets - const differentImages = images.filter(image => image.src !== url); - if (images.length - differentImages.length > 2) { - return differentImages; - } - images.push({ src: url, displayedWidth: element.clientWidth, @@ -94,11 +95,18 @@ function collectImageElementInfo() { usesObjectFit: false, resourceSize: 0, // this will get overwritten below }); + } - return images; - }, /** @type {Array} */ ([])); + return images; +} - return htmlImages.concat(cssImages); +/** @return {Array} */ +/* istanbul ignore next */ +function collectImageElementInfo() { + /** @type {Array} */ + // @ts-ignore - added by getElementsInDocumentFnString + const allElements = getElementsInDocument(); + return getHTMLImages(allElements).concat(getCSSImages(allElements)); } /** @@ -160,16 +168,23 @@ class ImageElements extends Gatherer { const expression = `(function() { ${pageFunctions.getElementsInDocumentString}; // define function on page - return (${collectImageElementInfo.toString()})(); + ${getClientRect.toString()}; + ${getHTMLImages.toString()}; + ${getCSSImages.toString()}; + ${collectImageElementInfo.toString()}; + + return collectImageElementInfo(); })()`; /** @type {Array} */ const elements = await driver.evaluateAsync(expression); + /** @type {Array} */ const imageUsage = []; const top50Images = Object.values(indexedNetworkRecords) .sort((a, b) => b.resourceSize - a.resourceSize) .slice(0, 50); + for (let element of elements) { // Pull some of our information directly off the network record. const networkRecord = indexedNetworkRecords[element.src] || {}; diff --git a/lighthouse-core/test/audits/byte-efficiency/uses-responsive-images-test.js b/lighthouse-core/test/audits/byte-efficiency/uses-responsive-images-test.js index 68aec09f8811..a10e7b69717d 100644 --- a/lighthouse-core/test/audits/byte-efficiency/uses-responsive-images-test.js +++ b/lighthouse-core/test/audits/byte-efficiency/uses-responsive-images-test.js @@ -147,6 +147,37 @@ describe('Page uses responsive images', () => { assert.equal(auditResult.items.length, 0); }); + it('ignores CSS', () => { + const urlA = 'https://google.com/logo.png'; + const naturalSizeA = generateSize(450, 450, 'natural'); + const recordA = generateRecord(100, 300); + + const auditResult = UsesResponsiveImagesAudit.audit_({ + ViewportDimensions: {devicePixelRatio: 1}, + ImageElements: [ + {...generateImage(generateSize(10, 10), naturalSizeA, recordA, urlA), isCss: true}, + ], + }); + + assert.equal(auditResult.items.length, 0); + }); + + it('handles failure', () => { + const urlA = 'https://google.com/logo.png'; + const naturalSizeA = generateSize(NaN, 450, 'natural'); + const recordA = generateRecord(100, 300); + + const auditResult = UsesResponsiveImagesAudit.audit_({ + ViewportDimensions: {devicePixelRatio: 1}, + ImageElements: [ + generateImage(generateSize(10, 10), naturalSizeA, recordA, urlA), + ], + }); + + assert.equal(auditResult.items.length, 0); + assert.equal(auditResult.warnings.length, 1); + }); + it('de-dupes images', () => { const urlA = 'https://google.com/logo.png'; const naturalSizeA = generateSize(450, 450, 'natural');