handle anchor tags with no href in noopener audit (#1238)

GoogleChrome · Dec 22, 2016 · ef95039 · ef95039
1 parent c5402c9
commit ef95039
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 6 deletions.
diff --git a/lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html b/lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html
@@ -82,6 +82,9 @@ <h2>Do better web tester page</h2>
 <template id="noopener-links-tmpl">
   <!-- FAIL - does not use rel="noopener" to open external link -->
   <a href="https://www.google.com/" target="_blank">external link</a>
+  <!-- FAIL - does not use rel="noopener" and has no href attribute, giving an
+    href value of '' when read, which will throw in a `new URL('')` constructor -->
+  <a target="_blank">external link</a>
   <!-- PASS -->
   <a href="https://www.google.com/" target="_blank" rel="noopener nofollow">external link that uses rel noopener and another unrelated rel attribute</a>
   <!-- PASS -->

diff --git a/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js b/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js
@@ -51,15 +51,16 @@ class ExternalAnchorsUseRelNoopenerAudit extends Audit {
 
     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
     const failingAnchors = artifacts.AnchorsWithNoRelNoopener.usages
-      .filter(anchor => new URL(anchor.href).host !== pageHost)
+      .filter(anchor => anchor.href === '' || new URL(anchor.href).host !== pageHost)
       .map(anchor => {
         return {
-          url: `<a
-            href="${anchor.href}"
-            ${anchor.target ? ` target="${anchor.target}"` : ''}
-            ${anchor.rel ? ` rel="${anchor.rel}"` : ''}>...
-          </a>`
+          url: '<a' +
+              (anchor.href ? ` href="${anchor.href}"` : '') +
+              (anchor.target ? ` target="${anchor.target}"` : '') +
+              (anchor.rel ? ` rel="${anchor.rel}"` : '') + '>'
         };
       });
 

diff --git a/lighthouse-core/test/audits/dobetterweb/external-anchors-use-rel-noopener-test.js b/lighthouse-core/test/audits/dobetterweb/external-anchors-use-rel-noopener-test.js
@@ -59,4 +59,17 @@ describe('External anchors use rel="noopener"', () => {
     assert.equal(auditResult.rawValue, false);
     assert.equal(auditResult.extendedInfo.value.length, 2);
   });
+
+  it('handles links with no href attribute', () => {
+    const auditResult = ExternalAnchorsAudit.audit({
+      AnchorsWithNoRelNoopener: {
+        usages: [
+          {href: ''}
+        ]
+      },
+      URL: {finalUrl: URL},
+    });
+    assert.equal(auditResult.rawValue, false);
+    assert.equal(auditResult.extendedInfo.value.length, 1);
+  });
 });