core: enhanced noopener output (#5857)

GoogleChrome · Aug 20, 2018 · 8e18e75 · 8e18e75
1 parent d103157
commit 8e18e75
Show file tree

Hide file tree

Showing 14 changed files with 78 additions and 114 deletions.
diff --git a/lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js b/lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js
@@ -39,7 +39,7 @@ module.exports = [
       },
       'external-anchors-use-rel-noopener': {
         score: 0,
-        warnings: [/Unable to determine/],
+        warnings: [/Unable to determine.*<a target="_blank">/],
         details: {
           items: {
             length: 3,

diff --git a/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js b/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js
@@ -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;
         }
@@ -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 || '',
         };
       });
 

diff --git a/lighthouse-core/gather/gatherers/accessibility.js b/lighthouse-core/gather/gatherers/accessibility.js
@@ -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.
@@ -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;
@@ -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 {
@@ -106,6 +96,7 @@ class Accessibility extends Gatherer {
   afterPass(passContext) {
     const driver = passContext.driver;
     const expression = `(function () {
+      ${pageFunctions.getOuterHTMLSnippet.toString()};
       ${axeLibSource};
       return (${runA11yChecks.toString()}());
     })()`;

diff --git a/lighthouse-core/gather/gatherers/dobetterweb/anchors-with-no-rel-noopener.js b/lighthouse-core/gather/gatherers/dobetterweb/anchors-with-no-rel-noopener.js
@@ -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 {
   /**
@@ -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),
       }));
     })()`;
 

diff --git a/lighthouse-core/gather/gatherers/dobetterweb/domstats.js b/lighthouse-core/gather/gatherers/dobetterweb/domstats.js
@@ -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
@@ -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));

diff --git a/lighthouse-core/gather/gatherers/dobetterweb/password-inputs-with-prevented-paste.js b/lighthouse-core/gather/gatherers/dobetterweb/password-inputs-with-prevented-paste.js
@@ -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),
     }));
 }
@@ -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()}());
+    })()`);
   }
 }
 

diff --git a/lighthouse-core/gather/gatherers/image-usage.js b/lighthouse-core/gather/gatherers/image-usage.js
@@ -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 */
@@ -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()})();
     })()`;
 

diff --git a/lighthouse-core/gather/gatherers/seo/crawlable-links.js b/lighthouse-core/gather/gatherers/seo/crawlable-links.js
@@ -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 {
   /**
@@ -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

diff --git a/lighthouse-core/gather/gatherers/seo/embedded-content.js b/lighthouse-core/gather/gatherers/seo/embedded-content.js
@@ -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 {
   /**
@@ -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

diff --git a/lighthouse-core/lib/dom-helpers.js b/lighthouse-core/lib/dom-helpers.js
diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js
@@ -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.
@@ -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,
 };
diff --git a/lighthouse-core/test/results/artifacts/artifacts.json b/lighthouse-core/test/results/artifacts/artifacts.json
@@ -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"
     },