Skip to content

Commit

Permalink
core(font-size): remove deprecated DOM.getFlattenedDocument (#11248)
Browse files Browse the repository at this point in the history
  • Loading branch information
@connorjclark
connorjclark committed Aug 11, 2020
1 parent 8fd7551 commit 7a462a2
Show file tree
Hide file tree
Showing 6 changed files with 178 additions and 160 deletions.
42 changes: 22 additions & 20 deletions lighthouse-core/audits/seo/font-size.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,15 @@ function getUniqueFailingRules(fontSizeArtifact) {
/** @type {Map<string, FailingNodeData>} */
const failingRules = new Map();

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

if (!failingRule) {
failingRules.set(artifactId, {
node,
nodeId,
parentNode,
cssRule,
fontSize,
textLength,
Expand Down Expand Up @@ -76,11 +78,11 @@ function getAttributeMap(attributes = []) {

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

if (attributeMap.has('id')) {
return '#' + attributeMap.get('id');
Expand All @@ -91,40 +93,40 @@ function getSelector(node) {
}
}

return node.localName.toLowerCase();
return parentNode.nodeName.toLowerCase();
}

/**
* @param {FailingNodeData['node']} node
* @param {FailingNodeData['parentNode']} parentNode
* @return {LH.Audit.Details.NodeValue}
*/
function nodeToTableNode(node) {
const attributes = node.attributes || [];
function nodeToTableNode(parentNode) {
const attributes = parentNode.attributes || [];
const attributesString = attributes.map((value, idx) =>
(idx % 2 === 0) ? ` ${value}` : `="${value}"`
).join('');

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

/**
* @param {string} baseURL
* @param {FailingNodeData['cssRule']} styleDeclaration
* @param {FailingNodeData['node']} node
* @param {FailingNodeData['parentNode']} parentNode
* @returns {{source: LH.Audit.Details.UrlValue | LH.Audit.Details.SourceLocationValue | LH.Audit.Details.CodeValue, selector: string | LH.Audit.Details.NodeValue}}
*/
function findStyleRuleSource(baseURL, styleDeclaration, node) {
function findStyleRuleSource(baseURL, styleDeclaration, parentNode) {
if (!styleDeclaration ||
styleDeclaration.type === 'Attributes' ||
styleDeclaration.type === 'Inline'
) {
return {
source: {type: 'url', value: baseURL},
selector: nodeToTableNode(node),
selector: nodeToTableNode(parentNode),
};
}

Expand Down Expand Up @@ -198,16 +200,16 @@ function findStyleRuleSource(baseURL, styleDeclaration, node) {

/**
* @param {FailingNodeData['cssRule']} styleDeclaration
* @param {FailingNodeData['node']} node
* @param {number} textNodeId
* @return {string}
*/
function getFontArtifactId(styleDeclaration, node) {
function getFontArtifactId(styleDeclaration, textNodeId) {
if (styleDeclaration && styleDeclaration.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}`;
return `node_${textNodeId}`;
}
}

Expand Down Expand Up @@ -274,9 +276,9 @@ class FontSize extends Audit {
];

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

return {
source: origin.source,
Expand Down
13 changes: 0 additions & 13 deletions lighthouse-core/gather/driver.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1204,19 +1204,6 @@ class Driver {
return new LHElement(targetNode, this);
}

/**
* Returns the flattened list of all DOM nodes within the document.
* @param {boolean=} pierce Whether to pierce through shadow trees and iframes.
* True by default.
* @return {Promise<Array<LH.Crdp.DOM.Node>>} The found nodes, or [], resolved in a promise
*/
async getNodesInDocument(pierce = true) {
const flattenedDocument = await this.sendCommand('DOM.getFlattenedDocument',
{depth: -1, pierce});

return flattenedDocument.nodes ? flattenedDocument.nodes : [];
}

/**
* Resolves a backend node ID (from a trace event, protocol, etc) to the object ID for use with
* `Runtime.callFunctionOn`. `undefined` means the node could not be found.
Expand Down
136 changes: 63 additions & 73 deletions lighthouse-core/gather/gatherers/seo/font-size.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,38 +24,8 @@ const MAX_NODES_SOURCE_RULE_FETCHED = 50; // number of nodes to fetch the source

/** @typedef {import('../../driver.js')} Driver */
/** @typedef {LH.Artifacts.FontSize['analyzedFailingNodesData'][0]} NodeFontData */
/** @typedef {LH.Artifacts.FontSize.DomNodeMaybeWithParent} DomNodeMaybeWithParent*/
/** @typedef {Map<number, {fontSize: number, textLength: number}>} BackendIdsToFontData */

/**
* @param {LH.Artifacts.FontSize.DomNodeMaybeWithParent=} node
* @returns {node is LH.Artifacts.FontSize.DomNodeWithParent}
*/
function nodeInBody(node) {
if (!node) {
return false;
}
if (node.nodeName === 'BODY') {
return true;
}
return nodeInBody(node.parentNode);
}

/**
* Get list of all nodes from the document body.
*
* @param {Driver} driver
* @returns {Promise<LH.Artifacts.FontSize.DomNodeWithParent[]>}
*/
async function getAllNodesFromBody(driver) {
const nodes = /** @type {DomNodeMaybeWithParent[]} */ (await driver.getNodesInDocument());
/** @type {Map<number|undefined, LH.Artifacts.FontSize.DomNodeMaybeWithParent>} */
const nodeMap = new Map();
nodes.forEach(node => nodeMap.set(node.nodeId, node));
nodes.forEach(node => (node.parentNode = nodeMap.get(node.parentId)));
return nodes.filter(nodeInBody);
}

/**
* @param {LH.Crdp.CSS.CSSStyle} [style]
* @return {boolean}
Expand Down Expand Up @@ -187,12 +157,12 @@ function getTextLength(text) {

/**
* @param {Driver} driver
* @param {LH.Crdp.DOM.Node} node text node
* @param {number} nodeId text node
* @returns {Promise<NodeFontData['cssRule']|undefined>}
*/
async function fetchSourceRule(driver, node) {
async function fetchSourceRule(driver, nodeId) {
const matchedRules = await driver.sendCommand('CSS.getMatchedStylesForNode', {
nodeId: node.nodeId,
nodeId,
});
const sourceRule = getEffectiveFontRule(matchedRules);
if (!sourceRule) return undefined;
Expand All @@ -214,12 +184,22 @@ class FontSize extends Gatherer {
* @param {Array<NodeFontData>} failingNodes
*/
static async fetchFailingNodeSourceRules(driver, failingNodes) {
const analysisPromises = failingNodes
const nodesToAnalyze = failingNodes
.sort((a, b) => b.textLength - a.textLength)
.slice(0, MAX_NODES_SOURCE_RULE_FETCHED)
.map(async failingNode => {
.slice(0, MAX_NODES_SOURCE_RULE_FETCHED);

// DOM.getDocument is necessary for pushNodesByBackendIdsToFrontend to properly retrieve nodeIds if the `DOM` domain was enabled before this gatherer, invoke it to be safe.
await driver.sendCommand('DOM.getDocument', {depth: -1, pierce: true});

const {nodeIds} = await driver.sendCommand('DOM.pushNodesByBackendIdsToFrontend', {
backendNodeIds: nodesToAnalyze.map(node => node.parentNode.backendNodeId),
});

const analysisPromises = nodesToAnalyze
.map(async (failingNode, i) => {
failingNode.nodeId = nodeIds[i];
try {
const cssRule = await fetchSourceRule(driver, failingNode.node);
const cssRule = await fetchSourceRule(driver, nodeIds[i]);
failingNode.cssRule = cssRule;
} catch (err) {
// The node was deleted. We don't need to distinguish between lack-of-rule
Expand All @@ -240,25 +220,35 @@ class FontSize extends Gatherer {
}

/**
* Maps backendNodeId of TextNodes to {fontSize, textLength}.
*
* Returns the TextNodes in a DOM Snapshot.
* Every entry is associated with a TextNode in the layout tree (not display: none).
* @param {LH.Crdp.DOMSnapshot.CaptureSnapshotResponse} snapshot
* @return {BackendIdsToFontData}
*/
calculateBackendIdsToFontData(snapshot) {
getTextNodesInLayoutFromSnapshot(snapshot) {
const strings = snapshot.strings;
/** @param {number} index */
const getString = (index) => strings[index];
/** @param {number} index */
const getFloat = (index) => parseFloat(strings[index]);

/** @type {BackendIdsToFontData} */
const backendIdsToFontData = new Map();

const textNodesData = [];
for (let j = 0; j < snapshot.documents.length; j++) {
// `doc` is a flattened property list describing all the Nodes in a document, with all string
// values deduped in the `strings` array.
const doc = snapshot.documents[j];

if (!doc.nodes.backendNodeId) {
if (!doc.nodes.backendNodeId || !doc.nodes.parentIndex ||
!doc.nodes.attributes || !doc.nodes.nodeName) {
throw new Error('Unexpected response from DOMSnapshot.captureSnapshot.');
}
const nodes = /** @type {Required<typeof doc['nodes']>} */ (doc.nodes);

/** @param {number} parentIndex */
const getParentData = (parentIndex) => ({
backendNodeId: nodes.backendNodeId[parentIndex],
attributes: nodes.attributes[parentIndex].map(getString),
nodeName: getString(nodes.nodeName[parentIndex]),
});

for (const layoutIndex of doc.textBoxes.layoutIndex) {
const text = strings[doc.layout.text[layoutIndex]];
Expand All @@ -267,45 +257,50 @@ class FontSize extends Gatherer {
const nodeIndex = doc.layout.nodeIndex[layoutIndex];
const styles = doc.layout.styles[layoutIndex];
const [fontSizeStringId] = styles;
const fontSize = getFloat(fontSizeStringId);

const parentIndex = nodes.parentIndex[nodeIndex];
const grandParentIndex = nodes.parentIndex[parentIndex];
const parentNode = getParentData(parentIndex);
const grandParentNode =
grandParentIndex !== undefined ? getParentData(grandParentIndex) : undefined;

const fontSize = parseFloat(strings[fontSizeStringId]);
backendIdsToFontData.set(doc.nodes.backendNodeId[nodeIndex], {
textNodesData.push({
nodeIndex,
backendNodeId: nodes.backendNodeId[nodeIndex],
fontSize,
textLength: getTextLength(text),
parentNode: {
...parentNode,
parentNode: grandParentNode,
},
});
}
}

return backendIdsToFontData;
return textNodesData;
}

/**
* The only connection between a snapshot Node and an actual Protocol Node is backendId,
* so that is used to join the two data structures. DOMSnapshot.captureSnapshot doesn't
* give the entire Node object, so DOM.getFlattenedDocument is used.
* @param {BackendIdsToFontData} backendIdsToFontData
* @param {LH.Artifacts.FontSize.DomNodeWithParent[]} crdpNodes
* Get all the failing text nodes that don't meet the legible text threshold.
* @param {LH.Crdp.DOMSnapshot.CaptureSnapshotResponse} snapshot
*/
findFailingNodes(backendIdsToFontData, crdpNodes) {
findFailingNodes(snapshot) {
/** @type {NodeFontData[]} */
const failingNodes = [];
let totalTextLength = 0;
let failingTextLength = 0;

for (const crdpNode of crdpNodes) {
const partialFontData = backendIdsToFontData.get(crdpNode.backendNodeId);
if (!partialFontData) continue;
// `crdpNode` is a non-empty TextNode that is in the layout tree (not display: none).

const {fontSize, textLength} = partialFontData;
totalTextLength += textLength;
if (fontSize < MINIMAL_LEGIBLE_FONT_SIZE_PX) {
for (const textNodeData of this.getTextNodesInLayoutFromSnapshot(snapshot)) {
totalTextLength += textNodeData.textLength;
if (textNodeData.fontSize < MINIMAL_LEGIBLE_FONT_SIZE_PX) {
// Once a bad TextNode is identified, its parent Node is needed.
failingTextLength += textLength;
failingTextLength += textNodeData.textLength;
failingNodes.push({
node: crdpNode.parentNode,
textLength,
fontSize,
nodeId: 0, // Set later in fetchFailingNodeSourceRules.
parentNode: textNodeData.parentNode,
textLength: textNodeData.textLength,
fontSize: textNodeData.fontSize,
});
}
}
Expand All @@ -331,20 +326,15 @@ class FontSize extends Gatherer {
]);

// Get the computed font-size style of every node.
const snapshotPromise = passContext.driver.sendCommand('DOMSnapshot.captureSnapshot', {
const snapshot = await passContext.driver.sendCommand('DOMSnapshot.captureSnapshot', {
computedStyles: ['font-size'],
});
const allNodesPromise = getAllNodesFromBody(passContext.driver);
const [snapshot, crdpNodes] = await Promise.all([snapshotPromise, allNodesPromise]);
const backendIdsToFontData = this.calculateBackendIdsToFontData(snapshot);
// `backendIdsToFontData` will include all non-empty TextNodes.
// `crdpNodes` will only contain the body node and its descendants.

const {
totalTextLength,
failingTextLength,
failingNodes,
} = this.findFailingNodes(backendIdsToFontData, crdpNodes);
} = this.findFailingNodes(snapshot);
const {
analyzedFailingNodesData,
analyzedFailingTextLength,
Expand Down
Loading

0 comments on commit 7a462a2

Please sign in to comment.