Skip to content

Commit

Permalink
core(image-elements): drop spritesheet logic (#8940)
Browse files Browse the repository at this point in the history
  • Loading branch information
patrickhulce authored and paulirish committed May 28, 2019
1 parent 3abe066 commit 48cecfa
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 56 deletions.
12 changes: 6 additions & 6 deletions lighthouse-cli/test/fixtures/byte-efficiency/tester.html
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ <h2>Byte efficiency tester page</h2>

<!-- PASS(optimized): image is JPEG optimized -->
<!-- PASS(webp): image is WebP optimized -->
<!-- PASSWARN(responsive): image is 25% used at DPR 2 (but small savings) -->
<!-- FAIL(responsive): image is 25% used at DPR 2 (but small savings) -->
<!-- FAIL(offscreen): image is offscreen -->
<img style="margin-top: 1000px; width: 120px; height: 80px;" src="lighthouse-480x320.webp">

Expand Down Expand Up @@ -130,15 +130,15 @@ <h2>Byte efficiency tester page</h2>
<!-- PASS(offscreen): image is onscreen -->
<img class="onscreen" src="lighthouse-320x212-poor.jpg?duplicate">

<!-- PASS(optimized): image is WebP -->
<!-- PASS(webp): image is WebP optimized -->
<!-- FAIL(responsive): image is used at 1/16 size -->
<!-- PASS(optimized): image is JPEG optimized -->
<!-- FAIL(webp): image is not WebP optimized -->
<!-- PASS(responsive): image is in CSS -->
<!-- PASS(offscreen): image is onscreen -->
<div class="onscreen" style="width: 120px; height: 80px; background: 50% 50% url(lighthouse-480x320.webp?css);"></div>
<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 used numerous times in sprite-fashion -->
<!-- PASS(responsive): image is in CSS -->
<!-- PASS(offscreen): image is onscreen -->
<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>
Expand Down
16 changes: 8 additions & 8 deletions lighthouse-cli/test/smokehouse/byte-efficiency/expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ module.exports = [
details: {
overallSavingsBytes: '>60000',
items: {
length: 4,
length: 5,
},
},
},
Expand All @@ -106,14 +106,14 @@ module.exports = [
},
},
'uses-responsive-images': {
displayValue: 'Potential savings of 75\xa0KB',
displayValue: 'Potential savings of 69\xa0KB',
details: {
overallSavingsBytes: '>75000',
items: [
{wastedPercent: '<60'},
{wastedPercent: '<60'},
{wastedPercent: '<60'},
],
overallSavingsBytes: '>65000',
items: {
0: {wastedPercent: '<60'},
1: {wastedPercent: '<60'},
length: 2,
},
},
},
},
Expand Down
17 changes: 10 additions & 7 deletions lighthouse-core/audits/byte-efficiency/uses-responsive-images.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,27 +94,30 @@ class UsesResponsiveImages extends ByteEfficiencyAudit {
const warnings = [];
/** @type {Map<string, LH.Audit.ByteEfficiencyItem>} */
const resultsMap = new Map();
images.forEach(image => {
// TODO: give SVG a free pass until a detail per pixel metric is available
if (!image.resourceSize || image.mimeType === 'image/svg+xml') {
return;
for (const image of images) {
// Ignore images without resource size information.
// Give SVG a free pass because creating a "responsive" SVG is of questionable value.
// Ignore CSS images because it's difficult to determine what is a spritesheet,
// and the reward-to-effort ratio for responsive CSS images is quite low https://css-tricks.com/responsive-images-css/.
if (!image.resourceSize || image.mimeType === 'image/svg+xml' || image.isCss) {
continue;
}

const processed = UsesResponsiveImages.computeWaste(image, DPR);
if (!processed) return;
if (!processed) continue;

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

// Don't warn about an image that was later used appropriately
const existing = resultsMap.get(processed.url);
if (!existing || existing.wastedBytes > processed.wastedBytes) {
resultsMap.set(processed.url, processed);
}
});
}

const items = Array.from(resultsMap.values())
.filter(item => item.wastedBytes > IGNORE_THRESHOLD_IN_BYTES);
Expand Down
85 changes: 50 additions & 35 deletions lighthouse-core/gather/gatherers/image-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,30 +15,31 @@ const Driver = require('../driver.js'); // eslint-disable-line no-unused-vars

/* global window, getElementsInDocument, Image */

/** @return {Array<LH.Artifacts.ImageElement>} */

/** @param {Element} element */
/* istanbul ignore next */
function collectImageElementInfo() {
/** @param {Element} element */
function getClientRect(element) {
const clientRect = element.getBoundingClientRect();
return {
// Just grab the DOMRect properties we want, excluding x/y/width/height
top: clientRect.top,
bottom: clientRect.bottom,
left: clientRect.left,
right: clientRect.right,
};
}
function getClientRect(element) {
const clientRect = element.getBoundingClientRect();
return {
// Just grab the DOMRect properties we want, excluding x/y/width/height
top: clientRect.top,
bottom: clientRect.bottom,
left: clientRect.left,
right: clientRect.right,
};
}

/** @type {Array<Element>} */
// @ts-ignore - added by getElementsInDocumentFnString
const allElements = getElementsInDocument();
/**
* @param {Array<Element>} allElements
* @return {Array<LH.Artifacts.ImageElement>}
*/
/* istanbul ignore next */
function getHTMLImages(allElements) {
const allImageElements = /** @type {Array<HTMLImageElement>} */ (allElements.filter(element => {
return element.localName === 'img';
}));

/** @type {Array<LH.Artifacts.ImageElement>} */
const htmlImages = allImageElements.map(element => {
return allImageElements.map(element => {
const computedStyle = window.getComputedStyle(element);
return {
// currentSrc used over src to get the url as determined by the browser
Expand All @@ -57,30 +58,30 @@ function collectImageElementInfo() {
),
};
});
}

/**
* @param {Array<Element>} allElements
* @return {Array<LH.Artifacts.ImageElement>}
*/
/* istanbul ignore next */
function getCSSImages(allElements) {
// Chrome normalizes background image style from getComputedStyle to be an absolute URL in quotes.
// Only match basic background-image: url("http://host/image.jpeg") declarations
const CSS_URL_REGEX = /^url\("([^"]+)"\)$/;
// Only find images that aren't specifically scaled
const CSS_SIZE_REGEX = /(auto|contain|cover)/;

const cssImages = allElements.reduce((images, element) => {
/** @type {Array<LH.Artifacts.ImageElement>} */
const images = [];

for (const element of allElements) {
const style = window.getComputedStyle(element);
if (!style.backgroundImage || !CSS_URL_REGEX.test(style.backgroundImage) ||
!style.backgroundSize || !CSS_SIZE_REGEX.test(style.backgroundSize)) {
return images;
}
// If the element didn't have a CSS background image, we're not interested.
if (!style.backgroundImage || !CSS_URL_REGEX.test(style.backgroundImage)) continue;

const imageMatch = style.backgroundImage.match(CSS_URL_REGEX);
// @ts-ignore test() above ensures that there is a match.
const url = imageMatch[1];

// Heuristic to filter out sprite sheets
const differentImages = images.filter(image => image.src !== url);
if (images.length - differentImages.length > 2) {
return differentImages;
}

images.push({
src: url,
displayedWidth: element.clientWidth,
Expand All @@ -94,11 +95,18 @@ function collectImageElementInfo() {
usesObjectFit: false,
resourceSize: 0, // this will get overwritten below
});
}

return images;
}, /** @type {Array<LH.Artifacts.ImageElement>} */ ([]));
return images;
}

return htmlImages.concat(cssImages);
/** @return {Array<LH.Artifacts.ImageElement>} */
/* istanbul ignore next */
function collectImageElementInfo() {
/** @type {Array<Element>} */
// @ts-ignore - added by getElementsInDocumentFnString
const allElements = getElementsInDocument();
return getHTMLImages(allElements).concat(getCSSImages(allElements));
}

/**
Expand Down Expand Up @@ -160,16 +168,23 @@ class ImageElements extends Gatherer {

const expression = `(function() {
${pageFunctions.getElementsInDocumentString}; // define function on page
return (${collectImageElementInfo.toString()})();
${getClientRect.toString()};
${getHTMLImages.toString()};
${getCSSImages.toString()};
${collectImageElementInfo.toString()};
return collectImageElementInfo();
})()`;

/** @type {Array<LH.Artifacts.ImageElement>} */
const elements = await driver.evaluateAsync(expression);

/** @type {Array<LH.Artifacts.ImageElement>} */
const imageUsage = [];
const top50Images = Object.values(indexedNetworkRecords)
.sort((a, b) => b.resourceSize - a.resourceSize)
.slice(0, 50);

for (let element of elements) {
// Pull some of our information directly off the network record.
const networkRecord = indexedNetworkRecords[element.src] || {};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,37 @@ describe('Page uses responsive images', () => {
assert.equal(auditResult.items.length, 0);
});

it('ignores CSS', () => {
const urlA = 'https://google.com/logo.png';
const naturalSizeA = generateSize(450, 450, 'natural');
const recordA = generateRecord(100, 300);

const auditResult = UsesResponsiveImagesAudit.audit_({
ViewportDimensions: {devicePixelRatio: 1},
ImageElements: [
{...generateImage(generateSize(10, 10), naturalSizeA, recordA, urlA), isCss: true},
],
});

assert.equal(auditResult.items.length, 0);
});

it('handles failure', () => {
const urlA = 'https://google.com/logo.png';
const naturalSizeA = generateSize(NaN, 450, 'natural');
const recordA = generateRecord(100, 300);

const auditResult = UsesResponsiveImagesAudit.audit_({
ViewportDimensions: {devicePixelRatio: 1},
ImageElements: [
generateImage(generateSize(10, 10), naturalSizeA, recordA, urlA),
],
});

assert.equal(auditResult.items.length, 0);
assert.equal(auditResult.warnings.length, 1);
});

it('de-dupes images', () => {
const urlA = 'https://google.com/logo.png';
const naturalSizeA = generateSize(450, 450, 'natural');
Expand Down

0 comments on commit 48cecfa

Please sign in to comment.