Skip to content

Commit

Permalink
core: remove fonts gatherer, move font-display to use CSSUsage artifa…
Browse files Browse the repository at this point in the history
…ct (#6166)
  • Loading branch information
patrickhulce authored and paulirish committed Oct 12, 2018
1 parent cb998e7 commit 0f680b3
Show file tree
Hide file tree
Showing 6 changed files with 224 additions and 464 deletions.
3 changes: 0 additions & 3 deletions lighthouse-cli/test/cli/__snapshots__/index-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -1104,9 +1104,6 @@ Object {
Object {
"path": "seo/robots-txt",
},
Object {
"path": "fonts",
},
],
"networkQuietThresholdMs": 1000,
"passName": "defaultPass",
Expand Down
142 changes: 94 additions & 48 deletions lighthouse-core/audits/font-display.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,21 @@
'use strict';

const Audit = require('./audit');
const NetworkRequest = require('../lib/network-request');
const allowedFontFaceDisplays = ['block', 'fallback', 'optional', 'swap'];
const URL = require('../lib/url-shim').URL;
const PASSING_FONT_DISPLAY_REGEX = /block|fallback|optional|swap/;
const CSS_URL_REGEX = /url\((.*?)\)/;
const CSS_URL_GLOBAL_REGEX = new RegExp(CSS_URL_REGEX, 'g');
const i18n = require('../lib/i18n/i18n.js');
const Sentry = require('../lib/sentry.js');

const UIStrings = {
/** Title of a diagnostic audit that provides detail on if all the text on a webpage was visible while the page was loading its webfonts. This descriptive title is shown to users when the amount is acceptable and no user action is required. */
title: 'All text remains visible during webfont loads',
/** Title of a diagnostic audit that provides detail on the load of the page's webfonts. Often the text is invisible for seconds before the webfont resource is loaded. This imperative title is shown to users when there is a significant amount of execution time that could be reduced. */
failureTitle: 'Ensure text remains visible during webfont load',
/** Description of a Lighthouse audit that tells the user *why* they should use the font-display CSS feature. 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: 'Leverage the font-display CSS feature to ensure text is user-visible while ' +
description:
'Leverage the font-display CSS feature to ensure text is user-visible while ' +
'webfonts are loading. ' +
'[Learn more](https://developers.google.com/web/updates/2016/02/font-display).',
};
Expand All @@ -33,59 +37,101 @@ class FontDisplay extends Audit {
title: str_(UIStrings.title),
failureTitle: str_(UIStrings.failureTitle),
description: str_(UIStrings.description),
requiredArtifacts: ['devtoolsLogs', 'Fonts'],
requiredArtifacts: ['devtoolsLogs', 'CSSUsage', 'URL'],
};
}

/**
* @param {LH.Artifacts} artifacts
*/
static findPassingFontDisplayDeclarations(artifacts) {
/** @type {Set<string>} */
const passingURLs = new Set();

// Go through all the stylesheets to find all @font-face declarations
for (const stylesheet of artifacts.CSSUsage.stylesheets) {
// Eliminate newlines so we can more easily scan through with a regex
const newlinesStripped = stylesheet.content.replace(/\n/g, ' ');
// Find the @font-faces
const fontFaceDeclarations = newlinesStripped.match(/@font-face\s*{(.*?)}/g) || [];
// Go through all the @font-face declarations to find a declared `font-display: ` property
for (const declaration of fontFaceDeclarations) {
const rawFontDisplay = declaration.match(/font-display:(.*?);/);
// If they didn't have a font-display property, it's the default, and it's failing; bail
if (!rawFontDisplay) continue;
// If they don't have one of the passing font-display values, it's failing; bail
const hasPassingFontDisplay = PASSING_FONT_DISPLAY_REGEX.test(rawFontDisplay[0]);
if (!hasPassingFontDisplay) continue;

// If it's passing, we'll try to find the URL it's referencing.
const rawFontURLs = declaration.match(CSS_URL_GLOBAL_REGEX);
// If no URLs, we can't really do anything; bail
if (!rawFontURLs) continue;

const relativeURLs = rawFontURLs
// @ts-ignore - guaranteed to match from previous regex, pull URL group out
.map(s => s.match(CSS_URL_REGEX)[1].trim())
.map(s => {
// remove any quotes surrounding the URL
if (/^('|").*\1$/.test(s)) {
return s.substr(1, s.length - 2);
}

return s;
});

// Convert the relative CSS URL to an absolute URL and add it to the passing set
for (const relativeURL of relativeURLs) {
try {
const absoluteURL = new URL(relativeURL, artifacts.URL.finalUrl);
passingURLs.add(absoluteURL.href);
} catch (err) {
Sentry.captureException(err, {tags: {audit: this.meta.id}});
}
}
}
}

return passingURLs;
}

/**
* @param {LH.Artifacts} artifacts
* @return {Promise<LH.Audit.Product>}
*/
static audit(artifacts) {
static async audit(artifacts) {
const devtoolsLogs = artifacts.devtoolsLogs[this.DEFAULT_PASS];
const fontFaces = artifacts.Fonts;

// Filter font-faces that do not have a display tag with optional or swap
const fontsWithoutProperDisplay = fontFaces.filter(fontFace =>
!fontFace.display || !allowedFontFaceDisplays.includes(fontFace.display)
);

return artifacts.requestNetworkRecords(devtoolsLogs).then((networkRecords) => {
const results = networkRecords.filter(record => {
const isFont = record.resourceType === NetworkRequest.TYPES.Font;

return isFont;
})
.filter(fontRecord => {
// find the fontRecord of a font
return !!fontsWithoutProperDisplay.find(fontFace => {
return !!fontFace.src && !!fontFace.src.find(src => fontRecord.url === src);
});
})
// calculate wasted time
.map(record => {
// In reality the end time should be calculated with paint time included
// all browsers wait 3000ms to block text so we make sure 3000 is our max wasted time
const wastedMs = Math.min((record.endTime - record.startTime) * 1000, 3000);

return {
url: record.url,
wastedMs,
};
});

const headings = [
{key: 'url', itemType: 'url', text: str_(i18n.UIStrings.columnURL)},
{key: 'wastedMs', itemType: 'ms', text: str_(i18n.UIStrings.columnWastedMs)},
];
const details = Audit.makeTableDetails(headings, results);

return {
score: Number(results.length === 0),
rawValue: results.length === 0,
details,
};
});
const networkRecords = await artifacts.requestNetworkRecords(devtoolsLogs);
const passingFontURLs = FontDisplay.findPassingFontDisplayDeclarations(artifacts);

const results = networkRecords
// Find all fonts...
.filter(record => record.resourceType === 'Font')
// ...that don't have a passing font-display value
.filter(record => !passingFontURLs.has(record.url))
.map(record => {
// In reality the end time should be calculated with paint time included
// all browsers wait 3000ms to block text so we make sure 3000 is our max wasted time
const wastedMs = Math.min((record.endTime - record.startTime) * 1000, 3000);

return {
url: record.url,
wastedMs,
};
});

const headings = [
{key: 'url', itemType: 'url', text: str_(i18n.UIStrings.columnURL)},
{key: 'wastedMs', itemType: 'ms', text: str_(i18n.UIStrings.columnWastedMs)},
];

const details = Audit.makeTableDetails(headings, results);

return {
score: Number(results.length === 0),
rawValue: results.length === 0,
details,
};
}
}

Expand Down
1 change: 0 additions & 1 deletion lighthouse-core/config/default-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ const defaultConfig = {
'seo/embedded-content',
'seo/canonical',
'seo/robots-txt',
'fonts',
],
},
{
Expand Down
Loading

0 comments on commit 0f680b3

Please sign in to comment.