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: enhanced noopener output #5857

Merged
merged 2 commits into from
Aug 20, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ module.exports = [
},
'external-anchors-use-rel-noopener': {
score: 0,
warnings: [/Unable to determine/],
warnings: [/Unable to determine.*<a target="_blank">external link/],
details: {
items: {
length: 3,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,12 @@ class ExternalAnchorsUseRelNoopenerAudit extends Audit {
const warnings = [];
const pageHost = new URL(artifacts.URL.finalUrl).host;
// Filter usages to exclude anchors that are same origin
// TODO: better extendedInfo for anchors with no href attribute:
// https://github.com/GoogleChrome/lighthouse/issues/1233
// https://github.com/GoogleChrome/lighthouse/issues/1345
const failingAnchors = artifacts.AnchorsWithNoRelNoopener
.filter(anchor => {
try {
return new URL(anchor.href).host !== pageHost;
} catch (err) {
// TODO(phulce): make this message better with anchor.outerHTML
warnings.push('Unable to determine the destination for anchor tag. ' +
warnings.push(`Unable to determine the destination for anchor (${anchor.outerHTML}). ` +
'If not used as a hyperlink, consider removing target=_blank.');
return true;
}
Expand All @@ -55,10 +51,7 @@ class ExternalAnchorsUseRelNoopenerAudit extends Audit {
href: anchor.href || 'Unknown',
target: anchor.target || '',
rel: anchor.rel || '',
url: '<a' +
(anchor.href ? ` href="${anchor.href}"` : '') +
(anchor.target ? ` target="${anchor.target}"` : '') +
(anchor.rel ? ` rel="${anchor.rel}"` : '') + '>',
outerHTML: anchor.outerHTML || '',
};
});

Expand Down
17 changes: 4 additions & 13 deletions lighthouse-core/gather/gatherers/accessibility.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
*/
'use strict';

/* global window, document, Node */
/* global window, document, Node, getOuterHTMLSnippet */

const Gatherer = require('./gatherer');
const fs = require('fs');
const axeLibSource = fs.readFileSync(require.resolve('axe-core/axe.min.js'), 'utf8');
const pageFunctions = require('../../lib/page-functions');

/**
* This is run in the page, not Lighthouse itself.
Expand Down Expand Up @@ -44,6 +45,7 @@ function runA11yChecks() {
// @ts-ignore
axeResult.violations.forEach(v => v.nodes.forEach(node => {
node.path = getNodePath(node.element);
// @ts-ignore - getOuterHTMLSnippet put into scope via stringification
node.snippet = getOuterHTMLSnippet(node.element);
// avoid circular JSON concerns
node.element = node.any = node.all = node.none = undefined;
Expand Down Expand Up @@ -84,18 +86,6 @@ function runA11yChecks() {
path.reverse();
return path.join(',');
}

/**
* Gets the opening tag text of the given node.
* @param {Element} el
* @return {string}
*/
function getOuterHTMLSnippet(el) {
const reOpeningTag = /^.*?>/;
const match = el.outerHTML.match(reOpeningTag);
// Should always be a snippet, so just fall back to '' to keep a string.
return match && match[0] || '';
}
}

class Accessibility extends Gatherer {
Expand All @@ -106,6 +96,7 @@ class Accessibility extends Gatherer {
afterPass(passContext) {
const driver = passContext.driver;
const expression = `(function () {
${pageFunctions.getOuterHTMLSnippet.toString()};
${axeLibSource};
return (${runA11yChecks.toString()}());
})()`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
'use strict';

const Gatherer = require('../gatherer');
const DOMHelpers = require('../../../lib/dom-helpers.js');
const pageFunctions = require('../../../lib/page-functions.js');

class AnchorsWithNoRelNoopener extends Gatherer {
/**
Expand All @@ -15,13 +15,15 @@ class AnchorsWithNoRelNoopener extends Gatherer {
*/
afterPass(passContext) {
const expression = `(function() {
${DOMHelpers.getElementsInDocumentFnString}; // define function on page
${pageFunctions.getOuterHTMLSnippet.toString()};
${pageFunctions.getElementsInDocument.toString()}; // define function on page
const selector = 'a[target="_blank"]:not([rel~="noopener"]):not([rel~="noreferrer"])';
const elements = getElementsInDocument(selector);
return elements.map(node => ({
href: node.href,
rel: node.getAttribute('rel'),
target: node.getAttribute('target')
target: node.getAttribute('target'),
outerHTML: getOuterHTMLSnippet(node),
}));
})()`;

Expand Down
17 changes: 3 additions & 14 deletions lighthouse-core/gather/gatherers/dobetterweb/domstats.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,12 @@
* and total number of nodes used on the page.
*/

/* global ShadowRoot */
/* global ShadowRoot, getOuterHTMLSnippet */

'use strict';

const Gatherer = require('../gatherer');

/**
* Gets the opening tag text of the given node.
* @param {Element} element
* @return {?string}
*/
/* istanbul ignore next */
function getOuterHTMLSnippet(element) {
const reOpeningTag = /^.*?>/;
const match = element.outerHTML.match(reOpeningTag);
return match && match[0];
}
const pageFunctions = require('../../../lib/page-functions');

/**
* Constructs a pretty label from element's selectors. For example, given
Expand Down Expand Up @@ -151,7 +140,7 @@ class DOMStats extends Gatherer {
*/
afterPass(passContext) {
const expression = `(function() {
${getOuterHTMLSnippet.toString()};
${pageFunctions.getOuterHTMLSnippet.toString()};
${createSelectorsLabel.toString()};
${elementPathInDOM.toString()};
return (${getDOMStats.toString()}(document.documentElement));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,35 +5,25 @@
*/
'use strict';

/* global document ClipboardEvent */
/* global document ClipboardEvent getOuterHTMLSnippet */

const Gatherer = require('../gatherer');
const pageFunctions = require('../../../lib/page-functions');

// This is run in the page, not Lighthouse itself.
/**
* @return {LH.Artifacts['PasswordInputsWithPreventedPaste']}
*/
/* istanbul ignore next */
function findPasswordInputsWithPreventedPaste() {
/**
* Gets the opening tag text of the given element.
* @param {Element} element
* @return {string}
*/
function getOuterHTMLSnippet(element) {
const reOpeningTag = /^.*?>/;
const match = element.outerHTML.match(reOpeningTag);
// @ts-ignore We are confident match was found.
return match && match[0];
}

return Array.from(document.querySelectorAll('input[type="password"]'))
.filter(passwordInput =>
!passwordInput.dispatchEvent(
new ClipboardEvent('paste', {cancelable: true})
)
)
.map(passwordInput => ({
// @ts-ignore - getOuterHTMLSnippet put into scope via stringification
snippet: getOuterHTMLSnippet(passwordInput),
}));
}
Expand All @@ -44,9 +34,10 @@ class PasswordInputsWithPreventedPaste extends Gatherer {
* @return {Promise<LH.Artifacts['PasswordInputsWithPreventedPaste']>}
*/
afterPass(passContext) {
return passContext.driver.evaluateAsync(
`(${findPasswordInputsWithPreventedPaste.toString()}())`
);
return passContext.driver.evaluateAsync(`(() => {
${pageFunctions.getOuterHTMLSnippet.toString()};
return (${findPasswordInputsWithPreventedPaste.toString()}());
})()`);
}
}

Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/gather/gatherers/image-usage.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
'use strict';

const Gatherer = require('./gatherer');
const DOMHelpers = require('../../lib/dom-helpers.js');
const pageFunctions = require('../../lib/page-functions.js');
const Driver = require('../driver.js'); // eslint-disable-line no-unused-vars

/* global window, getElementsInDocument, Image */
Expand Down Expand Up @@ -161,7 +161,7 @@ class ImageUsage extends Gatherer {
}, /** @type {Object<string, LH.Artifacts.SingleImageUsage['networkRecord']>} */ ({}));

const expression = `(function() {
${DOMHelpers.getElementsInDocumentFnString}; // define function on page
${pageFunctions.getElementsInDocument.toString()}; // define function on page
return (${collectImageElementInfo.toString()})();
})()`;

Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/gather/gatherers/seo/crawlable-links.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
'use strict';

const Gatherer = require('../gatherer');
const DOMHelpers = require('../../../lib/dom-helpers.js');
const pageFunctions = require('../../../lib/page-functions.js');

class CrawlableLinks extends Gatherer {
/**
Expand All @@ -15,7 +15,7 @@ class CrawlableLinks extends Gatherer {
*/
afterPass(passContext) {
const expression = `(function() {
${DOMHelpers.getElementsInDocumentFnString}; // define function on page
${pageFunctions.getElementsInDocument.toString()}; // define function on page
const selector = 'a[href]:not([rel~="nofollow"])';
const elements = getElementsInDocument(selector);
return elements
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/gather/gatherers/seo/embedded-content.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
'use strict';

const Gatherer = require('../gatherer');
const DOMHelpers = require('../../../lib/dom-helpers.js');
const pageFunctions = require('../../../lib/page-functions.js');

class EmbeddedContent extends Gatherer {
/**
Expand All @@ -15,7 +15,7 @@ class EmbeddedContent extends Gatherer {
*/
afterPass(passContext) {
const expression = `(function() {
${DOMHelpers.getElementsInDocumentFnString}; // define function on page
${pageFunctions.getElementsInDocument.toString()}; // define function on page
const selector = 'object, embed, applet';
const elements = getElementsInDocument(selector);
return elements
Expand Down
46 changes: 0 additions & 46 deletions lighthouse-core/lib/dom-helpers.js

This file was deleted.

43 changes: 42 additions & 1 deletion lighthouse-core/lib/page-functions.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
// @ts-nocheck
'use strict';

/* global window */
/* global window document */

/**
* Helper functions that are passed by `toString()` by Driver to be evaluated in target page.
Expand Down Expand Up @@ -78,8 +78,49 @@ function checkTimeSinceLastLongTask() {
});
}

/**
* @param {string=} selector Optional simple CSS selector to filter nodes on.
* Combinators are not supported.
* @return {Array<Element>}
*/
/* istanbul ignore next */
function getElementsInDocument(selector) {
/** @type {Array<Element>} */
const results = [];

/** @param {NodeListOf<Element>} nodes */
const _findAllElements = nodes => {
for (let i = 0, el; el = nodes[i]; ++i) {
if (!selector || el.matches(selector)) {
results.push(el);
}
// If the element has a shadow root, dig deeper.
if (el.shadowRoot) {
_findAllElements(el.shadowRoot.querySelectorAll('*'));
}
}
};
_findAllElements(document.querySelectorAll('*'));

return results;
}

/**
* Gets the opening tag text of the given node.
* @param {Element} element
* @return {string}
*/
/* istanbul ignore next */
function getOuterHTMLSnippet(element) {
const reOpeningTag = /^.*?>/;
const match = element.outerHTML.match(reOpeningTag);
return match && match[0] || '';
}

module.exports = {
wrapRuntimeEvalErrorInBrowser,
registerPerformanceObserverInPage,
checkTimeSinceLastLongTask,
getElementsInDocument,
getOuterHTMLSnippet,
};
3 changes: 3 additions & 0 deletions lighthouse-core/test/results/artifacts/artifacts.json
Original file line number Diff line number Diff line change
Expand Up @@ -2227,16 +2227,19 @@
"AnchorsWithNoRelNoopener": [
{
"href": "https://www.google.com/",
"outerHTML": "<a href=\"https://www.google.com/\" target=\"blank\">Hello</a>",
"rel": null,
"target": "_blank"
},
{
"href": "",
"outerHTML": "<a target=\"blank\">Hello</a>",
"rel": null,
"target": "_blank"
},
{
"href": "https://www.google.com/",
"outerHTML": "<a rel=\"nofollow\" href=\"https://www.google.com/\" target=\"blank\">Hello</a>",
"rel": "nofollow",
"target": "_blank"
},
Expand Down
Loading