From 99e85ebfa5e0a7dd458de4134ec6219a1465a3f5 Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Fri, 13 Mar 2020 18:37:08 -0300 Subject: [PATCH 01/18] new_audit: check images are big enough. New audit checking images are big enough. This is a complement to UsesResponsiveImages. Fixes #10434. --- .../audits/image-size-responsive.js | 204 ++++++++++ lighthouse-core/config/default-config.js | 2 + .../test/audits/image-size-responsive-test.js | 363 ++++++++++++++++++ 3 files changed, 569 insertions(+) create mode 100644 lighthouse-core/audits/image-size-responsive.js create mode 100644 lighthouse-core/test/audits/image-size-responsive-test.js diff --git a/lighthouse-core/audits/image-size-responsive.js b/lighthouse-core/audits/image-size-responsive.js new file mode 100644 index 000000000000..248a771408db --- /dev/null +++ b/lighthouse-core/audits/image-size-responsive.js @@ -0,0 +1,204 @@ +/** + * @license Copyright 2020 Sebastian Kreft All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + */ +/** + * @fileoverview Checks to see if the aspect ratio of the images used on + * the page are equal to the aspect ratio of their display sizes. The + * audit will list all images that don't match with their display size + * aspect ratio. + */ +'use strict'; + +const Audit = require('./audit.js'); +const URL = require('../lib/url-shim.js'); +const i18n = require('../lib/i18n/i18n.js'); + +const UIStrings = { + /** Title of a Lighthouse audit that provides detail on the size of visible images on the page. This descriptive title is shown to users when all images have correct sizes. */ + title: 'Displays images with correct size', + /** Title of a Lighthouse audit that provides detail on the size of visible images on the page. This descriptive title is shown to users when not all images have correct sizes. */ + failureTitle: 'Displays images with incorrect size', + /** 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: 'Displayed size', + /** Label for a column in a data table; entries in the column will be the numeric aspect ratio of the raw (actual) image. */ + columnActual: 'Actual size', + /** Label for a column in a data table; entries in the column will be the numeric aspect ratio of the raw (actual) image. */ + columnExpected: 'Expected size', +}; + +const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings); + +const TOLERANCE = 0.75; + +const ALLOWABLE_OFFSCREEN_X = 100; +const ALLOWABLE_OFFSCREEN_Y = 200; + +/** @typedef {{url: string, elidedUrl: string, displayedSize: string, actualSize: string, expectedSize: string, expectedPixels: number}} Result */ + +/** + * @param {{top: number, bottom: number, left: number, right: number}} imageRect + * @param {{innerWidth: number, innerHeight: number}} viewportDimensions + * @return {boolean} + */ +function isVisible(imageRect, viewportDimensions) { + return ( + (imageRect.bottom - imageRect.top) * (imageRect.right - imageRect.left) > 0 && + imageRect.top <= viewportDimensions.innerHeight + ALLOWABLE_OFFSCREEN_Y && + imageRect.bottom >= -ALLOWABLE_OFFSCREEN_Y && + imageRect.left <= viewportDimensions.innerWidth + ALLOWABLE_OFFSCREEN_X && + imageRect.right >= -ALLOWABLE_OFFSCREEN_X + ); +} + +/** + * @param {LH.Artifacts.ImageElement} image + * @return {boolean} + */ +function isCandidate(image) { + if (image.displayedWidth <= 1 || image.displayedHeight <= 1) { + return false; + } + if (image.naturalWidth === 0 || image.naturalHeight === 0) { + return false; + } + if (image.mimeType === 'image/svg+xml') { + return false; + } + if (image.isCss) { + return false; + } + if (image.usesObjectFit) { + return false; + } + return true; +} + +/** + * @param {LH.Artifacts.ImageElement} image + * @param {number} DPR + * @return {boolean} + */ +function imageHasRightSize(image, DPR) { + const [expectedWidth, expectedHeight] = + expectedImageSize(image.displayedWidth, image.displayedHeight, DPR); + return image.naturalWidth >= expectedWidth && image.naturalHeight >= expectedHeight; +} + +/** + * @param {LH.Artifacts.ImageElement} image + * @param {number} DPR + * @return {Result} + */ +function getResult(image, DPR) { + const [expectedWidth, expectedHeight] = + expectedImageSize(image.displayedWidth, image.displayedHeight, DPR); + return { + url: image.src, + elidedUrl: URL.elideDataURI(image.src), + displayedSize: `${image.displayedWidth} x ${image.displayedHeight}`, + actualSize: `${image.naturalWidth} x ${image.naturalHeight}`, + expectedSize: `${expectedWidth} x ${expectedHeight}`, + expectedPixels: expectedWidth * expectedHeight, + }; +} + +/** + * Compute the size an image should have given the display dimensions and pixel density. + * + * For smaller images, typically icons, the size must be proportional to the density. + * For larger images some tolerance is allowed as in thise cases the perceived degradation is not + * that bad. + * + * @param {number} displayedWidth + * @param {number} displayedHeight + * @param {number} DPR + * @return {[number, number]} + */ +function expectedImageSize(displayedWidth, displayedHeight, DPR) { + let factor = DPR; + if (displayedWidth > 64 || displayedHeight > 64) { + factor *= TOLERANCE; + } + const width = Math.ceil(factor * displayedWidth); + const height = Math.ceil(factor * displayedHeight); + return [width, height]; +} + +/** + * Remove repeated entries for the same source and sort them by source. + * + * It will keep the entry with the largest size. + * + * @param {Result[]} results + * @return {Result[]} + */ +function deduplicateAndSortResults(results) { + results.sort((a, b) => a.url === b.url ? 0 : (a.url < b. url ? -1 : 1)); + const deduplicated = /** @type {Result[]} */ ([]); + for (const r of results) { + if (deduplicated.length > 0 && r.url === deduplicated[deduplicated.length - 1].url) { + if (deduplicated[deduplicated.length - 1].expectedPixels < r.expectedPixels) { + deduplicated[deduplicated.length - 1] = r; + } + } else { + deduplicated.push(r); + } + } + return deduplicated; +} + +class ImageSizeResponsive extends Audit { + /** + * @return {LH.Audit.Meta} + */ + static get meta() { + return { + id: 'image-size-responsive', + title: str_(UIStrings.title), + failureTitle: str_(UIStrings.failureTitle), + description: str_(UIStrings.description), + requiredArtifacts: ['ImageElements', 'ViewportDimensions'], + }; + } + + /** + * @param {LH.Artifacts} artifacts + * @return {LH.Audit.Product} + */ + static audit(artifacts) { + const DPR = artifacts.ViewportDimensions.devicePixelRatio; + const results = Array + .from(artifacts.ImageElements) + .filter(isCandidate) + .filter(image => !imageHasRightSize(image, DPR)) + .filter(image => isVisible(image.clientRect, artifacts.ViewportDimensions)) + .map(image => getResult(image, DPR)); + + /** @type {LH.Audit.Details.Table['headings']} */ + const headings = [ + {key: 'url', itemType: 'thumbnail', text: ''}, + {key: 'elidedUrl', itemType: 'url', text: str_(i18n.UIStrings.columnURL)}, + {key: 'displayedSize', itemType: 'text', text: str_(UIStrings.columnDisplayed)}, + {key: 'actualSize', itemType: 'text', text: str_(UIStrings.columnActual)}, + {key: 'expectedSize', itemType: 'text', text: str_(UIStrings.columnExpected)}, + ]; + + return { + score: Number(results.length === 0), + details: Audit.makeTableDetails(headings, deduplicateAndSortResults(results)), + }; + } +} + +module.exports = ImageSizeResponsive; +module.exports.UIStrings = UIStrings; diff --git a/lighthouse-core/config/default-config.js b/lighthouse-core/config/default-config.js index 7a006d0550c5..040ef5290621 100644 --- a/lighthouse-core/config/default-config.js +++ b/lighthouse-core/config/default-config.js @@ -203,6 +203,7 @@ const defaultConfig = { 'maskable-icon', 'content-width', 'image-aspect-ratio', + 'image-size-responsive', 'deprecations', 'mainthread-work-breakdown', 'bootup-time', @@ -521,6 +522,7 @@ const defaultConfig = { {id: 'password-inputs-can-be-pasted-into', weight: 1}, {id: 'errors-in-console', weight: 1}, {id: 'image-aspect-ratio', weight: 1}, + {id: 'image-size-responsive', weight: 1}, ], }, 'seo': { diff --git a/lighthouse-core/test/audits/image-size-responsive-test.js b/lighthouse-core/test/audits/image-size-responsive-test.js new file mode 100644 index 000000000000..36c5ebdd108f --- /dev/null +++ b/lighthouse-core/test/audits/image-size-responsive-test.js @@ -0,0 +1,363 @@ +/** + * @license Copyright 2020 Sebastian Kreft All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + */ +'use strict'; + +const ImageSizeResponsiveAudit = require('../../audits/image-size-responsive.js'); +const assert = require('assert'); + +/* eslint-env jest */ + +const WIDTH = 800; +const HEIGHT = 600; + +function generateImage(clientSize, naturalSize, props, src = 'https://google.com/logo.png') { + const image = {src, mimeType: 'image/png'}; + const clientRect = { + clientRect: { + top: 0, + bottom: clientSize.displayedHeight, + left: 0, + right: clientSize.displayedWidth, + }, + }; + Object.assign(image, clientSize, naturalSize, clientRect, props); + return image; +} + +describe('Images: size audit', () => { + function testImage(condition, data) { + const description = `identifies when an image ${condition}`; + it(description, () => { + const result = ImageSizeResponsiveAudit.audit({ + ImageElements: [ + generateImage( + {displayedWidth: data.clientSize[0], displayedHeight: data.clientSize[1]}, + {naturalWidth: data.naturalSize[0], naturalHeight: data.naturalSize[1]}, + data.props + ), + ], + ViewportDimensions: { + innerWidth: WIDTH, + innerHeight: HEIGHT, + devicePixelRatio: data.devicePixelRatio || 1, + }, + }); + let details = ''; + if (result.score === 0) { + const {displayedSize: displayed, actualSize: actual, expectedSize: expected} = + result.details.items[0]; + details = ` (displayed: ${displayed}, actual: ${actual}, expected: ${expected})`; + } + assert.strictEqual(result.score, data.score, `score does not match${details}`); + }); + } + + testImage('invalid image', { + score: 0, + clientSize: [100, 100], + naturalSize: [5, 5], + }); + + describe('is empty', () => { + testImage('is empty along width', { + score: 1, + clientSize: [100, 100], + naturalSize: [0, 5], + }); + + testImage('is empty along height', { + score: 1, + clientSize: [100, 100], + naturalSize: [5, 0], + }); + }); + + describe('too small', () => { + testImage('is too small along width', { + score: 1, + clientSize: [1, 100], + naturalSize: [5, 5], + }); + + testImage('is too small along height', { + score: 1, + clientSize: [100, 1], + naturalSize: [5, 5], + }); + }); + + testImage('is an SVG image', { + score: 1, + clientSize: [100, 100], + naturalSize: [5, 5], + props: { + mimeType: 'image/svg+xml', + }, + }); + + testImage('is a css image', { + score: 1, + clientSize: [100, 100], + naturalSize: [5, 5], + props: { + isCss: true, + }, + }); + + testImage('uses object-fit', { + score: 1, + clientSize: [100, 100], + naturalSize: [5, 5], + props: { + usesObjectFit: true, + }, + }); + + describe('visibility', () => { + testImage('has no client area', { + score: 1, + clientSize: [100, 100], + naturalSize: [5, 5], + props: { + clientRect: { + top: 0, + bottom: 0, + left: 0, + right: 0, + }, + }, + }); + + testImage('is above the visible area', { + score: 1, + clientSize: [100, 100], + naturalSize: [5, 5], + props: { + clientRect: { + top: -201 - 100, + bottom: -201, + left: 0, + right: 100, + }, + }, + }); + + testImage('is almost above the visible area', { + score: 0, + clientSize: [100, 100], + naturalSize: [5, 5], + props: { + clientRect: { + top: -200 - 100, + bottom: -200, + left: 0, + right: 100, + }, + }, + }); + + testImage('is below the visible area', { + score: 1, + clientSize: [100, 100], + naturalSize: [5, 5], + props: { + clientRect: { + top: HEIGHT + 201, + bottom: HEIGHT + 201 + 100, + left: 0, + right: 100, + }, + }, + }); + + testImage('is almost below the visible area', { + score: 0, + clientSize: [100, 100], + naturalSize: [5, 5], + props: { + clientRect: { + top: HEIGHT + 200, + bottom: HEIGHT + 200 + 100, + left: 0, + right: 100, + }, + }, + }); + + testImage('is to the left of the visible area', { + score: 1, + clientSize: [100, 100], + naturalSize: [5, 5], + props: { + clientRect: { + top: 0, + bottom: 100, + left: -101 - 100, + right: -101, + }, + }, + }); + + testImage('is almost to the left of the visible area', { + score: 0, + clientSize: [100, 100], + naturalSize: [5, 5], + props: { + clientRect: { + top: 0, + bottom: 100, + left: -100 - 100, + right: -100, + }, + }, + }); + + testImage('is to the right of the visible area', { + score: 1, + clientSize: [100, 100], + naturalSize: [5, 5], + props: { + clientRect: { + top: 0, + bottom: 100, + left: WIDTH + 101, + right: WIDTH + 101 + 100, + }, + }, + }); + + testImage('is almost to the right of the visible area', { + score: 0, + clientSize: [100, 100], + naturalSize: [5, 5], + props: { + clientRect: { + top: 0, + bottom: 100, + left: WIDTH + 100, + right: WIDTH + 100 + 100, + }, + }, + }); + }); + + describe('check size', () => { + describe('DPR = 1', () => { + testImage('is an icon with right size', { + score: 1, + clientSize: [64, 64], + naturalSize: [64, 64], + }); + + testImage('is an icon with an invalid size', { + score: 0, + clientSize: [64, 64], + naturalSize: [63, 63], + }); + + testImage('has right size', { + score: 1, + clientSize: [65, 65], + naturalSize: [49, 49], + }); + + testImage('has an invalid size', { + score: 0, + clientSize: [65, 65], + naturalSize: [48, 48], + }); + }); + + describe('DPR = 2', () => { + testImage('is an icon with right size', { + score: 1, + clientSize: [64, 64], + naturalSize: [128, 128], + devicePixelRatio: 2, + }); + + testImage('is an icon with an invalid size', { + score: 0, + clientSize: [64, 64], + naturalSize: [127, 127], + devicePixelRatio: 2, + }); + + testImage('has right size', { + score: 1, + clientSize: [65, 65], + naturalSize: [98, 98], + devicePixelRatio: 2, + }); + + testImage('has an invalid size', { + score: 0, + clientSize: [65, 65], + naturalSize: [97, 97], + devicePixelRatio: 2, + }); + }); + }); + + it('de-dupes images', () => { + const result = ImageSizeResponsiveAudit.audit({ + ImageElements: [ + generateImage( + {displayedWidth: 80, displayedHeight: 40}, + {naturalWidth: 40, naturalHeight: 20} + ), + generateImage( + {displayedWidth: 160, displayedHeight: 80}, + {naturalWidth: 40, naturalHeight: 20} + ), + generateImage( + {displayedWidth: 60, displayedHeight: 30}, + {naturalWidth: 40, naturalHeight: 20} + ), + ], + ViewportDimensions: { + innerWidth: WIDTH, + innerHeight: HEIGHT, + devicePixelRatio: 1, + }, + }); + assert.equal(result.details.items.length, 1); + assert.equal(result.details.items[0].expectedSize, '120 x 60'); + }); + + it('sorts images', () => { + const result = ImageSizeResponsiveAudit.audit({ + ImageElements: [ + generateImage( + {displayedWidth: 80, displayedHeight: 40}, + {naturalWidth: 40, naturalHeight: 20}, + {}, + 'https://C.com/image.png' + ), + generateImage( + {displayedWidth: 80, displayedHeight: 40}, + {naturalWidth: 40, naturalHeight: 20}, + {}, + 'https://A.com/image.png' + ), + generateImage( + {displayedWidth: 80, displayedHeight: 40}, + {naturalWidth: 40, naturalHeight: 20}, + {}, + 'https://B.com/image.png' + ), + ], + ViewportDimensions: { + innerWidth: WIDTH, + innerHeight: HEIGHT, + devicePixelRatio: 1, + }, + }); + assert.equal(result.details.items.length, 3); + const srcs = result.details.items.map(item => item.url); + assert.deepEqual(Array.from(srcs), srcs.sort()); + }); +}); From ba71dc48a1d755b3d2f01a9ee7142debdc60a8d8 Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Fri, 13 Mar 2020 18:53:36 -0300 Subject: [PATCH 02/18] Fix strings and config test --- .../test/cli/__snapshots__/index-test.js.snap | 7 ++++++ .../audits/image-size-responsive.js | 22 +++++++------------ 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap b/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap index ceb3757ed125..96d9629e2e9e 100644 --- a/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap +++ b/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap @@ -96,6 +96,9 @@ Object { Object { "path": "image-aspect-ratio", }, + Object { + "path": "image-size-responsive", + }, Object { "path": "deprecations", }, @@ -748,6 +751,10 @@ Object { "id": "image-aspect-ratio", "weight": 1, }, + Object { + "id": "image-size-responsive", + "weight": 1, + }, ], "title": "Best Practices", }, diff --git a/lighthouse-core/audits/image-size-responsive.js b/lighthouse-core/audits/image-size-responsive.js index 248a771408db..38c3cbd1c1bc 100644 --- a/lighthouse-core/audits/image-size-responsive.js +++ b/lighthouse-core/audits/image-size-responsive.js @@ -4,10 +4,9 @@ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ /** - * @fileoverview Checks to see if the aspect ratio of the images used on - * the page are equal to the aspect ratio of their display sizes. The - * audit will list all images that don't match with their display size - * aspect ratio. + * @fileoverview Checks to see if the size of the visible images used on + * the page are large enough with respect to the pixel ratio. The + * audit will list all visible images that are too small. */ 'use strict'; @@ -21,18 +20,13 @@ const UIStrings = { /** Title of a Lighthouse audit that provides detail on the size of visible images on the page. This descriptive title is shown to users when not all images have correct sizes. */ failureTitle: 'Displays images with incorrect size', /** 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. */ + description: 'Image natural dimensions should be proportional to the display size and the ' + + 'pixel ratio. [Learn more](https://web.dev/image-size-responsive).', + /** Label for a column in a data table; entries in the column will be a string representing the displayed size of the image. */ columnDisplayed: 'Displayed size', - /** Label for a column in a data table; entries in the column will be the numeric aspect ratio of the raw (actual) image. */ + /** Label for a column in a data table; entries in the column will be a string representing the actual size of the image. */ columnActual: 'Actual size', - /** Label for a column in a data table; entries in the column will be the numeric aspect ratio of the raw (actual) image. */ + /** Label for a column in a data table; entries in the column will be a string representing the expected size of the image. */ columnExpected: 'Expected size', }; From 1ee0afb7c7d6e4890f6747e6f3ce558109acf9bc Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Fri, 13 Mar 2020 19:33:12 -0300 Subject: [PATCH 03/18] Add i18n messages --- lighthouse-core/lib/i18n/locales/en-US.json | 18 ++++++++++++++++++ lighthouse-core/lib/i18n/locales/en-XL.json | 18 ++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/lighthouse-core/lib/i18n/locales/en-US.json b/lighthouse-core/lib/i18n/locales/en-US.json index 6971d2883634..67b079a4a970 100644 --- a/lighthouse-core/lib/i18n/locales/en-US.json +++ b/lighthouse-core/lib/i18n/locales/en-US.json @@ -779,6 +779,24 @@ "lighthouse-core/audits/image-aspect-ratio.js | warningCompute": { "message": "Invalid image sizing information {url}" }, + "lighthouse-core/audits/image-size-responsive.js | columnActual": { + "message": "Actual size" + }, + "lighthouse-core/audits/image-size-responsive.js | columnDisplayed": { + "message": "Displayed size" + }, + "lighthouse-core/audits/image-size-responsive.js | columnExpected": { + "message": "Expected size" + }, + "lighthouse-core/audits/image-size-responsive.js | description": { + "message": "Image natural dimensions should be proportional to the display size and the pixel ratio. [Learn more](https://web.dev/image-size-responsive)." + }, + "lighthouse-core/audits/image-size-responsive.js | failureTitle": { + "message": "Displays images with incorrect size" + }, + "lighthouse-core/audits/image-size-responsive.js | title": { + "message": "Displays images with correct size" + }, "lighthouse-core/audits/installable-manifest.js | description": { "message": "Browsers can proactively prompt users to add your app to their homescreen, which can lead to higher engagement. [Learn more](https://web.dev/installable-manifest)." }, diff --git a/lighthouse-core/lib/i18n/locales/en-XL.json b/lighthouse-core/lib/i18n/locales/en-XL.json index 839a9c7c7ed7..df1c285783ca 100644 --- a/lighthouse-core/lib/i18n/locales/en-XL.json +++ b/lighthouse-core/lib/i18n/locales/en-XL.json @@ -779,6 +779,24 @@ "lighthouse-core/audits/image-aspect-ratio.js | warningCompute": { "message": "Îńv̂ál̂íd̂ ím̂áĝé ŝíẑín̂ǵ îńf̂ór̂ḿât́îón̂ {url}" }, + "lighthouse-core/audits/image-size-responsive.js | columnActual": { + "message": "Âćt̂úâĺ ŝíẑé" + }, + "lighthouse-core/audits/image-size-responsive.js | columnDisplayed": { + "message": "D̂íŝṕl̂áŷéd̂ śîźê" + }, + "lighthouse-core/audits/image-size-responsive.js | columnExpected": { + "message": "Êx́p̂éĉt́êd́ ŝíẑé" + }, + "lighthouse-core/audits/image-size-responsive.js | description": { + "message": "Îḿâǵê ńât́ûŕâĺ d̂ím̂én̂śîón̂ś ŝh́ôúl̂d́ b̂é p̂ŕôṕôŕt̂íôńâĺ t̂ó t̂h́ê d́îśp̂ĺâý ŝíẑé âńd̂ t́ĥé p̂íx̂él̂ ŕât́îó. [L̂éâŕn̂ ḿôŕê](https://web.dev/image-size-responsive)." + }, + "lighthouse-core/audits/image-size-responsive.js | failureTitle": { + "message": "D̂íŝṕl̂áŷś îḿâǵêś ŵít̂h́ îńĉór̂ŕêćt̂ śîźê" + }, + "lighthouse-core/audits/image-size-responsive.js | title": { + "message": "D̂íŝṕl̂áŷś îḿâǵêś ŵít̂h́ ĉór̂ŕêćt̂ śîźê" + }, "lighthouse-core/audits/installable-manifest.js | description": { "message": "B̂ŕôẃŝér̂ś ĉán̂ ṕr̂óâćt̂ív̂él̂ý p̂ŕôḿp̂t́ ûśêŕŝ t́ô ád̂d́ ŷóûŕ âṕp̂ t́ô t́ĥéîŕ ĥóm̂éŝćr̂éêń, ŵh́îćĥ ćâń l̂éâd́ t̂ó ĥíĝh́êŕ êńĝáĝém̂én̂t́. [L̂éâŕn̂ ḿôŕê](https://web.dev/installable-manifest)." }, From 17189b023f89d137c0056629bc4705baa65ffbdb Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Mon, 16 Mar 2020 12:46:59 -0300 Subject: [PATCH 04/18] Rename test case to show intent Co-Authored-By: Patrick Hulce --- lighthouse-core/test/audits/image-size-responsive-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/test/audits/image-size-responsive-test.js b/lighthouse-core/test/audits/image-size-responsive-test.js index 36c5ebdd108f..939cca857135 100644 --- a/lighthouse-core/test/audits/image-size-responsive-test.js +++ b/lighthouse-core/test/audits/image-size-responsive-test.js @@ -75,7 +75,7 @@ describe('Images: size audit', () => { }); }); - describe('too small', () => { + describe('too small to bother testing', () => { testImage('is too small along width', { score: 1, clientSize: [1, 100], From c33b0bc8b10bf32d20a0d713f156f6091992f25d Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Mon, 16 Mar 2020 12:48:02 -0300 Subject: [PATCH 05/18] Update audit description with a better intent Co-Authored-By: Patrick Hulce --- lighthouse-core/audits/image-size-responsive.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/audits/image-size-responsive.js b/lighthouse-core/audits/image-size-responsive.js index 38c3cbd1c1bc..1a16791a7c14 100644 --- a/lighthouse-core/audits/image-size-responsive.js +++ b/lighthouse-core/audits/image-size-responsive.js @@ -21,7 +21,7 @@ const UIStrings = { failureTitle: 'Displays images with incorrect size', /** 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 natural dimensions should be proportional to the display size and the ' + - 'pixel ratio. [Learn more](https://web.dev/image-size-responsive).', + 'pixel ratio to maximize image clarity. [Learn more](https://web.dev/image-size-responsive).', /** Label for a column in a data table; entries in the column will be a string representing the displayed size of the image. */ columnDisplayed: 'Displayed size', /** Label for a column in a data table; entries in the column will be a string representing the actual size of the image. */ From a28598c50be8be9c0555e93fc8194ab2b604bf56 Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Mon, 16 Mar 2020 12:48:19 -0300 Subject: [PATCH 06/18] Fix typo Co-Authored-By: Patrick Hulce --- lighthouse-core/audits/image-size-responsive.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/audits/image-size-responsive.js b/lighthouse-core/audits/image-size-responsive.js index 1a16791a7c14..c6f5eaf55101 100644 --- a/lighthouse-core/audits/image-size-responsive.js +++ b/lighthouse-core/audits/image-size-responsive.js @@ -110,7 +110,7 @@ function getResult(image, DPR) { * Compute the size an image should have given the display dimensions and pixel density. * * For smaller images, typically icons, the size must be proportional to the density. - * For larger images some tolerance is allowed as in thise cases the perceived degradation is not + * For larger images some tolerance is allowed as in those cases the perceived degradation is not * that bad. * * @param {number} displayedWidth From 238c2ded6cfcc5f39d1b2472871673ccfa68de3f Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Mon, 16 Mar 2020 12:48:48 -0300 Subject: [PATCH 07/18] Add comment to deduplication Co-Authored-By: Patrick Hulce --- lighthouse-core/audits/image-size-responsive.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lighthouse-core/audits/image-size-responsive.js b/lighthouse-core/audits/image-size-responsive.js index c6f5eaf55101..b0ad8ce46ede 100644 --- a/lighthouse-core/audits/image-size-responsive.js +++ b/lighthouse-core/audits/image-size-responsive.js @@ -141,6 +141,7 @@ function deduplicateAndSortResults(results) { const deduplicated = /** @type {Result[]} */ ([]); for (const r of results) { if (deduplicated.length > 0 && r.url === deduplicated[deduplicated.length - 1].url) { + // If the URL was the same, this is a duplicate. Keep the largest image. if (deduplicated[deduplicated.length - 1].expectedPixels < r.expectedPixels) { deduplicated[deduplicated.length - 1] = r; } From 8baf3c3b306a86c4b88afe63c6e1f185928cdd6d Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Mon, 16 Mar 2020 13:36:19 -0300 Subject: [PATCH 08/18] PR suggestions --- .../audits/image-size-responsive.js | 78 ++++++++++++++----- .../test/audits/image-size-responsive-test.js | 34 ++++++-- 2 files changed, 86 insertions(+), 26 deletions(-) diff --git a/lighthouse-core/audits/image-size-responsive.js b/lighthouse-core/audits/image-size-responsive.js index b0ad8ce46ede..68c1f94ce0ec 100644 --- a/lighthouse-core/audits/image-size-responsive.js +++ b/lighthouse-core/audits/image-size-responsive.js @@ -16,10 +16,10 @@ const i18n = require('../lib/i18n/i18n.js'); const UIStrings = { /** Title of a Lighthouse audit that provides detail on the size of visible images on the page. This descriptive title is shown to users when all images have correct sizes. */ - title: 'Displays images with correct size', + title: 'Displays images with appropriate size', /** Title of a Lighthouse audit that provides detail on the size of visible images on the page. This descriptive title is shown to users when not all images have correct sizes. */ - failureTitle: 'Displays images with incorrect size', - /** 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. */ + failureTitle: 'Displays images with inappropriate size', + /** Description of a Lighthouse audit that tells the user why they should maintain an appropriate size 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 natural dimensions should be proportional to the display size and the ' + 'pixel ratio to maximize image clarity. [Learn more](https://web.dev/image-size-responsive).', /** Label for a column in a data table; entries in the column will be a string representing the displayed size of the image. */ @@ -32,12 +32,22 @@ const UIStrings = { const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings); -const TOLERANCE = 0.75; +// Factors used to allow for smaller effective density. +// A factor of 1 means the actual device pixel density will be used. +// A factor of 0.5, means half the density is required. For example if the device pixel ratio is 3, +// then the images should have at least a density of 1.5. +const SMALL_IMAGE_FACTOR = 1.0; +const LARGE_IMAGE_FACTOR = 0.75; +// An image has must have both its dimensions lower or equal to the threshold in order to be +// considered SMALL. +const SMALL_IMAGE_THRESHOLD = 64; + +// This constants were taken from the OffscreenImages audit. const ALLOWABLE_OFFSCREEN_X = 100; const ALLOWABLE_OFFSCREEN_Y = 200; -/** @typedef {{url: string, elidedUrl: string, displayedSize: string, actualSize: string, expectedSize: string, expectedPixels: number}} Result */ +/** @typedef {{url: string, elidedUrl: string, displayedSize: string, actualSize: string, actualPixels: number, expectedSize: string, expectedPixels: number}} Result */ /** * @param {{top: number, bottom: number, left: number, right: number}} imageRect @@ -84,7 +94,7 @@ function isCandidate(image) { */ function imageHasRightSize(image, DPR) { const [expectedWidth, expectedHeight] = - expectedImageSize(image.displayedWidth, image.displayedHeight, DPR); + allowedImageSize(image.displayedWidth, image.displayedHeight, DPR); return image.naturalWidth >= expectedWidth && image.naturalHeight >= expectedHeight; } @@ -101,13 +111,15 @@ function getResult(image, DPR) { elidedUrl: URL.elideDataURI(image.src), displayedSize: `${image.displayedWidth} x ${image.displayedHeight}`, actualSize: `${image.naturalWidth} x ${image.naturalHeight}`, + actualPixels: image.naturalWidth * image.naturalHeight, expectedSize: `${expectedWidth} x ${expectedHeight}`, expectedPixels: expectedWidth * expectedHeight, }; } /** - * Compute the size an image should have given the display dimensions and pixel density. + * Compute the size an image should have given the display dimensions and pixel density in order to + * pass the audit. * * For smaller images, typically icons, the size must be proportional to the density. * For larger images some tolerance is allowed as in those cases the perceived degradation is not @@ -118,31 +130,46 @@ function getResult(image, DPR) { * @param {number} DPR * @return {[number, number]} */ -function expectedImageSize(displayedWidth, displayedHeight, DPR) { - let factor = DPR; - if (displayedWidth > 64 || displayedHeight > 64) { - factor *= TOLERANCE; +function allowedImageSize(displayedWidth, displayedHeight, DPR) { + let factor = SMALL_IMAGE_FACTOR; + if (displayedWidth > SMALL_IMAGE_THRESHOLD || displayedHeight > SMALL_IMAGE_THRESHOLD) { + factor = LARGE_IMAGE_FACTOR; } - const width = Math.ceil(factor * displayedWidth); - const height = Math.ceil(factor * displayedHeight); + const width = Math.ceil(factor * DPR * displayedWidth); + const height = Math.ceil(factor * DPR * displayedHeight); return [width, height]; } /** - * Remove repeated entries for the same source and sort them by source. + * Compute the size an image should have given the display dimensions and pixel density. + * + * @param {number} displayedWidth + * @param {number} displayedHeight + * @param {number} DPR + * @return {[number, number]} + */ +function expectedImageSize(displayedWidth, displayedHeight, DPR) { + const width = Math.ceil(DPR * displayedWidth); + const height = Math.ceil(DPR * displayedHeight); + return [width, height]; +} + +/** + * Remove repeated entries for the same source. * * It will keep the entry with the largest size. * * @param {Result[]} results * @return {Result[]} */ -function deduplicateAndSortResults(results) { +function deduplicateResultsByUrl(results) { results.sort((a, b) => a.url === b.url ? 0 : (a.url < b. url ? -1 : 1)); const deduplicated = /** @type {Result[]} */ ([]); for (const r of results) { - if (deduplicated.length > 0 && r.url === deduplicated[deduplicated.length - 1].url) { + const previousResult = deduplicated[deduplicated.length - 1]; + if (previousResult && previousResult.url === r.url) { // If the URL was the same, this is a duplicate. Keep the largest image. - if (deduplicated[deduplicated.length - 1].expectedPixels < r.expectedPixels) { + if (previousResult.expectedPixels < r.expectedPixels) { deduplicated[deduplicated.length - 1] = r; } } else { @@ -152,6 +179,19 @@ function deduplicateAndSortResults(results) { return deduplicated; } +/** + * Remove repeated entries for the same source and sort them by source. + * + * It will keep the entry with the largest size. + * + * @param {Result[]} results + * @return {Result[]} + */ +function sortResultsBySizeDelta(results) { + return results.sort( + (a, b) => (b.expectedPixels - b.actualPixels) - (a.expectedPixels - a.actualPixels)); +} + class ImageSizeResponsive extends Audit { /** * @return {LH.Audit.Meta} @@ -188,9 +228,11 @@ class ImageSizeResponsive extends Audit { {key: 'expectedSize', itemType: 'text', text: str_(UIStrings.columnExpected)}, ]; + const finalResults = sortResultsBySizeDelta(deduplicateResultsByUrl(results)); + return { score: Number(results.length === 0), - details: Audit.makeTableDetails(headings, deduplicateAndSortResults(results)), + details: Audit.makeTableDetails(headings, finalResults), }; } } diff --git a/lighthouse-core/test/audits/image-size-responsive-test.js b/lighthouse-core/test/audits/image-size-responsive-test.js index 939cca857135..47ec8afa5ff5 100644 --- a/lighthouse-core/test/audits/image-size-responsive-test.js +++ b/lighthouse-core/test/audits/image-size-responsive-test.js @@ -325,29 +325,29 @@ describe('Images: size audit', () => { }, }); assert.equal(result.details.items.length, 1); - assert.equal(result.details.items[0].expectedSize, '120 x 60'); + assert.equal(result.details.items[0].expectedSize, '160 x 80'); }); - it('sorts images', () => { + it('sorts images by size delta', () => { const result = ImageSizeResponsiveAudit.audit({ ImageElements: [ generateImage( {displayedWidth: 80, displayedHeight: 40}, {naturalWidth: 40, naturalHeight: 20}, {}, - 'https://C.com/image.png' + 'image1.png' ), generateImage( - {displayedWidth: 80, displayedHeight: 40}, + {displayedWidth: 120, displayedHeight: 60}, {naturalWidth: 40, naturalHeight: 20}, {}, - 'https://A.com/image.png' + 'image2.png' ), generateImage( - {displayedWidth: 80, displayedHeight: 40}, + {displayedWidth: 90, displayedHeight: 45}, {naturalWidth: 40, naturalHeight: 20}, {}, - 'https://B.com/image.png' + 'image3.png' ), ], ViewportDimensions: { @@ -358,6 +358,24 @@ describe('Images: size audit', () => { }); assert.equal(result.details.items.length, 3); const srcs = result.details.items.map(item => item.url); - assert.deepEqual(Array.from(srcs), srcs.sort()); + assert.deepEqual(srcs, ['image2.png', 'image3.png', 'image1.png']); + }); + + it('shows the right expected size', () => { + const result = ImageSizeResponsiveAudit.audit({ + ImageElements: [ + generateImage( + {displayedWidth: 80, displayedHeight: 40}, + {naturalWidth: 40, naturalHeight: 20} + ), + ], + ViewportDimensions: { + innerWidth: WIDTH, + innerHeight: HEIGHT, + devicePixelRatio: 2.71, + }, + }); + assert.equal(result.details.items.length, 1); + assert.equal(result.details.items[0].expectedSize, '217 x 109'); }); }); From c78d095fc4876de9a770e0420d5e755df472a841 Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Mon, 16 Mar 2020 13:47:51 -0300 Subject: [PATCH 09/18] Change is visible to only check within the actual visible screen, to prevent contradictions with the advice given for offscreen images --- .../audits/image-size-responsive.js | 12 +++---- .../test/audits/image-size-responsive-test.js | 32 +++++++++---------- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/lighthouse-core/audits/image-size-responsive.js b/lighthouse-core/audits/image-size-responsive.js index 68c1f94ce0ec..c18e13db0b1d 100644 --- a/lighthouse-core/audits/image-size-responsive.js +++ b/lighthouse-core/audits/image-size-responsive.js @@ -43,10 +43,6 @@ const LARGE_IMAGE_FACTOR = 0.75; // considered SMALL. const SMALL_IMAGE_THRESHOLD = 64; -// This constants were taken from the OffscreenImages audit. -const ALLOWABLE_OFFSCREEN_X = 100; -const ALLOWABLE_OFFSCREEN_Y = 200; - /** @typedef {{url: string, elidedUrl: string, displayedSize: string, actualSize: string, actualPixels: number, expectedSize: string, expectedPixels: number}} Result */ /** @@ -57,10 +53,10 @@ const ALLOWABLE_OFFSCREEN_Y = 200; function isVisible(imageRect, viewportDimensions) { return ( (imageRect.bottom - imageRect.top) * (imageRect.right - imageRect.left) > 0 && - imageRect.top <= viewportDimensions.innerHeight + ALLOWABLE_OFFSCREEN_Y && - imageRect.bottom >= -ALLOWABLE_OFFSCREEN_Y && - imageRect.left <= viewportDimensions.innerWidth + ALLOWABLE_OFFSCREEN_X && - imageRect.right >= -ALLOWABLE_OFFSCREEN_X + imageRect.top <= viewportDimensions.innerHeight && + imageRect.bottom >= 0 && + imageRect.left <= viewportDimensions.innerWidth && + imageRect.right >= 0 ); } diff --git a/lighthouse-core/test/audits/image-size-responsive-test.js b/lighthouse-core/test/audits/image-size-responsive-test.js index 47ec8afa5ff5..153b1c7be437 100644 --- a/lighthouse-core/test/audits/image-size-responsive-test.js +++ b/lighthouse-core/test/audits/image-size-responsive-test.js @@ -137,8 +137,8 @@ describe('Images: size audit', () => { naturalSize: [5, 5], props: { clientRect: { - top: -201 - 100, - bottom: -201, + top: -1 - 100, + bottom: -1, left: 0, right: 100, }, @@ -151,8 +151,8 @@ describe('Images: size audit', () => { naturalSize: [5, 5], props: { clientRect: { - top: -200 - 100, - bottom: -200, + top: - 100, + bottom: 0, left: 0, right: 100, }, @@ -165,8 +165,8 @@ describe('Images: size audit', () => { naturalSize: [5, 5], props: { clientRect: { - top: HEIGHT + 201, - bottom: HEIGHT + 201 + 100, + top: HEIGHT + 1, + bottom: HEIGHT + 1 + 100, left: 0, right: 100, }, @@ -179,8 +179,8 @@ describe('Images: size audit', () => { naturalSize: [5, 5], props: { clientRect: { - top: HEIGHT + 200, - bottom: HEIGHT + 200 + 100, + top: HEIGHT, + bottom: HEIGHT + 100, left: 0, right: 100, }, @@ -195,8 +195,8 @@ describe('Images: size audit', () => { clientRect: { top: 0, bottom: 100, - left: -101 - 100, - right: -101, + left: -1 - 100, + right: -1, }, }, }); @@ -209,8 +209,8 @@ describe('Images: size audit', () => { clientRect: { top: 0, bottom: 100, - left: -100 - 100, - right: -100, + left: -100, + right: 0, }, }, }); @@ -223,8 +223,8 @@ describe('Images: size audit', () => { clientRect: { top: 0, bottom: 100, - left: WIDTH + 101, - right: WIDTH + 101 + 100, + left: WIDTH + 1, + right: WIDTH + 1 + 100, }, }, }); @@ -237,8 +237,8 @@ describe('Images: size audit', () => { clientRect: { top: 0, bottom: 100, - left: WIDTH + 100, - right: WIDTH + 100 + 100, + left: WIDTH, + right: WIDTH + 100, }, }, }); From c0bf731ff7e32e6775a3af0e772cedf74a9a433a Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Mon, 16 Mar 2020 13:51:10 -0300 Subject: [PATCH 10/18] Update strings --- lighthouse-core/lib/i18n/locales/en-US.json | 6 +++--- lighthouse-core/lib/i18n/locales/en-XL.json | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lighthouse-core/lib/i18n/locales/en-US.json b/lighthouse-core/lib/i18n/locales/en-US.json index 67b079a4a970..285d84509100 100644 --- a/lighthouse-core/lib/i18n/locales/en-US.json +++ b/lighthouse-core/lib/i18n/locales/en-US.json @@ -789,13 +789,13 @@ "message": "Expected size" }, "lighthouse-core/audits/image-size-responsive.js | description": { - "message": "Image natural dimensions should be proportional to the display size and the pixel ratio. [Learn more](https://web.dev/image-size-responsive)." + "message": "Image natural dimensions should be proportional to the display size and the pixel ratio to maximize image clarity. [Learn more](https://web.dev/image-size-responsive)." }, "lighthouse-core/audits/image-size-responsive.js | failureTitle": { - "message": "Displays images with incorrect size" + "message": "Displays images with inappropriate size" }, "lighthouse-core/audits/image-size-responsive.js | title": { - "message": "Displays images with correct size" + "message": "Displays images with appropriate size" }, "lighthouse-core/audits/installable-manifest.js | description": { "message": "Browsers can proactively prompt users to add your app to their homescreen, which can lead to higher engagement. [Learn more](https://web.dev/installable-manifest)." diff --git a/lighthouse-core/lib/i18n/locales/en-XL.json b/lighthouse-core/lib/i18n/locales/en-XL.json index df1c285783ca..df8d2e728050 100644 --- a/lighthouse-core/lib/i18n/locales/en-XL.json +++ b/lighthouse-core/lib/i18n/locales/en-XL.json @@ -789,13 +789,13 @@ "message": "Êx́p̂éĉt́êd́ ŝíẑé" }, "lighthouse-core/audits/image-size-responsive.js | description": { - "message": "Îḿâǵê ńât́ûŕâĺ d̂ím̂én̂śîón̂ś ŝh́ôúl̂d́ b̂é p̂ŕôṕôŕt̂íôńâĺ t̂ó t̂h́ê d́îśp̂ĺâý ŝíẑé âńd̂ t́ĥé p̂íx̂él̂ ŕât́îó. [L̂éâŕn̂ ḿôŕê](https://web.dev/image-size-responsive)." + "message": "Îḿâǵê ńât́ûŕâĺ d̂ím̂én̂śîón̂ś ŝh́ôúl̂d́ b̂é p̂ŕôṕôŕt̂íôńâĺ t̂ó t̂h́ê d́îśp̂ĺâý ŝíẑé âńd̂ t́ĥé p̂íx̂él̂ ŕât́îó t̂ó m̂áx̂ím̂íẑé îḿâǵê ćl̂ár̂ít̂ý. [L̂éâŕn̂ ḿôŕê](https://web.dev/image-size-responsive)." }, "lighthouse-core/audits/image-size-responsive.js | failureTitle": { - "message": "D̂íŝṕl̂áŷś îḿâǵêś ŵít̂h́ îńĉór̂ŕêćt̂ śîźê" + "message": "D̂íŝṕl̂áŷś îḿâǵêś ŵít̂h́ îńâṕp̂ŕôṕr̂íât́ê śîźê" }, "lighthouse-core/audits/image-size-responsive.js | title": { - "message": "D̂íŝṕl̂áŷś îḿâǵêś ŵít̂h́ ĉór̂ŕêćt̂ śîźê" + "message": "D̂íŝṕl̂áŷś îḿâǵêś ŵít̂h́ âṕp̂ŕôṕr̂íât́ê śîźê" }, "lighthouse-core/audits/installable-manifest.js | description": { "message": "B̂ŕôẃŝér̂ś ĉán̂ ṕr̂óâćt̂ív̂él̂ý p̂ŕôḿp̂t́ ûśêŕŝ t́ô ád̂d́ ŷóûŕ âṕp̂ t́ô t́ĥéîŕ ĥóm̂éŝćr̂éêń, ŵh́îćĥ ćâń l̂éâd́ t̂ó ĥíĝh́êŕ êńĝáĝém̂én̂t́. [L̂éâŕn̂ ḿôŕê](https://web.dev/installable-manifest)." From db628cc42a7c5ee8017ec6cf6f77fdfba4cca84b Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Tue, 17 Mar 2020 13:33:28 -0300 Subject: [PATCH 11/18] Fix test data --- lighthouse-core/test/results/sample_v2.json | 76 ++++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json index 2d6027310d62..ce07c88cea58 100644 --- a/lighthouse-core/test/results/sample_v2.json +++ b/lighthouse-core/test/results/sample_v2.json @@ -630,6 +630,54 @@ ] } }, + "image-size-responsive": { + "id": "image-size-responsive", + "title": "Displays images with inappropriate size", + "description": "Image natural dimensions should be proportional to the display size and the pixel ratio to maximize image clarity. [Learn more](https://web.dev/image-size-responsive).", + "score": 0, + "scoreDisplayMode": "binary", + "details": { + "type": "table", + "headings": [ + { + "key": "url", + "itemType": "thumbnail", + "text": "" + }, + { + "key": "elidedUrl", + "itemType": "url", + "text": "URL" + }, + { + "key": "displayedSize", + "itemType": "text", + "text": "Displayed size" + }, + { + "key": "actualSize", + "itemType": "text", + "text": "Actual size" + }, + { + "key": "expectedSize", + "itemType": "text", + "text": "Expected size" + } + ], + "items": [ + { + "url": "http://localhost:10200/dobetterweb/lighthouse-480x318.jpg", + "elidedUrl": "http://localhost:10200/dobetterweb/lighthouse-480x318.jpg", + "displayedSize": "480 x 318", + "actualSize": "480 x 318", + "actualPixels": 152640, + "expectedSize": "1260 x 835", + "expectedPixels": 1052100 + } + ] + } + }, "deprecations": { "id": "deprecations", "title": "Uses deprecated APIs", @@ -4179,10 +4227,14 @@ { "id": "image-aspect-ratio", "weight": 1 + }, + { + "id": "image-size-responsive", + "weight": 1 } ], "id": "best-practices", - "score": 0.07 + "score": 0.06 }, "seo": { "title": "SEO", @@ -4810,6 +4862,12 @@ "duration": 100, "entryType": "measure" }, + { + "startTime": 0, + "name": "lh:audit:image-size-responsive", + "duration": 100, + "entryType": "measure" + }, { "startTime": 0, "name": "lh:audit:deprecations", @@ -5763,6 +5821,7 @@ "lighthouse-core/lib/i18n/i18n.js | columnURL": [ "audits[errors-in-console].details.headings[0].text", "audits[image-aspect-ratio].details.headings[1].text", + "audits[image-size-responsive].details.headings[1].text", "audits.deprecations.details.headings[1].text", "audits[bootup-time].details.headings[0].text", "audits[network-rtt].details.headings[0].text", @@ -5884,6 +5943,21 @@ "lighthouse-core/audits/image-aspect-ratio.js | columnActual": [ "audits[image-aspect-ratio].details.headings[3].text" ], + "lighthouse-core/audits/image-size-responsive.js | failureTitle": [ + "audits[image-size-responsive].title" + ], + "lighthouse-core/audits/image-size-responsive.js | description": [ + "audits[image-size-responsive].description" + ], + "lighthouse-core/audits/image-size-responsive.js | columnDisplayed": [ + "audits[image-size-responsive].details.headings[2].text" + ], + "lighthouse-core/audits/image-size-responsive.js | columnActual": [ + "audits[image-size-responsive].details.headings[3].text" + ], + "lighthouse-core/audits/image-size-responsive.js | columnExpected": [ + "audits[image-size-responsive].details.headings[4].text" + ], "lighthouse-core/audits/deprecations.js | failureTitle": [ "audits.deprecations.title" ], From 7c487ba1cbc50e4f2f96ebcfd732bc2bf9ad02d8 Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Tue, 17 Mar 2020 13:36:47 -0300 Subject: [PATCH 12/18] Change license header to The Lighthouse Authors. --- AUTHORS | 1 + lighthouse-core/audits/image-size-responsive.js | 2 +- lighthouse-core/test/audits/image-size-responsive-test.js | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/AUTHORS b/AUTHORS index 8ffc1ed7c61e..effe5f2d0901 100644 --- a/AUTHORS +++ b/AUTHORS @@ -4,3 +4,4 @@ # Name/Organization Google Inc. +Sebastian Kreft diff --git a/lighthouse-core/audits/image-size-responsive.js b/lighthouse-core/audits/image-size-responsive.js index c18e13db0b1d..cd21557b3f50 100644 --- a/lighthouse-core/audits/image-size-responsive.js +++ b/lighthouse-core/audits/image-size-responsive.js @@ -1,5 +1,5 @@ /** - * @license Copyright 2020 Sebastian Kreft All Rights Reserved. + * @license Copyright 2020 The Lighthouse Authors. All Rights Reserved. * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ diff --git a/lighthouse-core/test/audits/image-size-responsive-test.js b/lighthouse-core/test/audits/image-size-responsive-test.js index 153b1c7be437..e3c719e08019 100644 --- a/lighthouse-core/test/audits/image-size-responsive-test.js +++ b/lighthouse-core/test/audits/image-size-responsive-test.js @@ -1,5 +1,5 @@ /** - * @license Copyright 2020 Sebastian Kreft All Rights Reserved. + * @license Copyright 2020 The Lighthouse Authors. All Rights Reserved. * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ From 36476eb430775caedf999b455dcdae268ee08214 Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Tue, 17 Mar 2020 14:41:45 -0300 Subject: [PATCH 13/18] Update proto file --- proto/sample_v2_round_trip.json | 59 ++++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/proto/sample_v2_round_trip.json b/proto/sample_v2_round_trip.json index 0ca76463ea2c..f234e9c95635 100644 --- a/proto/sample_v2_round_trip.json +++ b/proto/sample_v2_round_trip.json @@ -1123,6 +1123,53 @@ "title": "Displays images with incorrect aspect ratio", "warnings": [] }, + "image-size-responsive": { + "description": "Image natural dimensions should be proportional to the display size and the pixel ratio to maximize image clarity. [Learn more](https://web.dev/image-size-responsive).", + "details": { + "headings": [ + { + "itemType": "thumbnail", + "key": "url" + }, + { + "itemType": "url", + "key": "elidedUrl", + "text": "URL" + }, + { + "itemType": "text", + "key": "displayedSize", + "text": "Displayed size" + }, + { + "itemType": "text", + "key": "actualSize", + "text": "Actual size" + }, + { + "itemType": "text", + "key": "expectedSize", + "text": "Expected size" + } + ], + "items": [ + { + "actualPixels": 152640.0, + "actualSize": "480 x 318", + "displayedSize": "480 x 318", + "elidedUrl": "http://localhost:10200/dobetterweb/lighthouse-480x318.jpg", + "expectedPixels": 1052100.0, + "expectedSize": "1260 x 835", + "url": "http://localhost:10200/dobetterweb/lighthouse-480x318.jpg" + } + ], + "type": "table" + }, + "id": "image-size-responsive", + "score": 0.0, + "scoreDisplayMode": "binary", + "title": "Displays images with inappropriate size" + }, "input-image-alt": { "description": "When an image is being used as an `` button, providing alternative text can help screen reader users understand the purpose of the button. [Learn more](https://web.dev/input-image-alt/).", "id": "input-image-alt", @@ -3787,10 +3834,14 @@ { "id": "image-aspect-ratio", "weight": 1.0 + }, + { + "id": "image-size-responsive", + "weight": 1.0 } ], "id": "best-practices", - "score": 0.07, + "score": 0.06, "title": "Best Practices" }, "performance": { @@ -4725,6 +4776,12 @@ "name": "lh:audit:image-aspect-ratio", "startTime": 0.0 }, + { + "duration": 100.0, + "entryType": "measure", + "name": "lh:audit:image-size-responsive", + "startTime": 0.0 + }, { "duration": 100.0, "entryType": "measure", From acc81e59f2cd685f678c6801dc7229fdbb50d26f Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Thu, 19 Mar 2020 12:59:30 -0300 Subject: [PATCH 14/18] Apply suggestions from code review Co-Authored-By: Patrick Hulce --- lighthouse-core/audits/image-size-responsive.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lighthouse-core/audits/image-size-responsive.js b/lighthouse-core/audits/image-size-responsive.js index cd21557b3f50..51d9a2374d3c 100644 --- a/lighthouse-core/audits/image-size-responsive.js +++ b/lighthouse-core/audits/image-size-responsive.js @@ -153,7 +153,7 @@ function expectedImageSize(displayedWidth, displayedHeight, DPR) { /** * Remove repeated entries for the same source. * - * It will keep the entry with the largest size. + * It will keep the entry with the largest expected size. * * @param {Result[]} results * @return {Result[]} @@ -176,9 +176,7 @@ function deduplicateResultsByUrl(results) { } /** - * Remove repeated entries for the same source and sort them by source. - * - * It will keep the entry with the largest size. + * Sort entries in descending order by the magnitude of the size deficit, i.e. most pressing issues listed first. * * @param {Result[]} results * @return {Result[]} From e12f7a5b07f9ca5628c065c5a92e383f743c9f75 Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Thu, 19 Mar 2020 13:35:34 -0300 Subject: [PATCH 15/18] Add TODOs to isCandidate and to imageHasRightSize to avoid false positives. --- lighthouse-core/audits/image-size-responsive.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lighthouse-core/audits/image-size-responsive.js b/lighthouse-core/audits/image-size-responsive.js index 51d9a2374d3c..607237f22ba4 100644 --- a/lighthouse-core/audits/image-size-responsive.js +++ b/lighthouse-core/audits/image-size-responsive.js @@ -65,6 +65,8 @@ function isVisible(imageRect, viewportDimensions) { * @return {boolean} */ function isCandidate(image) { + // TODO: skip images using PixelArt scaling. + // See PR https://github.com/GoogleChrome/lighthouse/pull/10481 if (image.displayedWidth <= 1 || image.displayedHeight <= 1) { return false; } @@ -89,6 +91,9 @@ function isCandidate(image) { * @return {boolean} */ function imageHasRightSize(image, DPR) { + // TODO: in case the image has a srcset with pixel density descriptor, the natural size will not + // be the actual image size but rather the image size divided by the density descriptor. + // We can approximate it with the devicePixelRatio. const [expectedWidth, expectedHeight] = allowedImageSize(image.displayedWidth, image.displayedHeight, DPR); return image.naturalWidth >= expectedWidth && image.naturalHeight >= expectedHeight; From 8b3b48809f4abfce9e7027c6f670c69c1aa17b0b Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Thu, 19 Mar 2020 16:27:58 -0300 Subject: [PATCH 16/18] Add tests for new flags --- .../audits/image-size-responsive.js | 11 ++++++----- .../test/audits/image-size-responsive-test.js | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/lighthouse-core/audits/image-size-responsive.js b/lighthouse-core/audits/image-size-responsive.js index 607237f22ba4..a0bfc79b4237 100644 --- a/lighthouse-core/audits/image-size-responsive.js +++ b/lighthouse-core/audits/image-size-responsive.js @@ -65,8 +65,6 @@ function isVisible(imageRect, viewportDimensions) { * @return {boolean} */ function isCandidate(image) { - // TODO: skip images using PixelArt scaling. - // See PR https://github.com/GoogleChrome/lighthouse/pull/10481 if (image.displayedWidth <= 1 || image.displayedHeight <= 1) { return false; } @@ -82,6 +80,12 @@ function isCandidate(image) { if (image.usesObjectFit) { return false; } + if (image.usesPixelArtScaling) { + return false; + } + if (image.usesSrcSetDensityDescriptor) { + return false; + } return true; } @@ -91,9 +95,6 @@ function isCandidate(image) { * @return {boolean} */ function imageHasRightSize(image, DPR) { - // TODO: in case the image has a srcset with pixel density descriptor, the natural size will not - // be the actual image size but rather the image size divided by the density descriptor. - // We can approximate it with the devicePixelRatio. const [expectedWidth, expectedHeight] = allowedImageSize(image.displayedWidth, image.displayedHeight, DPR); return image.naturalWidth >= expectedWidth && image.naturalHeight >= expectedHeight; diff --git a/lighthouse-core/test/audits/image-size-responsive-test.js b/lighthouse-core/test/audits/image-size-responsive-test.js index e3c719e08019..cc61c27dc6d9 100644 --- a/lighthouse-core/test/audits/image-size-responsive-test.js +++ b/lighthouse-core/test/audits/image-size-responsive-test.js @@ -116,6 +116,24 @@ describe('Images: size audit', () => { }, }); + testImage('uses PixelArt scaling', { + score: 1, + clientSize: [100, 100], + naturalSize: [5, 5], + props: { + usesPixelArtScaling: true, + }, + }); + + testImage('uses srcset density descriptor', { + score: 1, + clientSize: [100, 100], + naturalSize: [5, 5], + props: { + usesSrcSetDensityDescriptor: true, + }, + }); + describe('visibility', () => { testImage('has no client area', { score: 1, From c31e76c4f98cf921c5cefd4dd12831ce81ef4e0a Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Thu, 19 Mar 2020 17:33:31 -0300 Subject: [PATCH 17/18] Add smoke test --- .../test/fixtures/dobetterweb/dbw_tester.html | 13 +++++++++++-- .../dobetterweb/dbw-expectations.js | 18 +++++++++++++++--- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html b/lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html index 727e6543cd85..403af919f2f8 100644 --- a/lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html +++ b/lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html @@ -201,9 +201,18 @@

Do better web tester page

- + - + + + + + + + + + + diff --git a/lighthouse-cli/test/smokehouse/test-definitions/dobetterweb/dbw-expectations.js b/lighthouse-cli/test/smokehouse/test-definitions/dobetterweb/dbw-expectations.js index 139faf8eae9e..8c1bd4d14953 100644 --- a/lighthouse-cli/test/smokehouse/test-definitions/dobetterweb/dbw-expectations.js +++ b/lighthouse-cli/test/smokehouse/test-definitions/dobetterweb/dbw-expectations.js @@ -353,7 +353,19 @@ const expectations = [ details: { items: { 0: { - displayedAspectRatio: /^480 x 57/, + displayedAspectRatio: /^120 x 15/, + url: 'http://localhost:10200/dobetterweb/lighthouse-480x318.jpg?iar1', + }, + length: 1, + }, + }, + }, + 'image-size-responsive': { + score: 0, + details: { + items: { + 0: { + url: 'http://localhost:10200/dobetterweb/lighthouse-480x318.jpg?isr1', }, length: 1, }, @@ -385,10 +397,10 @@ const expectations = [ }, 'dom-size': { score: 1, - numericValue: 143, + numericValue: 147, details: { items: [ - {statistic: 'Total DOM Elements', value: '143'}, + {statistic: 'Total DOM Elements', value: '147'}, {statistic: 'Maximum DOM Depth', value: '4'}, { statistic: 'Maximum Child Elements', From 0aa071917b18faeb6772cb41bdc42ca19fc4efbc Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Thu, 19 Mar 2020 17:56:40 -0300 Subject: [PATCH 18/18] Increase bundle size by 1kiB --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index f05bea6b01bc..3761ba7b729e 100644 --- a/package.json +++ b/package.json @@ -198,7 +198,7 @@ }, { "path": "./dist/lighthouse-dt-bundle.js", - "maxSize": "455 kB" + "maxSize": "456 kB" }, { "path": "./dist/lightrider/report-generator-bundle.js",