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(meta-elements): add NodeDetails #12274

Merged
merged 4 commits into from
Apr 6, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion lighthouse-core/audits/seo/is-crawlable.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class IsCrawlable extends Audit {
if (isBlocking) {
blockingDirectives.push({
source: {
type: /** @type {'node'} */ ('node'),
...Audit.makeNodeItem(metaRobots.node),
snippet: `<meta name="robots" content="${metaRobotsContent}" />`,
},
});
Expand Down
18 changes: 14 additions & 4 deletions lighthouse-core/gather/gatherers/meta-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,18 @@
const FRGatherer = require('../../fraggle-rock/gather/base-gatherer.js');
const pageFunctions = require('../../lib/page-functions.js');

/* globals getElementsInDocument */
/* globals getElementsInDocument getNodeDetails */

/* c8 ignore start */
function collectMetaElements() {
// @ts-expect-error - getElementsInDocument put into scope via stringification
const metas = /** @type {HTMLMetaElement[]} */ (getElementsInDocument('head meta'));
const functions = /** @type {typeof pageFunctions} */({
// @ts-expect-error - getElementsInDocument put into scope via stringification
getElementsInDocument,
// @ts-expect-error - getNodeDetails put into scope via stringification
getNodeDetails,
});

const metas = functions.getElementsInDocument('head meta');
return metas.map(meta => {
/** @param {string} name */
const getAttribute = name => {
Expand All @@ -27,6 +33,7 @@ function collectMetaElements() {
property: getAttribute('property'),
httpEquiv: meta.httpEquiv ? meta.httpEquiv.toLowerCase() : undefined,
charset: getAttribute('charset'),
node: functions.getNodeDetails(meta),
};
});
}
Expand All @@ -50,7 +57,10 @@ class MetaElements extends FRGatherer {
return driver.executionContext.evaluate(collectMetaElements, {
args: [],
useIsolation: true,
deps: [pageFunctions.getElementsInDocument],
deps: [
pageFunctions.getElementsInDocument,
pageFunctions.getNodeDetailsString,
],
});
}
}
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/lib/asset-saver.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ async function saveArtifacts(artifacts, basePath) {
}

// save everything else, using a replacer to serialize LHErrors in the artifacts.
const restArtifactsString = JSON.stringify(restArtifacts, stringifyReplacer, 2);
const restArtifactsString = JSON.stringify(restArtifacts, stringifyReplacer, 2) + '\n';
fs.writeFileSync(`${basePath}/${artifactsFilename}`, restArtifactsString, 'utf8');
log.log('Artifacts saved to disk in folder:', basePath);
log.timeEnd(status);
Expand Down
7 changes: 6 additions & 1 deletion lighthouse-core/test/audits/seo/is-crawlable-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const networkRecordsToDevtoolsLog = require('../../network-records-to-devtools-l
/* eslint-env jest */

describe('SEO: Is page crawlable audit', () => {
const makeMetaElements = content => [{name: 'robots', content}];
const makeMetaElements = content => [{name: 'robots', content, node: {}}];

it('fails when page is blocked from indexing with a robots metatag', () => {
const robotsValues = [
Expand Down Expand Up @@ -323,6 +323,11 @@ describe('SEO: Is page crawlable audit', () => {
Array [
Object {
"source": Object {
"boundingRect": undefined,
"lhId": undefined,
"nodeLabel": undefined,
"path": undefined,
"selector": undefined,
"snippet": "<meta name=\\"robots\\" content=\\"noindex\\" />",
"type": "node",
},
Expand Down
52 changes: 49 additions & 3 deletions lighthouse-core/test/results/artifacts/artifacts.json
Original file line number Diff line number Diff line change
Expand Up @@ -711,16 +711,62 @@
"MetaElements": [
{
"name": "",
"content": ""
"content": "",
"charset": "utf-8",
"node": {
"lhId": "5-22-META",
"devtoolsNodePath": "3,HTML,0,HEAD,1,META",
"selector": "head > meta",
"boundingRect": {
"top": 0,
"bottom": 0,
"left": 0,
"right": 0,
"width": 0,
"height": 0
},
"snippet": "<meta charset=\"utf-8\">",
"nodeLabel": "meta"
}
},
{
"name": "viewport",
"content": "width=device-width, initial-scale=1, minimum-scale=1"
"content": "width=device-width, initial-scale=1, minimum-scale=1",
"node": {
"lhId": "5-23-META",
"devtoolsNodePath": "3,HTML,0,HEAD,2,META",
"selector": "head > meta",
"boundingRect": {
"top": 0,
"bottom": 0,
"left": 0,
"right": 0,
"width": 0,
"height": 0
},
"snippet": "<meta name=\"viewport\" content=\"width=device-width, initial-scale=1, minimum-scale=1\">",
"nodeLabel": "meta"
}
},
{
"name": "",
"content": "Open Graph smoke test description",
"property": "og:description"
"property": "og:description",
"node": {
"lhId": "5-24-META",
"devtoolsNodePath": "3,HTML,0,HEAD,3,META",
"selector": "head > meta",
"boundingRect": {
"top": 0,
"bottom": 0,
"left": 0,
"right": 0,
"width": 0,
"height": 0
},
"snippet": "<meta property=\"og:description\" content=\"Open Graph smoke test description\">",
"nodeLabel": "meta"
}
}
],
"ConsoleMessages": [
Expand Down
8 changes: 4 additions & 4 deletions lighthouse-core/test/results/sample_v2.json
Original file line number Diff line number Diff line change
Expand Up @@ -4056,9 +4056,9 @@
},
"charset": {
"id": "charset",
"title": "Charset declaration is missing or occurs too late in the HTML",
"title": "Properly defines charset",
"description": "A character encoding declaration is required. It can be done with a `<meta>` tag in the first 1024 bytes of the HTML or in the Content-Type HTTP response header. [Learn more](https://web.dev/charset/).",
"score": 0,
"score": 1,
"scoreDisplayMode": "binary"
},
"dom-size": {
Expand Down Expand Up @@ -5564,7 +5564,7 @@
}
],
"id": "best-practices",
"score": 0.13
"score": 0.19
},
"seo": {
"title": "SEO",
Expand Down Expand Up @@ -8227,7 +8227,7 @@
"lighthouse-core/audits/dobetterweb/doctype.js | description": [
"audits.doctype.description"
],
"lighthouse-core/audits/dobetterweb/charset.js | failureTitle": [
"lighthouse-core/audits/dobetterweb/charset.js | title": [
"audits.charset.title"
],
"lighthouse-core/audits/dobetterweb/charset.js | description": [
Expand Down
2 changes: 1 addition & 1 deletion types/artifacts.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ declare global {
/** All the link elements on the page or equivalently declared in `Link` headers. @see https://html.spec.whatwg.org/multipage/links.html */
LinkElements: Artifacts.LinkElement[];
/** The values of the <meta> elements in the head. */
MetaElements: Array<{name?: string, content?: string, property?: string, httpEquiv?: string, charset?: string}>;
MetaElements: Array<{name?: string, content?: string, property?: string, httpEquiv?: string, charset?: string, node: LH.Artifacts.NodeDetails}>;
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
/** Information on all script elements in the page. Also contains the content of all requested scripts and the networkRecord requestId that contained their content. Note, HTML documents will have one entry per script tag, all with the same requestId. */
ScriptElements: Array<Artifacts.ScriptElement>;
/** The dimensions and devicePixelRatio of the loaded viewport. */
Expand Down