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

new_audit: add unsized-images to experimental config #11115

Merged
merged 50 commits into from
Jul 29, 2020
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
c00a579
Added new docTypeError fxn and LHError
lemcardenas Jun 26, 2020
09682f4
starting testing code
lemcardenas Jun 26, 2020
eec825d
starting tests
lemcardenas Jun 29, 2020
a63f813
finished unit tests for getDocTypeError
lemcardenas Jun 30, 2020
40bf826
bugfixing
lemcardenas Jun 30, 2020
28cad19
integrated getDocTypeError into getPageLoadError
lemcardenas Jun 30, 2020
6ac8d69
removed console logging from debugging
lemcardenas Jun 30, 2020
fbc6380
changes from code review
lemcardenas Jul 1, 2020
ba1dedd
more code review changes
lemcardenas Jul 1, 2020
85ac35a
committed new strings for testing
lemcardenas Jul 2, 2020
4f1b321
even more code review changes
lemcardenas Jul 2, 2020
6d36004
Merge branch 'master' into error-nonHTML
lemcardenas Jul 6, 2020
9dd709d
changed naming from nonHtml to notHTML, removed an unnecessary if sta…
lemcardenas Jul 7, 2020
cd32b47
added changed i18n strings
lemcardenas Jul 7, 2020
387649b
Merge branch 'error-nonHTML' of github.com:GoogleChrome/lighthouse in…
lemcardenas Jul 7, 2020
8e3b371
removed regex and added const str for comparison
lemcardenas Jul 8, 2020
6f90641
included a comment about Chrome MIME type normalization
lemcardenas Jul 8, 2020
d8437ca
started fleshing out new audit
lemcardenas Jul 16, 2020
0fca1e6
working audit on basic examples
lemcardenas Jul 16, 2020
37628a8
removed spaces and added titles and comments
lemcardenas Jul 16, 2020
f3295ef
merged with origin
lemcardenas Jul 16, 2020
488f73b
added a file overview
lemcardenas Jul 16, 2020
a6417f1
baked new strings & changed duplicate string assertion
lemcardenas Jul 17, 2020
3cc6c24
feedback fixed from draft pr, sized-images-test
lemcardenas Jul 17, 2020
b5c3679
fixed testing bugs, updated sample json
lemcardenas Jul 17, 2020
18abbc6
test edits and removed from default config
lemcardenas Jul 17, 2020
be69f4c
Merge branch 'master' into unsized-images
lemcardenas Jul 17, 2020
9c16d43
added to experimental config
lemcardenas Jul 17, 2020
1498e2d
fixed ts error and edited exp config
lemcardenas Jul 17, 2020
d874c37
Merge branch 'master' into unsized-images
lemcardenas Jul 20, 2020
bc64ee3
added preliminary css sizing logic
lemcardenas Jul 22, 2020
1d784b0
Merge branch 'master' into unsized-images
lemcardenas Jul 22, 2020
b5bc587
added and shifted comments
lemcardenas Jul 22, 2020
ed14f53
edited UIstrings and filenames
lemcardenas Jul 22, 2020
e5a2c7c
made css props optional in artifacts & refactored image-elements
lemcardenas Jul 23, 2020
30648c0
fixed unsized-images-test
lemcardenas Jul 23, 2020
febcbf9
Merge branch 'master' into unsized-images
lemcardenas Jul 23, 2020
b004ee3
removed contents of image-elements file
lemcardenas Jul 23, 2020
e724369
reverted image-elements to html size attribute changes
lemcardenas Jul 23, 2020
7c80ad3
fixed image-elements indenting
lemcardenas Jul 23, 2020
1e0504c
changed size typedef, isValidCss, added tests
lemcardenas Jul 23, 2020
bf9e377
Merge branch 'master' into unsized-images
lemcardenas Jul 23, 2020
cb50193
fixed isValidCss and added css testing
lemcardenas Jul 24, 2020
7c59358
fixed commenting format
lemcardenas Jul 27, 2020
9c97f25
updated audit description UIString
lemcardenas Jul 27, 2020
a684510
fixed nits
lemcardenas Jul 27, 2020
1aad5e3
resolving merge conflicts; have new ts errors
lemcardenas Jul 28, 2020
f7059a4
Merge branch 'master' into unsized-images
lemcardenas Jul 28, 2020
1504d1d
refactored unsized-images-test
lemcardenas Jul 29, 2020
5d545e4
small change in runAudit
lemcardenas Jul 29, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 101 additions & 0 deletions lighthouse-core/audits/sized-images.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/**
* @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.
*/
'use strict';

const Audit = require('./audit.js');
const i18n = require('./../lib/i18n/i18n.js');
const URL = require('./../lib/url-shim.js');

const UIStrings = {
/** Title of a Lighthouse audit that provides detail on whether all images had width and height attributes. This descriptive title is shown to users when every image has width and height attributes */
title: 'Image elements have `width` and `height` attributes',
/** Title of a Lighthouse audit that provides detail on whether all images had width and height attributes. This descriptive title is shown to users when one or more images does not have width and height attributes */
failureTitle: 'Image elements do not have `width` and `height` attributes',
/** Description of a Lighthouse audit that tells the user why they should include width and height attributes 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: 'Always include width and height attributes on your image elements to reduce layout shifting and improve CLS. [Learn more](https://web.dev/optimize-cls/#images-without-dimensions)',
/** Table column header for the image elements that do not have image and height attributes. */
columnFailingElem: 'Failing Elements',
};

const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);

/**
* @fileoverview
* Audit that checks whether all images have width and height attributes.
*/

class SizedImages extends Audit {
/**
* @return {LH.Audit.Meta}
*/
static get meta() {
return {
id: 'sized-images',
title: str_(UIStrings.title),
failureTitle: str_(UIStrings.failureTitle),
description: str_(UIStrings.description),
requiredArtifacts: ['ImageElements'],
};
}

/**
* @param {string} attr
* @return {boolean}
*/
static isValid(attr) {
// an img size attribute is valid for preventing CLS
// if it is a non-negative, non-zero integer
const NON_NEGATIVE_INT_REGEX = /^\d+$/;
const ZERO_REGEX = /^0+$/;
return NON_NEGATIVE_INT_REGEX.test(attr) && !ZERO_REGEX.test(attr);
}

/**
* @param {LH.Artifacts} artifacts
* @param {LH.Audit.Context} context
* @return {Promise<LH.Audit.Product>}
*/
static async audit(artifacts, context) {
// CSS background-images are ignored for this audit
const images = artifacts.ImageElements.filter(el => !el.isCss);
const unsizedImages = [];

for (const image of images) {
const width = image.attributeWidth;
const height = image.attributeHeight;
// images are considered sized if they have defined & valid values
if (!width || !height || !SizedImages.isValid(width) || !SizedImages.isValid(height)) {
const url = URL.elideDataURI(image.src);
unsizedImages.push({
url,
node: /** @type {LH.Audit.Details.NodeValue} */ ({
type: 'node',
path: image.devtoolsNodePath,
selector: image.selector,
nodeLabel: image.nodeLabel,
snippet: image.snippet,
}),
});
}
}

/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
{key: 'url', itemType: 'thumbnail', text: ''},
{key: 'url', itemType: 'url', text: str_(i18n.UIStrings.columnURL)},
{key: 'node', itemType: 'node', text: str_(UIStrings.columnFailingElem)},
];

return {
score: unsizedImages.length > 0 ? 0 : 1,
notApplicable: images.length === 0,
details: Audit.makeTableDetails(headings, unsizedImages),
};
}
}

module.exports = SizedImages;
module.exports.UIStrings = UIStrings;
2 changes: 2 additions & 0 deletions lighthouse-core/config/default-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ const defaultConfig = {
'content-width',
'image-aspect-ratio',
'image-size-responsive',
'sized-images',
'deprecations',
'mainthread-work-breakdown',
'bootup-time',
Expand Down Expand Up @@ -550,6 +551,7 @@ const defaultConfig = {
{id: 'no-vulnerable-libraries', weight: 1, group: 'best-practices-trust-safety'},
// User Experience
{id: 'password-inputs-can-be-pasted-into', weight: 1, group: 'best-practices-ux'},
{id: 'sized-images', weight: 1, group: 'best-practices-ux'},
{id: 'image-aspect-ratio', weight: 1, group: 'best-practices-ux'},
{id: 'image-size-responsive', weight: 1, group: 'best-practices-ux'},
// Browser Compatibility
Expand Down
26 changes: 25 additions & 1 deletion lighthouse-core/gather/gatherers/image-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const Gatherer = require('./gatherer.js');
const pageFunctions = require('../../lib/page-functions.js');
const Driver = require('../driver.js'); // eslint-disable-line no-unused-vars

/* global window, getElementsInDocument, Image */
/* global window, getElementsInDocument, Image, getNodePath, getNodeSelector, getNodeLabel, getOuterHTMLSnippet */


/** @param {Element} element */
Expand Down Expand Up @@ -51,6 +51,8 @@ function getHTMLImages(allElements) {
clientRect: getClientRect(element),
naturalWidth: element.naturalWidth,
naturalHeight: element.naturalHeight,
attributeWidth: element.getAttribute('width') || '',
attributeHeight: element.getAttribute('height') || '',
isCss: false,
// @ts-ignore: loading attribute not yet added to HTMLImageElement definition.
loading: element.loading,
Expand All @@ -64,6 +66,14 @@ function getHTMLImages(allElements) {
),
// https://html.spec.whatwg.org/multipage/images.html#pixel-density-descriptor
usesSrcSetDensityDescriptor: / \d+(\.\d+)?x/.test(element.srcset),
// @ts-ignore - getNodePath put into scope via stringification
devtoolsNodePath: getNodePath(element),
// @ts-ignore - put into scope via stringification
selector: getNodeSelector(element),
// @ts-ignore - put into scope via stringification
nodeLabel: getNodeLabel(element),
// @ts-ignore - put into scope via stringification
snippet: getOuterHTMLSnippet(element),
};
});
}
Expand Down Expand Up @@ -99,6 +109,8 @@ function getCSSImages(allElements) {
// CSS Images do not expose natural size, we'll determine the size later
naturalWidth: 0,
naturalHeight: 0,
attributeWidth: element.getAttribute('width') || '',
attributeHeight: element.getAttribute('height') || '',
isCss: true,
isPicture: false,
usesObjectFit: false,
Expand All @@ -107,6 +119,14 @@ function getCSSImages(allElements) {
),
usesSrcSetDensityDescriptor: false,
resourceSize: 0, // this will get overwritten below
// @ts-ignore - getNodePath put into scope via stringification
devtoolsNodePath: getNodePath(element),
// @ts-ignore - put into scope via stringification
selector: getNodeSelector(element),
// @ts-ignore - put into scope via stringification
nodeLabel: getNodeLabel(element),
// @ts-ignore - put into scope via stringification
snippet: getOuterHTMLSnippet(element),
});
}

Expand Down Expand Up @@ -192,6 +212,10 @@ class ImageElements extends Gatherer {

const expression = `(function() {
${pageFunctions.getElementsInDocumentString}; // define function on page
${pageFunctions.getNodePathString};
${pageFunctions.getNodeSelectorString};
${pageFunctions.getNodeLabelString};
${pageFunctions.getOuterHTMLSnippetString};
${getClientRect.toString()};
${getHTMLImages.toString()};
${getCSSImages.toString()};
Expand Down
12 changes: 12 additions & 0 deletions lighthouse-core/lib/i18n/locales/en-US.json
Original file line number Diff line number Diff line change
Expand Up @@ -1205,6 +1205,18 @@
"lighthouse-core/audits/service-worker.js | title": {
"message": "Registers a service worker that controls page and `start_url`"
},
"lighthouse-core/audits/sized-images.js | columnFailingElem": {
"message": "Failing Elements"
},
"lighthouse-core/audits/sized-images.js | description": {
"message": "Always include width and height attributes on your image elements to reduce layout shifting and improve CLS. [Learn more](https://web.dev/optimize-cls/#images-without-dimensions)"
},
"lighthouse-core/audits/sized-images.js | failureTitle": {
"message": "Image elements do not have `width` and `height` attributes"
},
"lighthouse-core/audits/sized-images.js | title": {
"message": "Image elements have `width` and `height` attributes"
},
"lighthouse-core/audits/splash-screen.js | description": {
"message": "A themed splash screen ensures a high-quality experience when users launch your app from their homescreens. [Learn more](https://web.dev/splash-screen/)."
},
Expand Down
12 changes: 12 additions & 0 deletions lighthouse-core/lib/i18n/locales/en-XL.json
Original file line number Diff line number Diff line change
Expand Up @@ -1205,6 +1205,18 @@
"lighthouse-core/audits/service-worker.js | title": {
"message": "R̂éĝíŝt́êŕŝ á ŝér̂v́îćê ẃôŕk̂ér̂ t́ĥát̂ ćôńt̂ŕôĺŝ ṕâǵê án̂d́ `start_url`"
},
"lighthouse-core/audits/sized-images.js | columnFailingElem": {
"message": "F̂áîĺîńĝ Él̂ém̂én̂t́ŝ"
},
"lighthouse-core/audits/sized-images.js | description": {
"message": "Âĺŵáŷś îńĉĺûd́ê ẃîd́t̂h́ âńd̂ h́êíĝh́t̂ át̂t́r̂íb̂út̂éŝ ón̂ ýôúr̂ ím̂áĝé êĺêḿêńt̂ś t̂ó r̂éd̂úĉé l̂áŷóût́ ŝh́îf́t̂ín̂ǵ âńd̂ ím̂ṕr̂óv̂é ĈĹŜ. [Ĺêár̂ń m̂ór̂é](https://web.dev/optimize-cls/#images-without-dimensions)"
},
"lighthouse-core/audits/sized-images.js | failureTitle": {
"message": "Îḿâǵê él̂ém̂én̂t́ŝ d́ô ńôt́ ĥáv̂é `width` âńd̂ `height` át̂t́r̂íb̂út̂éŝ"
},
"lighthouse-core/audits/sized-images.js | title": {
"message": "Îḿâǵê él̂ém̂én̂t́ŝ h́âv́ê `width` án̂d́ `height` ât́t̂ŕîb́ût́êś"
},
"lighthouse-core/audits/splash-screen.js | description": {
"message": "Â t́ĥém̂éd̂ śp̂ĺâśĥ śĉŕêén̂ én̂śûŕêś â h́îǵĥ-q́ûál̂ít̂ý êx́p̂ér̂íêńĉé ŵh́êń ûśêŕŝ ĺâún̂ćĥ ýôúr̂ áp̂ṕ f̂ŕôḿ t̂h́êír̂ h́ôḿêśĉŕêén̂ś. [L̂éâŕn̂ ḿôŕê](https://web.dev/splash-screen/)."
},
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/scripts/i18n/collect-strings.js
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,7 @@ if (require.main === module) {

if ((collisions) > 0) {
console.log(`MEANING COLLISION: ${collisions} string(s) have the same content.`);
assert.equal(collisions, 16, `The number of duplicate strings have changed, update this assertion if that is expected, or reword strings. Collisions: ${collisionStrings}`);
assert.equal(collisions, 17, `The number of duplicate strings have changed, update this assertion if that is expected, or reword strings. Collisions: ${collisionStrings}`);
}

const strings = {...coreStrings, ...stackPackStrings};
Expand Down
9 changes: 9 additions & 0 deletions types/artifacts.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,10 @@ declare global {
naturalWidth: number;
/** The natural height of the underlying image, uses img.naturalHeight. See https://codepen.io/patrickhulce/pen/PXvQbM for examples. */
naturalHeight: number;
/** The attribute width of the image, ... */
attributeWidth: string;
/** The attribute height of the image, ... */
attributeHeight: string;
/** The BoundingClientRect of the element. */
clientRect: {
top: number;
Expand All @@ -423,6 +427,11 @@ declare global {
usesSrcSetDensityDescriptor: boolean;
/** The size of the underlying image file in bytes. 0 if the file could not be identified. */
resourceSize: number;
/** Path that uniquely identifies the node in the DOM */
devtoolsNodePath: string;
snippet: string;
selector: string;
nodeLabel: string;
/** The MIME type of the underlying image file. */
mimeType?: string;
/** The loading attribute of the image. */
Expand Down