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

new_audit(crawlable-anchors): adds an anchor-href audit #10662

Merged
merged 18 commits into from
May 20, 2020
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
8 changes: 8 additions & 0 deletions lighthouse-cli/test/cli/__snapshots__/index-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,9 @@ Object {
Object {
"path": "seo/link-text",
},
Object {
"path": "seo/crawlable-anchors",
},
Object {
"path": "seo/is-crawlable",
},
Expand Down Expand Up @@ -1117,6 +1120,11 @@ Object {
"id": "link-text",
"weight": 1,
},
Object {
"group": "seo-crawl",
"id": "crawlable-anchors",
"weight": 1,
},
Object {
"group": "seo-crawl",
"id": "is-crawlable",
Expand Down
18 changes: 18 additions & 0 deletions lighthouse-cli/test/fixtures/seo/seo-failure-cases.html
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,24 @@ <h2>Anchor text</h2>
</a>
</svg>

<!-- FAIL(crawlable-anchors): Link using window.open() -->
<a onclick="window.open(`http://example.com`)">Some link</a>

<!-- FAIL(crawlable-anchors): Link using javascript:void -->
<a href="javascript:void(0)">Some link</a>

<!-- FAIL(crawlable-anchors): Link with a mousedown event listener -->
<a class="some-link">Some link</a>
<script>
document.querySelector('.some-link').addEventListener('mousedown', () => {});
</script>

<!-- FAIL(crawlable-anchors): Link event listener on parent -->
<div class="link-parent"><a>Some link</a></div>
<script>
document.querySelector('.link-parent').addEventListener('click', () => {});
</script>

<!-- FAIL(plugins): java applet on page -->
<applet code=HelloWorld.class width="200" height="200" id='applet'>
Your browser does not support the <code>applet</code> tag.
Expand Down
14 changes: 13 additions & 1 deletion lighthouse-cli/test/fixtures/seo/seo-tester.html
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ <h2>Anchor text</h2>
<a href='https://example.com'>descriptive link</a>
<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>
<a href='javascript:;'>click this</a>

<h2>Small text</h2>
<!-- PASS(font-size): amount of illegible text is below the 60% threshold -->
Expand All @@ -66,5 +66,17 @@ <h6>2</h6>
<script>URL = '';</script>
<!-- PASS(plugins): external content does not require java, flash or silverlight -->
<object data="test.pdf" type="application/pdf" width="300" height="200"></object>

<!-- PASS(crawlable-anchors): Link with a relative path -->
<a href="/pass">Some link</a>

<!-- PASS(crawlable-anchors): Link with a click event listener -->
<a class="some-link">Some link</a>
<script>
document.querySelector('.some-link').addEventListener('click', () => {});
</script>

<!-- PASS(crawlable-anchors): Link with an onclick which won't match the audit regex -->
<a onclick="window.Location = '';">Some link</a>
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,9 @@ const expectations = [
],
},
},
'crawlable-anchors': {
score: 1,
},
'link-text': {
score: 1,
},
Expand Down Expand Up @@ -242,6 +245,14 @@ const expectations = [
explanation:
'Text is illegible because there\'s no viewport meta tag optimized for mobile screens.',
},
'crawlable-anchors': {
score: 0,
details: {
items: {
length: 4,
},
},
},
'link-text': {
score: 0,
displayValue: '4 links found',
Expand Down Expand Up @@ -308,6 +319,9 @@ const expectations = [
'font-size': {
score: null,
},
'crawlable-anchors': {
score: null,
},
'link-text': {
score: null,
},
Expand Down
100 changes: 100 additions & 0 deletions lighthouse-core/audits/seo/crawlable-anchors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/**
* @license Copyright 2020 The Lighthouse Authors. 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 Audit = require('../audit.js');
const i18n = require('../../lib/i18n/i18n.js');

const UIStrings = {
/** Title of a Lighthouse audit that provides detail on whether links have potentially-crawlable href attributes. This descriptive title is shown when all links on the page are potentially-crawlable. */
title: 'Links are crawlable',
/** Descriptive title of a Lighthouse audit that provides detail on whether links have potentially-crawlable href attributes. This descriptive title is shown when there are href attributes which are not crawlable by search engines. */
failureTitle: 'Links are not crawlable',
/** Description of a Lighthouse audit that tells the user why href attributes on links should be crawlable. This is displayed after a user expands the section to see more. 'Learn More' becomes link text to additional documentation. */
description: 'Search engines may use `href` attributes on links to crawl websites. Ensure that the `href` attribute of anchor elements links to an appropriate destination, so more pages of the site can be discovered. [Learn More](https://support.google.com/webmasters/answer/9112205)',
/** Label for a column in a data table; entries will be the HTML anchor elements that failed the audit. Anchors are DOM elements that are links. */
columnFailingLink: 'Uncrawlable Link',
};

const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);

class CrawlableAnchors extends Audit {
/**
* @return {LH.Audit.Meta}
*/
static get meta() {
return {
id: 'crawlable-anchors',
title: str_(UIStrings.title),
failureTitle: str_(UIStrings.failureTitle),
description: str_(UIStrings.description),
requiredArtifacts: ['AnchorElements'],
};
}

/**
* @param {LH.Artifacts} artifacts
* @return {LH.Audit.Product}
*/
static audit({AnchorElements: anchorElements}) {
const failingAnchors = anchorElements.filter(({
rawHref,
listeners = [],
onclick = '',
name = '',
role = '',
}) => {
onclick = onclick.replace( /\s/g, '');
patrickhulce marked this conversation as resolved.
Show resolved Hide resolved
rawHref = rawHref.replace( /\s/g, '');
name = name.trim();
role = role.trim();

if (role.length > 0) return;

const windowLocationRegExp = /window\.location=/;
const windowOpenRegExp = /window\.open\(/;
const javaScriptVoidRegExp = /javascript:void(\(|)0(\)|)/;

if (rawHref.startsWith('file:')) return true;
if (windowLocationRegExp.test(onclick)) return true;
if (windowOpenRegExp.test(onclick)) return true;

const hasClickHandler = listeners.some(({type}) => type === 'click');
if (hasClickHandler || name.length > 0) return;

if (rawHref === '') return true;
if (javaScriptVoidRegExp.test(rawHref)) return true;
});

/** @type {LH.Audit.Details.Table['headings']} */
const headings = [{
key: 'node',
itemType: 'node',
text: str_(UIStrings.columnFailingLink),
}];

/** @type {LH.Audit.Details.Table['items']} */
const itemsToDisplay = failingAnchors.map(anchor => {
return {
node: {
type: 'node',
umaar marked this conversation as resolved.
Show resolved Hide resolved
path: anchor.devtoolsNodePath || '',
selector: anchor.selector || '',
nodeLabel: anchor.nodeLabel || '',
snippet: anchor.outerHTML || '',
},
};
});

return {
score: Number(failingAnchors.length === 0),
details: Audit.makeTableDetails(headings, itemsToDisplay),
};
}
}

module.exports = CrawlableAnchors;
module.exports.UIStrings = UIStrings;
2 changes: 2 additions & 0 deletions lighthouse-core/config/default-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ const defaultConfig = {
'seo/http-status-code',
'seo/font-size',
'seo/link-text',
'seo/crawlable-anchors',
'seo/is-crawlable',
'seo/robots-txt',
'seo/tap-targets',
Expand Down Expand Up @@ -563,6 +564,7 @@ const defaultConfig = {
{id: 'meta-description', weight: 1, group: 'seo-content'},
{id: 'http-status-code', weight: 1, group: 'seo-crawl'},
{id: 'link-text', weight: 1, group: 'seo-content'},
{id: 'crawlable-anchors', weight: 1, group: 'seo-crawl'},
{id: 'is-crawlable', weight: 1, group: 'seo-crawl'},
{id: 'robots-txt', weight: 1, group: 'seo-crawl'},
{id: 'image-alt', weight: 1, group: 'seo-content'},
Expand Down
53 changes: 51 additions & 2 deletions lighthouse-core/gather/gatherers/anchor-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ function collectAnchorElements() {
}
};

/** @param {HTMLAnchorElement|SVGAElement} node */
function getTruncatedOnclick(node) {
const onclick = node.getAttribute('onclick') || '';
return onclick.slice(0, 1024);
}

/** @type {Array<HTMLAnchorElement|SVGAElement>} */
// @ts-ignore - put into scope via stringification
const anchorElements = getElementsInDocument('a'); // eslint-disable-line no-undef
Expand All @@ -47,6 +53,10 @@ function collectAnchorElements() {
if (node instanceof HTMLAnchorElement) {
return {
href: node.href,
rawHref: node.getAttribute('href') || '',
onclick: getTruncatedOnclick(node),
role: node.getAttribute('role') || '',
name: node.name,
text: node.innerText, // we don't want to return hidden text, so use innerText
rel: node.rel,
target: node.target,
Expand All @@ -59,6 +69,9 @@ function collectAnchorElements() {

return {
href: resolveURLOrEmpty(node.href.baseVal),
rawHref: node.getAttribute('href') || '',
patrickhulce marked this conversation as resolved.
Show resolved Hide resolved
onclick: getTruncatedOnclick(node),
role: node.getAttribute('role') || '',
text: node.textContent || '',
rel: '',
target: node.target.baseVal || '',
Expand All @@ -70,6 +83,26 @@ function collectAnchorElements() {
});
}

/**
* @param {LH.Gatherer.PassContext['driver']} driver
* @param {string} devtoolsNodePath
*/
async function getEventListeners(driver, devtoolsNodePath) {
const {nodeId} = await driver.sendCommand('DOM.pushNodeByPathToFrontend', {
path: devtoolsNodePath,
});

const {object: {objectId = ''}} = await driver.sendCommand('DOM.resolveNode', {
nodeId,
});

const response = await driver.sendCommand('DOMDebugger.getEventListeners', {
objectId,
});

return response.listeners.map(({type}) => ({type}));
}

class AnchorElements extends Gatherer {
/**
* @param {LH.Gatherer.PassContext} passContext
Expand All @@ -87,8 +120,24 @@ class AnchorElements extends Gatherer {
return (${collectAnchorElements})();
})()`;

/** @type {Array<LH.Artifacts.AnchorElement>} */
return driver.evaluateAsync(expression, {useIsolation: true});
/** @type {LH.Artifacts['AnchorElements']} */
const anchors = await driver.evaluateAsync(expression, {useIsolation: true});
await driver.sendCommand('DOM.enable');

// 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 anchorsWithEventListeners = anchors.map(async anchor => {
const listeners = await getEventListeners(driver, anchor.devtoolsNodePath);

return {
...anchor,
listeners,
};
});

const result = await Promise.all(anchorsWithEventListeners);
await driver.sendCommand('DOM.disable');
return result;
}
}

Expand Down
12 changes: 12 additions & 0 deletions lighthouse-core/lib/i18n/locales/en-US.json
Original file line number Diff line number Diff line change
Expand Up @@ -1007,6 +1007,18 @@
"lighthouse-core/audits/seo/canonical.js | title": {
"message": "Document has a valid `rel=canonical`"
},
"lighthouse-core/audits/seo/crawlable-anchors.js | columnFailingLink": {
"message": "Uncrawlable Link"
},
"lighthouse-core/audits/seo/crawlable-anchors.js | description": {
"message": "Search engines may use `href` attributes on links to crawl websites. Ensure that the `href` attribute of anchor elements links to an appropriate destination, so more pages of the site can be discovered. [Learn More](https://support.google.com/webmasters/answer/9112205)"
},
"lighthouse-core/audits/seo/crawlable-anchors.js | failureTitle": {
"message": "Links are not crawlable"
},
"lighthouse-core/audits/seo/crawlable-anchors.js | title": {
"message": "Links are crawlable"
},
"lighthouse-core/audits/seo/font-size.js | description": {
"message": "Font sizes less than 12px are too small to be legible and require mobile visitors to “pinch to zoom” in order to read. Strive to have >60% of page text ≥12px. [Learn more](https://web.dev/font-size)."
},
Expand Down
12 changes: 12 additions & 0 deletions lighthouse-core/lib/i18n/locales/en-XL.json
Original file line number Diff line number Diff line change
Expand Up @@ -1007,6 +1007,18 @@
"lighthouse-core/audits/seo/canonical.js | title": {
"message": "D̂óĉúm̂én̂t́ ĥáŝ á v̂ál̂íd̂ `rel=canonical`"
},
"lighthouse-core/audits/seo/crawlable-anchors.js | columnFailingLink": {
"message": "Ûńĉŕâẃl̂áb̂ĺê Ĺîńk̂"
},
"lighthouse-core/audits/seo/crawlable-anchors.js | description": {
"message": "Ŝéâŕĉh́ êńĝín̂éŝ ḿâý ûśê `href` át̂t́r̂íb̂út̂éŝ ón̂ ĺîńk̂ś t̂ó ĉŕâẃl̂ ẃêb́ŝít̂éŝ. Én̂śûŕê t́ĥát̂ t́ĥé `href` ât́t̂ŕîb́ût́ê óf̂ án̂ćĥór̂ él̂ém̂én̂t́ŝ ĺîńk̂ś t̂ó âń âṕp̂ŕôṕr̂íât́ê d́êśt̂ín̂át̂íôń, ŝó m̂ór̂é p̂áĝéŝ óf̂ t́ĥé ŝít̂é ĉán̂ b́ê d́îśĉóv̂ér̂éd̂. [Ĺêár̂ń M̂ór̂é](https://support.google.com/webmasters/answer/9112205)"
},
"lighthouse-core/audits/seo/crawlable-anchors.js | failureTitle": {
"message": "L̂ín̂ḱŝ ár̂é n̂ót̂ ćr̂áŵĺâb́l̂é"
},
"lighthouse-core/audits/seo/crawlable-anchors.js | title": {
"message": "L̂ín̂ḱŝ ár̂é ĉŕâẃl̂áb̂ĺê"
},
"lighthouse-core/audits/seo/font-size.js | description": {
"message": "F̂ón̂t́ ŝíẑéŝ ĺêśŝ t́ĥán̂ 12ṕx̂ ár̂é t̂óô śm̂ál̂ĺ t̂ó b̂é l̂éĝíb̂ĺê án̂d́ r̂éq̂úîŕê ḿôb́îĺê v́îśît́ôŕŝ t́ô “ṕîńĉh́ t̂ó ẑóôḿ” îń ôŕd̂ér̂ t́ô ŕêád̂. Śt̂ŕîv́ê t́ô h́âv́ê >60% óf̂ ṕâǵê t́êx́t̂ ≥12ṕx̂. [Ĺêár̂ń m̂ór̂é](https://web.dev/font-size)."
},
Expand Down
Loading