Skip to content

Commit

Permalink
new_audit(font-size-audit): legible font sizes audit (#3533)
Browse files Browse the repository at this point in the history
* Font Size audit - WIP

* Calculate percentage of ilegible text on page. Show failing elements in a table.

* Getting font-size information togheter with associated CSS rules - WIP

* Matched rules - WIP

* Extracting style info - WIP

* Expose effective rule

* Show what percentage of text each rule affected. Show table with failing selectors even if test passed. Clean up.

* Work on inline styles, clean up jsdoc

* Handle inline styels, attribute styles and user agent styles.

* Expose stylesheet URL, line number and column.

* Update output table according to the latst decisions.

* Changes to the result table. Clean up.

* Gatherer test.

* Smoke tests. Fixing edgecase when size is inherited from attribute styles of a parent node. Fixing linting errors, improvic jsdoc.

* Add unit tests.

* Remove info about color prop.

* Remove debugMessage from passing test expectations as it doesn't exist.

* Replace URL with parseURL

* Make linter happy.

* Check number of failing items in the smoke test.

* getOrigin -> findStyleRuleSource
s/Lenght/Length

* Collect stylesheet metadata in font-size gatherer. Get rid of separate run.

* Font Size audit - WIP

* Calculate percentage of ilegible text on page. Show failing elements in a table.

* Getting font-size information togheter with associated CSS rules - WIP

* Matched rules - WIP

* Extracting style info - WIP

* Expose effective rule

* Show what percentage of text each rule affected. Show table with failing selectors even if test passed. Clean up.

* Work on inline styles, clean up jsdoc

* Handle inline styels, attribute styles and user agent styles.

* Expose stylesheet URL, line number and column.

* Update output table according to the latst decisions.

* Changes to the result table. Clean up.

* Gatherer test.

* Smoke tests. Fixing edgecase when size is inherited from attribute styles of a parent node. Fixing linting errors, improvic jsdoc.

* Add unit tests.

* Remove info about color prop.

* Remove debugMessage from passing test expectations as it doesn't exist.

* Replace URL with parseURL

* Make linter happy.

* Check number of failing items in the smoke test.

* getOrigin -> findStyleRuleSource
s/Lenght/Length

* Collect stylesheet metadata in font-size gatherer. Get rid of separate run.

* Address some of the review comments

* Add comment, fix jsdoc

* Limit number of CDP calls to make sure that gatherer doesn't take too much time

* Reuse method from driver, introduce block list for text nodes

* Adjust tests and make linter happy.

* A word.

* Sort nodes by text length before limiting number of visited.

* Typo

* Typo 🔎

* Move analyzedFailingTextLength calculation to the gatherer, rename failingTextLength to analyzedFailingTextLength. Adjust tests.

* Add fileoverview to font-size gatherer

* Remove gatherers from seo config as they are already in default config.

* Prevent gatherer from failing if one of the nodes can't be found

* Take into account that each property can be defined multiple times inside one CSS block.
Count visited text length after getting font sizes.

* Update copy, remove audits from seo config, show 'dynamic' as source for dynamically injected styles

* Fix test

* Fix tests

* Add simple selector to nodes in the details table.

* Add'l 🚲🏠

* Fix tests.

* Additional info in the helpText
  • Loading branch information
kdzwinel authored and patrickhulce committed Dec 14, 2017
1 parent 020a321 commit 9ef858a
Show file tree
Hide file tree
Showing 12 changed files with 848 additions and 18 deletions.
1 change: 1 addition & 0 deletions lighthouse-cli/test/fixtures/seo/seo-failure-cases.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
<head>
<!-- no <title>SEO audit failures page</title> -->
<meta charset="utf-8">
<!-- FAIL(font-size): missing viewport -->
<meta name="viewport" content="invalid-content=should_have_looked_it_up">
<!-- no <meta name="description" content=""> -->
<meta name="robots" content="nofollow, NOINDEX, all">
Expand Down
16 changes: 16 additions & 0 deletions lighthouse-cli/test/fixtures/seo/seo-tester.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@
<LINK REL="ALTERNATE" HREFLANG="ru-RU" HREF="https://ru.example.com" />
<LINK REL="alternate" HREFLANG="zh-Hans-TW" HREF="https://zh.example.com" />
<link rel="alternate" href="http://example.com/" hreflang="x-default" />

<style>
.small {
font-size: 15px;
}
</style>
</head>
<body>
<h1>SEO</h1>
Expand All @@ -26,5 +32,15 @@ <h2>Anchor text</h2>
<a href='#non-descriptive-local'>click this</a>
<a href='https://example.com/non-descriptive-no-follow.html' rel="nofollow">click this</a>
<a href='javascript:void(0);'>click this</a>

<h2>Small text</h2>
<!-- PASS(font-size): amount of illegible text is below the 75% threshold -->
<p class='small'> 1 </p>
<small>2</small>
<font size="1">3<b>4</b></font>
<p style='font-size:10px'>5 </p>
<script class='small'>
// text from SCRIPT tags should be ignored by the font-size audit
</script>
</body>
</html>
12 changes: 12 additions & 0 deletions lighthouse-cli/test/smokehouse/seo/expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ module.exports = [
'http-status-code': {
score: true,
},
'font-size': {
rawValue: true,
details: {
items: {
length: 6,
},
},
},
'link-text': {
score: true,
},
Expand Down Expand Up @@ -73,6 +81,10 @@ module.exports = [
score: false,
displayValue: '403',
},
'font-size': {
rawValue: false,
debugString: 'Text is illegible because of a missing viewport config',
},
'link-text': {
score: false,
displayValue: '3 links found',
Expand Down
286 changes: 286 additions & 0 deletions lighthouse-core/audits/seo/font-size.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,286 @@
/**
* @license Copyright 2017 Google Inc. 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 parseURL = require('url').parse;
const Audit = require('../audit');
const ViewportAudit = require('../viewport');
const CSSStyleDeclaration = require('../../lib/web-inspector').CSSStyleDeclaration;
const MINIMAL_PERCENTAGE_OF_LEGIBLE_TEXT = 75;

/**
* @param {Array<{cssRule: WebInspector.CSSStyleDeclaration, fontSize: number, textLength: number, node: Node}>} fontSizeArtifact
* @returns {Array<{cssRule: WebInspector.CSSStyleDeclaration, fontSize: number, textLength: number, node: Node}>}
*/
function getUniqueFailingRules(fontSizeArtifact) {
const failingRules = new Map();

fontSizeArtifact.forEach(({cssRule, fontSize, textLength, node}) => {
const artifactId = getFontArtifactId(cssRule, node);
const failingRule = failingRules.get(artifactId);

if (!failingRule) {
failingRules.set(artifactId, {
node,
cssRule,
fontSize,
textLength,
});
} else {
failingRule.textLength += textLength;
}
});

return failingRules.valuesArray();
}

/**
* @param {Array<string>} attributes
* @returns {Map<string, string>}
*/
function getAttributeMap(attributes) {
const map = new Map();

for (let i=0; i<attributes.length; i+=2) {
const name = attributes[i].toLowerCase();
const value = attributes[i + 1].trim();

if (value) {
map.set(name, value);
}
}

return map;
}

/**
* TODO: return unique selector, like axe-core does, instead of just id/class/name of a single node
* @param {Node} node
* @returns {string}
*/
function getSelector(node) {
const attributeMap = getAttributeMap(node.attributes);

if (attributeMap.has('id')) {
return '#' + attributeMap.get('id');
} else if (attributeMap.has('class')) {
return '.' + attributeMap.get('class').split(/\s+/).join('.');
}

return node.localName.toLowerCase();
}

/**
* @param {Node} node
* @return {{type:string, selector: string, snippet:string}}
*/
function nodeToTableNode(node) {
const attributesString = node.attributes.map((value, idx) =>
(idx % 2 === 0) ? ` ${value}` : `="${value}"`
).join('');

return {
type: 'node',
selector: node.parentNode ? getSelector(node.parentNode) : '',
snippet: `<${node.localName}${attributesString}>`,
};
}

/**
* @param {string} baseURL
* @param {WebInspector.CSSStyleDeclaration} styleDeclaration
* @param {Node} node
* @returns {{source:!string, selector:string|object}}
*/
function findStyleRuleSource(baseURL, styleDeclaration, node) {
if (
!styleDeclaration ||
styleDeclaration.type === CSSStyleDeclaration.Type.Attributes ||
styleDeclaration.type === CSSStyleDeclaration.Type.Inline
) {
return {
source: baseURL,
selector: nodeToTableNode(node),
};
}

if (styleDeclaration.parentRule &&
styleDeclaration.parentRule.origin === global.CSSAgent.StyleSheetOrigin.USER_AGENT) {
return {
selector: styleDeclaration.parentRule.selectors.map(item => item.text).join(', '),
source: 'User Agent Stylesheet',
};
}

if (styleDeclaration.type === CSSStyleDeclaration.Type.Regular && styleDeclaration.parentRule) {
const rule = styleDeclaration.parentRule;
const stylesheet = styleDeclaration.stylesheet;

if (stylesheet) {
let source;
const selector = rule.selectors.map(item => item.text).join(', ');

if (stylesheet.sourceURL) {
const url = parseURL(stylesheet.sourceURL, baseURL);
const range = styleDeclaration.range;
source = `${url.href}`;

if (range) {
// `stylesheet` can be either an external file (stylesheet.startLine will always be 0),
// or a <style> block (stylesheet.startLine will vary)
const absoluteStartLine = range.startLine + stylesheet.startLine + 1;
const absoluteStartColumn = range.startColumn + stylesheet.startColumn + 1;

source += `:${absoluteStartLine}:${absoluteStartColumn}`;
}
} else {
// dynamically injected to page
source = 'dynamic';
}

return {
selector,
source,
};
}
}

return {
source: 'Unknown',
};
}

/**
* @param {WebInspector.CSSStyleDeclaration} styleDeclaration
* @param {Node} node
* @return string
*/
function getFontArtifactId(styleDeclaration, node) {
if (styleDeclaration && styleDeclaration.type === CSSStyleDeclaration.Type.Regular) {
const startLine = styleDeclaration.range ? styleDeclaration.range.startLine : 0;
const startColumn = styleDeclaration.range ? styleDeclaration.range.startColumn : 0;
return `${styleDeclaration.styleSheetId}@${startLine}:${startColumn}`;
} else {
return `node_${node.nodeId}`;
}
}

class FontSize extends Audit {
/**
* @return {!AuditMeta}
*/
static get meta() {
return {
name: 'font-size',
description: 'Document uses legible font sizes.',
failureDescription: 'Document doesn\'t use legible font sizes.',
helpText: 'Font sizes less than 16px are too small to be legible and require mobile ' +
'visitors to “pinch to zoom” in order to read. Strive to have >75% of page text ≥16px. ' +
'[Learn more](https://developers.google.com/speed/docs/insights/UseLegibleFontSizes).',
requiredArtifacts: ['FontSize', 'URL', 'Viewport'],
};
}

/**
* @param {!Artifacts} artifacts
* @return {!AuditResult}
*/
static audit(artifacts) {
const hasViewportSet = ViewportAudit.audit(artifacts).rawValue;
if (!hasViewportSet) {
return {
rawValue: false,
debugString: 'Text is illegible because of a missing viewport config',
};
}

const {
analyzedFailingNodesData,
analyzedFailingTextLength,
failingTextLength,
visitedTextLength,
totalTextLength,
} = artifacts.FontSize;

if (totalTextLength === 0) {
return {
rawValue: true,
};
}

const failingRules = getUniqueFailingRules(analyzedFailingNodesData);
const percentageOfPassingText =
(visitedTextLength - failingTextLength) / visitedTextLength * 100;
const pageUrl = artifacts.URL.finalUrl;

const headings = [
{key: 'source', itemType: 'url', text: 'Source'},
{key: 'selector', itemType: 'code', text: 'Selector'},
{key: 'coverage', itemType: 'text', text: '% of Page Text'},
{key: 'fontSize', itemType: 'text', text: 'Font Size'},
];

const tableData = failingRules.sort((a, b) => b.textLength - a.textLength)
.map(({cssRule, textLength, fontSize, node}) => {
const percentageOfAffectedText = textLength / visitedTextLength * 100;
const origin = findStyleRuleSource(pageUrl, cssRule, node);

return {
source: origin.source,
selector: origin.selector,
coverage: `${percentageOfAffectedText.toFixed(2)}%`,
fontSize: `${fontSize}px`,
};
});

// all failing nodes that were not fully analyzed will be displayed in a single row
if (analyzedFailingTextLength < failingTextLength) {
const percentageOfUnanalyzedFailingText =
(failingTextLength - analyzedFailingTextLength) / visitedTextLength * 100;

tableData.push({
source: 'Add\'l illegible text',
selector: null,
coverage: `${percentageOfUnanalyzedFailingText.toFixed(2)}%`,
fontSize: '< 16px',
});
}

if (percentageOfPassingText > 0) {
tableData.push({
source: 'Legible text',
selector: null,
coverage: `${percentageOfPassingText.toFixed(2)}%`,
fontSize: '≥ 16px',
});
}

const details = Audit.makeTableDetails(headings, tableData);
const passed = percentageOfPassingText >= MINIMAL_PERCENTAGE_OF_LEGIBLE_TEXT;
let debugString = null;

if (!passed) {
const percentageOfFailingText = parseFloat((100 - percentageOfPassingText).toFixed(2));
let disclaimer = '';

// if we were unable to visit all text nodes we should disclose that information
if (visitedTextLength < totalTextLength) {
const percentageOfVisitedText = visitedTextLength / totalTextLength * 100;
disclaimer = ` (based on ${percentageOfVisitedText.toFixed()}% sample)`;
}

debugString = `${percentageOfFailingText}% of text is too small${disclaimer}.`;
}

return {
rawValue: passed,
details,
debugString,
};
}
}

module.exports = FontSize;
2 changes: 2 additions & 0 deletions lighthouse-core/config/default.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ module.exports = {
'dobetterweb/tags-blocking-first-paint',
'dobetterweb/websql',
'seo/meta-description',
'seo/font-size',
'seo/crawlable-links',
'seo/meta-robots',
'seo/hreflang',
Expand Down Expand Up @@ -164,6 +165,7 @@ module.exports = {
'dobetterweb/uses-passive-event-listeners',
'seo/meta-description',
'seo/http-status-code',
'seo/font-size',
'seo/link-text',
'seo/is-crawlable',
'seo/hreflang',
Expand Down
16 changes: 3 additions & 13 deletions lighthouse-core/config/seo.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,9 @@ module.exports = {
extends: 'lighthouse:default',
passes: [{
passName: 'defaultPass',
gatherers: [
'seo/meta-description',
'seo/crawlable-links',
'seo/meta-robots',
'seo/hreflang',
],
gatherers: [],
}],
audits: [
'seo/meta-description',
'seo/http-status-code',
'seo/link-text',
'seo/is-crawlable',
'seo/hreflang',
],
audits: [],
groups: {
'seo-mobile': {
title: 'Mobile Friendly',
Expand All @@ -47,6 +36,7 @@ module.exports = {
{id: 'document-title', weight: 1, group: 'seo-content'},
{id: 'meta-description', weight: 1, group: 'seo-content'},
{id: 'http-status-code', weight: 1, group: 'seo-crawl'},
{id: 'font-size', weight: 1, group: 'seo-mobile'},
{id: 'link-text', weight: 1, group: 'seo-content'},
{id: 'is-crawlable', weight: 1, group: 'seo-crawl'},
{id: 'hreflang', weight: 1, group: 'seo-content'},
Expand Down
Loading

0 comments on commit 9ef858a

Please sign in to comment.