new_audit(font-size-audit): legible font sizes audit #3533
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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); | ||
|
Collaborator
There was a problem hiding this comment. perhaps something got thrown off between the iterations but it seems like analyzed text nodes aren't always getting de-duped anymore?
rule is in CSS
Collaborator
Author
There was a problem hiding this comment. Turned out not to be a regression but rather bug that was there from the beginning. Note that font-size is defined twice in this rule and I've been only examining first occurrence in
This was causing effective rule not to be found and every node listed as a separate entry in the details table. Fixed 🔧 |
||
| const failingRule = failingRules.get(artifactId); | ||
|
|
||
| if (!failingRule) { | ||
| failingRules.set(artifactId, { | ||
| node, | ||
|
Collaborator
There was a problem hiding this comment. will all duplicate rules correspond to the same node? or is this the optional node that's only set for inline style?
Collaborator
Author
There was a problem hiding this comment. It's always set, but it only matters for inline (
Collaborator
There was a problem hiding this comment. ah ok and in the inline and attribute cases it's obviously the same node since that's the source, gotcha |
||
| 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', | ||
|
Collaborator
There was a problem hiding this comment. this seems like an unfortunate situation to show to a user, is the default on mobile illegible text?
Collaborator
Author
There was a problem hiding this comment. Default UA font size value is legible ( |
||
| }; | ||
| } | ||
|
|
||
| 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; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps we can assert how many elements fail too?
details: { items: { length: x }}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we fail because viewport is not configured (see debugString below), so we don't even go into evaluating text size and
detailswill be empty. Viewport is empty on this page because of theviewportaudit that we also want to fail here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I figured that out last time, but who knows what I was thinking 18 days ago :)