Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

core(image-elements): collect CSS sizing, ShadowRoot, & position #11188

Merged
merged 81 commits into from
Aug 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
81 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
e8ff1d0
fixed small bugs in image-elements
lemcardenas Jul 24, 2020
f531a74
changed size typedef, isValidCss, added tests
lemcardenas Jul 23, 2020
efb033d
fixed isValidCss and added css testing
lemcardenas Jul 24, 2020
7b4753c
removed an empty string branch in findMostSpecificMatched
lemcardenas Jul 27, 2020
6f9c280
Merge branch 'master' into image-elements-css-sizing
lemcardenas Jul 27, 2020
9f4b2c9
first attempt at smoke test
lemcardenas Jul 27, 2020
e4a6e23
fixed commenting format
lemcardenas Jul 27, 2020
cfdb2be
updated audit description UIString
lemcardenas Jul 27, 2020
a578844
fixed nits
lemcardenas Jul 27, 2020
3e0098c
working smoke test
lemcardenas Jul 28, 2020
8a83084
Merge branch 'master' into image-elements-css-sizing
lemcardenas Jul 28, 2020
f969904
updated sample json
lemcardenas Jul 28, 2020
2aa3f61
fixed index snapshot
lemcardenas Jul 28, 2020
6acbbfd
fixed default config
lemcardenas Jul 28, 2020
d4574f1
update sample json
lemcardenas Jul 28, 2020
430d97f
new comments
lemcardenas Jul 29, 2020
0d1b43b
refactored unsized-images-test
lemcardenas Jul 29, 2020
c5a1b9c
small change in runAudit
lemcardenas Jul 29, 2020
abf0d74
Merge branch 'master' into image-elements-css-sizing
lemcardenas Jul 29, 2020
e77d35f
comment changes
lemcardenas Jul 29, 2020
cc80c1d
spacing changes
lemcardenas Jul 29, 2020
6fb5c39
removed unsized-images out of experimental config
lemcardenas Jul 29, 2020
9613e4f
removed unsized-images from default config
lemcardenas Jul 29, 2020
c1c7dc1
byte-config extends experimental-config
lemcardenas Jul 29, 2020
66e491e
comment changes, typedef change
lemcardenas Jul 29, 2020
20fb350
added a shadowroot unit test
lemcardenas Jul 29, 2020
9f260e5
added two extra image sizing tests in byte-efficiency
lemcardenas Jul 29, 2020
44cbb66
added unique urls for images in smoke test
lemcardenas Jul 30, 2020
e3b13c9
refactored byte-config to not use experimental-config
lemcardenas Jul 30, 2020
dbe1812
changed isShadow to isInShadowDOM
lemcardenas Jul 30, 2020
ae7ac76
refactored findMostSpecificMatchedCSSRule to take extra param
lemcardenas Jul 31, 2020
e19ab30
Merge branch 'master' into image-elements-css-sizing
lemcardenas Jul 31, 2020
9133fff
moved driver commands inside a try/catch
lemcardenas Jul 31, 2020
490cc03
added positioning checking for the audit
lemcardenas Jul 31, 2020
48d39ed
made audit description less imperative, added tests
lemcardenas Jul 31, 2020
c88e876
removed extra line
lemcardenas Jul 31, 2020
9bb9849
fixed naming convention
lemcardenas Jul 31, 2020
6e18cbe
nits, positioning -> cssComputedPosition, spacing
lemcardenas Aug 3, 2020
34080a9
changed @function -> @param
lemcardenas Aug 3, 2020
d00ceb0
Merge branch 'master' into image-elements-css-sizing
lemcardenas Aug 3, 2020
1763d26
changed cssWidth/Height to be optional
lemcardenas Aug 4, 2020
dc7ebf5
changed initialization of cssWidth/Height to be undefined
lemcardenas Aug 4, 2020
0e332ea
Update lighthouse-core/audits/unsized-images.js
lemcardenas Aug 4, 2020
c39f687
Merge branch 'master' into image-elements-css-sizing
lemcardenas Aug 5, 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
32 changes: 32 additions & 0 deletions lighthouse-cli/test/fixtures/byte-efficiency/tester.html
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ <h2>Byte efficiency tester page</h2>
<!-- PASS(responsive): image is used at full size -->
<!-- PASS(responsive-inverse): image is not visible -->
<!-- FAIL(offscreen): image is offscreen -->
<!-- PASS(unsized-images): css position is absolute -->
<img style="position: absolute; top: -10000px;" src="lighthouse-unoptimized.jpg">

<!-- Image is cross-origin, which are disabled for now -->
Expand All @@ -90,41 +91,63 @@ <h2>Byte efficiency tester page</h2>
<!-- FAIL(responsive): image is 50% used at DPR 3 -->
<!-- PASS(responsive-inverse): image is big enough -->
<!-- PASS(offscreen): image is onscreen -->
<!-- PASS(unsized-images): has CSS sizing -->
<img style="width: 256px; height: 170px;" src="lighthouse-1024x680.jpg">

<!-- PASS(optimized): image is JPEG optimized -->
<!-- PASS(webp): image is WebP optimized -->
<!-- FAIL(responsive): image is 25% used at DPR 2.625 -->
<!-- PASS(responsive-inverse): image is big enough -->
<!-- PASS(offscreen): image is offscreen but lazily loaded -->
<!-- PASS(unsized-images): CSS sizing is valid with current checks -->
<img style="width: 0px; height: 0px;" src="lighthouse-2048x1356.webp?size0" loading="lazy">

<!-- PASS(optimized): image is JPEG optimized -->
<!-- FAIL(webp): image is not WebP optimized -->
<!-- FAIL(responsive): image is not fully used at DPR 2.625 -->
<!-- PASS(responsive-inverse): image is big enough -->
<!-- PASS(offscreen): image is onscreen -->
<!-- PASS(unsized-images): has CSS sizing -->
<img style="width: 160px; height: 110px;" src="lighthouse-480x320.jpg">

<!-- PASS(optimized): image is JPEG optimized -->
<!-- FAIL(webp): image is not WebP optimized -->
<!-- FAIL(responsive): image is not fully used at DPR 2.625 -->
<!-- PASS(responsive-inverse): image is big enough -->
<!-- PASS(offscreen): image is onscreen -->
<!-- PASS(unsized-images): has attribute sizing -->
<img width="160" height="110" src="lighthouse-480x320.jpg?attributesized">

<!-- PASS(optimized): image is JPEG optimized -->
<!-- PASS(webp): image has insigificant WebP savings -->
<!-- PASS(responsive): image is used at full size -->
<!-- FAIL(responsive-inverse): image does not account for DPR 2.625 -->
<!-- PASS(offscreen): image is onscreen -->
<!-- FAIL(unsized-images): no sizing information -->
<img src="lighthouse-320x212-poor.jpg">

<!-- PASS(optimized): image is JPEG optimized -->
<!-- PASS(webp): image has insigificant WebP savings -->
<!-- PASS(responsive): image is used at full size -->
<!-- FAIL(responsive-inverse): image does not account for DPR 2.625 -->
<!-- PASS(offscreen): image is onscreen -->
<!-- FAIL(unsized-images): invalid CSS sizing -->
<img style="width: auto; height: auto;" src="lighthouse-320x212-poor.jpg?cssauto">

<!-- PASS(optimized): image is JPEG optimized -->
<!-- PASS(webp): image is WebP savings -->
<!-- PASS(responsive): image is used at full size with srcset -->
<!-- PASS(responsive-inverse): image is big enough -->
<!-- PASS(offscreen): image is onscreen -->
<!-- PASS(unsized-images): css position is absolute -->
<img class="onscreen" sizes="(max-width: 2000px) 160px, 2048px" src="lighthouse-320x212-poor.jpg?srcset" srcset="lighthouse-480x320.webp?srcset 480w, lighthouse-2048x1356.webp?srcset 2048w">

<!-- PASS(optimized): image is JPEG optimized -->
<!-- PASS(webp): image is WebP savings -->
<!-- PASS(responsive): image is used at full size with <picture> -->
<!-- PASS(responsive-inverse): image is big enough -->
<!-- PASS(offscreen): image is onscreen -->
<!-- PASS(unsized-images): css position is absolute -->
<picture class="onscreen">
<source media="(max-width: 2000px)" srcset="lighthouse-480x320.webp?picture">
<source media="(min-width: 2001px)" srcset="lighthouse-2048x1356.webp?picture">
Expand All @@ -136,55 +159,63 @@ <h2>Byte efficiency tester page</h2>
<!-- FAIL(responsive): image is 50% used at DPR 2.625 (but small savings) -->
<!-- PASS(responsive-inverse): image is big enough -->
<!-- FAIL(offscreen): image is offscreen -->
<!-- PASS(unsized-images): has CSS sizing -->
<img style="margin-top: 4000px; width: 120px; height: 80px;" src="lighthouse-480x320.webp">

<!-- PASS(optimized): image is JPEG optimized -->
<!-- PASS(webp): image is WebP optimized -->
<!-- PASS(responsive): image is not visible -->
<!-- PASS(responsive-inverse): is not visible -->
<!-- FAIL(offscreen): image is not visible -->
<!-- PASS(unsized-images): has CSS sizing -->
<div class="onscreen" style="display: none;"><img class="onscreen" style="width: 120px; height: 80px;" src="lighthouse-480x320.webp?invisible"></div>

<!-- PASS(optimized): image is vector -->
<!-- PASS(webp): image is vector -->
<!-- PASS(responsive): image is vector -->
<!-- PASS(responsive-inverse): is vector -->
<!-- FAIL(offscreen): image is offscreen -->
<!-- PASS(unsized-images): has CSS sizing -->
<img style="width: 100px; height: 100px;" src="large.svg">

<!-- PASS(optimized): image is vector -->
<!-- PASS(webp): image is vector -->
<!-- PASS(responsive): image is vector -->
<!-- PASS(responsive-inverse): is vector -->
<!-- PASS(offscreen): image is offscreen and loads before TTI, but loads lazily -->
<!-- PASS(unsized-images): has CSS sizing -->
<img style="width: 100px; height: 100px;" src="large.svg?nativeLazyLoad" loading="lazy">

<!-- PASS(optimized): image is JPEG optimized -->
<!-- PASS(webp): image has insigificant WebP savings -->
<!-- PASS(responsive): image is later used at full size -->
<!-- PASS(responsive-inverse): image is big enough -->
<!-- PASS(offscreen): image is later used onscreen -->
<!-- PASS(unsized-images): has CSS sizing -->
<img style="width: 24px; height: 16px;"src="lighthouse-320x212-poor.jpg?duplicate">

<!-- PASS(optimized): image is JPEG optimized -->
<!-- PASS(webp): image has insigificant WebP savings -->
<!-- PASS(responsive): image is used at full size -->
<!-- FAIL(responsive-inverse): image does not account for DPR 2.625 -->
<!-- PASS(offscreen): image is onscreen -->
<!-- PASS(unsized-images): css position is absolute -->
<img class="onscreen" src="lighthouse-320x212-poor.jpg?duplicate">

<!-- PASS(optimized): image is JPEG optimized -->
<!-- FAIL(webp): image is not WebP optimized -->
<!-- PASS(responsive): image is in CSS -->
<!-- PASS(responsive-inverse): image is in CSS -->
<!-- PASS(offscreen): image is onscreen -->
<!-- PASS(unsized-images): CSS background images are ignored -->
<div class="onscreen" style="width: 120px; height: 80px; background: 50% 50% url(lighthouse-480x320.jpg?css);"></div>

<!-- PASS(optimized): image is JPEG optimized -->
<!-- FAIL(webp): image is not WebP optimized -->
<!-- PASS(responsive): image is in CSS -->
<!-- PASS(responsive-inverse): image is in CSS -->
<!-- PASS(offscreen): image is onscreen -->
<!-- PASS(unsized-images): CSS background images are ignored -->
<div class="onscreen" style="width: 30px; height: 30px; background: 0% 50% url(lighthouse-480x320.jpg?sprite);"></div>
<div class="onscreen" style="width: 30px; height: 30px; background: 25% 50% url(lighthouse-480x320.jpg?sprite);"></div>
<div class="onscreen" style="width: 30px; height: 30px; background: 50% 50% url(lighthouse-480x320.jpg?sprite);"></div>
Expand All @@ -196,6 +227,7 @@ <h2>Byte efficiency tester page</h2>
<!-- PASS(optimized): image is WebP -->
<!-- PASS(responsive): image is used at full size -->
<!-- PASS(offscreen): image is lazily loaded after TTI -->
<!-- PASS(unsized-images): css position is absolute -->
<img style="position: absolute; top: -5000px;" src="lighthouse-480x320.webp?lazilyLoaded=true">
</template>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,14 @@ const config = {
// image-size-responsive is not a byte-efficiency audit but a counterbalance to the byte-efficiency audits
// that makes sense to test together.
'image-size-responsive',
// unsized-images is not a byte-efficiency audit but can easily leverage the variety of images present in
// byte-efficiency tests & thus makes sense to test together.
'unsized-images',
],
throttlingMethod: 'devtools',
},
audits: [
'unsized-images',
{path: 'byte-efficiency/unused-javascript', options: {
// Lower the threshold so we don't need huge resources to make a test.
unusedThreshold: 2000,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ const expectations = [
details: {
overallSavingsBytes: '>60000',
items: {
length: 5,
length: 6,
},
},
},
Expand All @@ -220,23 +220,34 @@ const expectations = [
// Check that images aren't TOO BIG.
'uses-responsive-images': {
details: {
overallSavingsBytes: '108000 +/- 5000',
overallSavingsBytes: '113000 +/- 5000',
items: [
{wastedPercent: '56 +/- 5', url: /lighthouse-1024x680.jpg/},
{wastedPercent: '78 +/- 5', url: /lighthouse-2048x1356.webp\?size0/},
{wastedPercent: '56 +/- 5', url: /lighthouse-480x320.webp/},
{wastedPercent: '20 +/- 5', url: /lighthouse-480x320.jpg/},
{wastedPercent: '20 +/- 5', url: /lighthouse-480x320\.jpg\?attributesized/},
],
},
},
// Checks that images aren't TOO SMALL.
'image-size-responsive': {
details: {
items: [
// One of these two is the ?duplicate variant but sort order isn't guaranteed
// One of these is the ?duplicate variant and another is the
// ?cssauto variant but sort order isn't guaranteed
// since the pixel diff is equivalent for identical images.
{url: /lighthouse-320x212-poor.jpg/},
{url: /lighthouse-320x212-poor.jpg/},
{url: /lighthouse-320x212-poor.jpg/},
],
},
},
'unsized-images': {
details: {
items: [
{url: /lighthouse-320x212-poor\.jpg/},
{url: /lighthouse-320x212-poor\.jpg\?cssauto/},
],
},
},
Expand Down
27 changes: 15 additions & 12 deletions lighthouse-core/audits/unsized-images.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ const UIStrings = {
/** Title of a Lighthouse audit that provides detail on whether all images have explicit width and height. This descriptive title is shown to users when one or more images does not have explicit width and height */
failureTitle: 'Image elements do not have explicit `width` and `height`',
/** Description of a Lighthouse audit that tells the user why they should include explicit width and height 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 explicit width and height on image elements to reduce layout shifts and improve CLS. [Learn more](https://web.dev/optimize-cls/#images-without-dimensions)',
description: 'Set an explicit width and height on image elements to reduce layout shifts and improve CLS. [Learn more](https://web.dev/optimize-cls/#images-without-dimensions)',
};

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

class SizedImages extends Audit {
class UnsizedImages extends Audit {
/**
* @return {LH.Audit.Meta}
*/
Expand Down Expand Up @@ -67,31 +67,34 @@ class SizedImages extends Audit {
* @param {LH.Artifacts.ImageElement} image
* @return {boolean}
*/
static isUnsizedImage(image) {
static isSizedImage(image) {
const attrWidth = image.attributeWidth;
const attrHeight = image.attributeHeight;
const cssWidth = image.cssWidth;
const cssHeight = image.cssHeight;
const widthIsValidAttribute = SizedImages.isValidAttr(attrWidth);
const widthIsValidCss = SizedImages.isValidCss(cssWidth);
const heightIsValidAttribute = SizedImages.isValidAttr(attrHeight);
const heightIsValidCss = SizedImages.isValidCss(cssHeight);
const widthIsValidAttribute = UnsizedImages.isValidAttr(attrWidth);
const widthIsValidCss = UnsizedImages.isValidCss(cssWidth);
const heightIsValidAttribute = UnsizedImages.isValidAttr(attrHeight);
const heightIsValidCss = UnsizedImages.isValidCss(cssHeight);
const validWidth = widthIsValidAttribute || widthIsValidCss;
const validHeight = heightIsValidAttribute || heightIsValidCss;
return !validWidth || !validHeight;
return validWidth && validHeight;
}

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

for (const image of images) {
if (!SizedImages.isUnsizedImage(image)) continue;
const isFixedImage =
image.cssComputedPosition === 'fixed' || image.cssComputedPosition === 'absolute';

if (isFixedImage || UnsizedImages.isSizedImage(image)) continue;
const url = URL.elideDataURI(image.src);
unsizedImages.push({
url,
Expand Down Expand Up @@ -120,5 +123,5 @@ class SizedImages extends Audit {
}
}

module.exports = SizedImages;
module.exports = UnsizedImages;
module.exports.UIStrings = UIStrings;
Loading