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): drop spritesheet logic #8940

Merged
merged 4 commits into from
May 23, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
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 I mean just c'mon https://css-tricks.com/responsive-images-css/.
patrickhulce marked this conversation as resolved.
Show resolved Hide resolved
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