From d568ded27cd6fb244d51d758bd35ee3af8b6eda3 Mon Sep 17 00:00:00 2001
From: sanjsanj <sanjsanj@hotmail.com>
Date: Tue, 12 Dec 2017 19:25:56 +0000
Subject: [PATCH 1/4] core(rel=noopener-audit): Only test http/https links

---
 .../audits/dobetterweb/external-anchors-use-rel-noopener.js  | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js b/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js
index 296f94dc9647..8e3c5a7545a3 100644
--- a/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js
+++ b/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js
@@ -46,10 +46,7 @@ class ExternalAnchorsUseRelNoopenerAudit extends Audit {
           return true;
         }
       })
-      .filter(anchor => {
-        // Ignore href's that are not real links
-        return !anchor.href || !anchor.href.toLowerCase().startsWith('javascript:');
-      })
+      .filter(anchor => anchor.href.toLowerCase().startsWith('http'))
       .map(anchor => {
         return {
           href: anchor.href || 'Unknown',

From abca3c53e8fa049ff6eff34f5326a2adb2e9a1e5 Mon Sep 17 00:00:00 2001
From: sanjsanj <sanjsanj@hotmail.com>
Date: Tue, 12 Dec 2017 19:52:01 +0000
Subject: [PATCH 2/4] also audit links without an href to fix failing test, add
 test case for the bug that raised this issue

---
 .../dobetterweb/external-anchors-use-rel-noopener.js |  4 +++-
 .../external-anchors-use-rel-noopener-test.js        | 12 ++++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js b/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js
index 8e3c5a7545a3..1e47810887ca 100644
--- a/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js
+++ b/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js
@@ -46,7 +46,9 @@ class ExternalAnchorsUseRelNoopenerAudit extends Audit {
           return true;
         }
       })
-      .filter(anchor => anchor.href.toLowerCase().startsWith('http'))
+      .filter(anchor => {
+        return !anchor.href || anchor.href.toLowerCase().startsWith('http');
+      })
       .map(anchor => {
         return {
           href: anchor.href || 'Unknown',
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
index c89aab41e7c9..02773dd19d76 100644
--- 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
@@ -66,4 +66,16 @@ describe('External anchors use rel="noopener"', () => {
     assert.equal(auditResult.rawValue, true);
     assert.equal(auditResult.details.items.length, 0);
   });
+
+  it('does not fail for links with mailto in href attribute', () => {
+    const auditResult = ExternalAnchorsAudit.audit({
+      AnchorsWithNoRelNoopener: [
+        {href: 'mailto:inbox@email.com'},
+        {href: 'MAILTO:INBOX@EMAIL.COM'},
+      ],
+      URL: {finalUrl: URL},
+    });
+    assert.equal(auditResult.rawValue, true);
+    assert.equal(auditResult.details.items.length, 0);
+  });
 });

From 3aa94952c9d1b370ac80dcb8e42bac7ad0d283ca Mon Sep 17 00:00:00 2001
From: sanjsanj <sanjsanj@hotmail.com>
Date: Tue, 12 Dec 2017 20:50:33 +0000
Subject: [PATCH 3/4] add more robust test coverage

---
 .../test/fixtures/dobetterweb/dbw_tester.html | 12 +++-
 .../external-anchors-use-rel-noopener-test.js | 58 ++++++++++++-------
 2 files changed, 46 insertions(+), 24 deletions(-)

diff --git a/lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html b/lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html
index a79165794080..73349895337a 100644
--- a/lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html
+++ b/lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html
@@ -106,10 +106,18 @@ <h2>Do better web tester page</h2>
   <!-- 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 -->
+  <!-- FAIL - external link that does have a rel attribute but it is not "noopener" -->
+  <a href="https://www.google.com/" target="_blank" rel="nofollow">external link</a>
+  <!-- PASS - external link correctly uses rel="noopener" and an additional rel value -->
   <a href="https://www.google.com/" target="_blank" rel="noopener nofollow">external link that uses rel noopener and another unrelated rel attribute</a>
-  <!-- PASS -->
+  <!-- PASS - external link correctly uses rel="noopener" -->
+  <a href="https://www.google.com/" target="_blank" rel="noopener">external link that uses rel noopener</a>
+  <!-- PASS - internal link without rel="noopener" -->
   <a href="./doesnotexist" target="_blank">internal link is ok</a>
+  <!-- PASS - href uses javascript: -->
+  <a href="javascript:void(0)" target="_blank"></a>
+  <!-- PASS - href uses mailto: -->
+  <a href="mailto:inbox@email.com" target="_blank"></a>
 </template>
 
 <template id="password-inputs-can-be-pasted-into">
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
index 02773dd19d76..fb3bf5a36981 100644
--- 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
@@ -27,6 +27,30 @@ describe('External anchors use rel="noopener"', () => {
     assert.equal(auditResult.details.items.length, 0);
   });
 
+  it('passes when links have javascript in href attribute', () => {
+    const auditResult = ExternalAnchorsAudit.audit({
+      AnchorsWithNoRelNoopener: [
+        {href: 'javascript:void(0)'},
+        {href: 'JAVASCRIPT:void(0)'},
+      ],
+      URL: {finalUrl: URL},
+    });
+    assert.equal(auditResult.rawValue, true);
+    assert.equal(auditResult.details.items.length, 0);
+  });
+
+  it('passes when links have mailto in href attribute', () => {
+    const auditResult = ExternalAnchorsAudit.audit({
+      AnchorsWithNoRelNoopener: [
+        {href: 'mailto:inbox@email.com'},
+        {href: 'MAILTO:INBOX@EMAIL.COM'},
+      ],
+      URL: {finalUrl: URL},
+    });
+    assert.equal(auditResult.rawValue, true);
+    assert.equal(auditResult.details.items.length, 0);
+  });
+
   it('fails when links are from different hosts than the page host', () => {
     const auditResult = ExternalAnchorsAudit.audit({
       AnchorsWithNoRelNoopener: [
@@ -40,42 +64,32 @@ describe('External anchors use rel="noopener"', () => {
     assert.equal(auditResult.details.items.length, 2);
   });
 
-  it('handles links with no href attribute', () => {
+  it('fails when links have no href attribute', () => {
     const auditResult = ExternalAnchorsAudit.audit({
       AnchorsWithNoRelNoopener: [
         {href: ''},
-        {href: 'http://'},
-        {href: 'http:'},
       ],
       URL: {finalUrl: URL},
     });
     assert.equal(auditResult.rawValue, false);
-    assert.equal(auditResult.details.items.length, 3);
-    assert.equal(auditResult.details.items.length, 3);
+    assert.equal(auditResult.details.items.length, 1);
+    assert.equal(auditResult.details.items.length, 1);
     assert.ok(auditResult.debugString, 'includes debugString');
   });
 
-  it('does not fail for links with javascript in href attribute', () => {
+  it('fails when links have href attribute starting with a protocol', () => {
     const auditResult = ExternalAnchorsAudit.audit({
       AnchorsWithNoRelNoopener: [
-        {href: 'javascript:void(0)'},
-        {href: 'JAVASCRIPT:void(0)'},
-      ],
-      URL: {finalUrl: URL},
-    });
-    assert.equal(auditResult.rawValue, true);
-    assert.equal(auditResult.details.items.length, 0);
-  });
-
-  it('does not fail for links with mailto in href attribute', () => {
-    const auditResult = ExternalAnchorsAudit.audit({
-      AnchorsWithNoRelNoopener: [
-        {href: 'mailto:inbox@email.com'},
-        {href: 'MAILTO:INBOX@EMAIL.COM'},
+        {href: 'http://'},
+        {href: 'http:'},
+        {href: 'https://'},
+        {href: 'https:'},
       ],
       URL: {finalUrl: URL},
     });
-    assert.equal(auditResult.rawValue, true);
-    assert.equal(auditResult.details.items.length, 0);
+    assert.equal(auditResult.rawValue, false);
+    assert.equal(auditResult.details.items.length, 4);
+    assert.equal(auditResult.details.items.length, 4);
+    assert.ok(auditResult.debugString, 'includes debugString');
   });
 });

From 5031179e0dfdb0c354949df2ee5b981e51c9da07 Mon Sep 17 00:00:00 2001
From: sanjsanj <sanjsanj@hotmail.com>
Date: Tue, 12 Dec 2017 21:12:23 +0000
Subject: [PATCH 4/4] update the number of expected failing tests for
 rel="noopener" in the dbw smoketest

---
 .../test/smokehouse/dobetterweb/dbw-expectations.js           | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js b/lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js
index 29afcc778c3c..fdab95f1538d 100644
--- a/lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js
+++ b/lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js
@@ -53,12 +53,12 @@ module.exports = [
                      'If they are not used as hyperlinks, consider removing the _blank target.',
         extendedInfo: {
           value: {
-            length: 2,
+            length: 3,
           },
         },
         details: {
           items: {
-            length: 2,
+            length: 3,
           },
         },
       },