From b661b02f8199d1fa94309c5508fa512f6ce2169c Mon Sep 17 00:00:00 2001 From: Karan Thakkar Date: Mon, 16 Oct 2017 15:42:33 +0530 Subject: [PATCH 1/6] core(audit): ignore href=javascript:.* for rel=noopener audit ISSUES CLOSED: #3079 --- .../dobetterweb/external-anchors-use-rel-noopener.js | 4 ++++ .../external-anchors-use-rel-noopener-test.js | 11 +++++++++++ 2 files changed, 15 insertions(+) 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 a7eb425f680a..4ff3d85f38dd 100644 --- a/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js +++ b/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js @@ -47,6 +47,10 @@ class ExternalAnchorsUseRelNoopenerAudit extends Audit { return true; } }) + .filter(anchor => { + // Ignore href's that do not redirect to a new url + return !/javascript:.*/.test(anchor.href); + }) .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 a1b0237c500a..53fb7805582a 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 @@ -54,4 +54,15 @@ describe('External anchors use rel="noopener"', () => { assert.equal(auditResult.details.items.length, 3); assert.ok(auditResult.debugString, 'includes debugString'); }); + + it('does not fail for links with javascript in href attribute', () => { + const auditResult = ExternalAnchorsAudit.audit({ + AnchorsWithNoRelNoopener: [ + {href: 'javascript:void(0)'}, + ], + URL: {finalUrl: URL}, + }); + assert.equal(auditResult.rawValue, true); + assert.equal(auditResult.details.items.length, 0); + }); }); From 7df8c3471733b515344fca96924f63ebe533cd53 Mon Sep 17 00:00:00 2001 From: Karan Thakkar Date: Mon, 16 Oct 2017 22:34:14 +0530 Subject: [PATCH 2/6] Fix regex for javascript check in href --- .../audits/dobetterweb/external-anchors-use-rel-noopener.js | 2 +- 1 file changed, 1 insertion(+), 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 4ff3d85f38dd..846fad36d45a 100644 --- a/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js +++ b/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js @@ -49,7 +49,7 @@ class ExternalAnchorsUseRelNoopenerAudit extends Audit { }) .filter(anchor => { // Ignore href's that do not redirect to a new url - return !/javascript:.*/.test(anchor.href); + return !/^javascript:/.test(anchor.href); }) .map(anchor => { return { From dd8e472a17b25b501ce7e9f33c6f49199f8df1e3 Mon Sep 17 00:00:00 2001 From: Karan Thakkar Date: Tue, 17 Oct 2017 13:57:19 +0530 Subject: [PATCH 3/6] Use .startsWith instead of regex --- .../audits/dobetterweb/external-anchors-use-rel-noopener.js | 2 +- 1 file changed, 1 insertion(+), 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 846fad36d45a..14353f386923 100644 --- a/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js +++ b/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js @@ -49,7 +49,7 @@ class ExternalAnchorsUseRelNoopenerAudit extends Audit { }) .filter(anchor => { // Ignore href's that do not redirect to a new url - return !/^javascript:/.test(anchor.href); + return !anchor.href.startsWith('javascript:'); }) .map(anchor => { return { From 754e8322dc1518bfc9cde5138b54d52200e9d372 Mon Sep 17 00:00:00 2001 From: Karan Thakkar Date: Tue, 17 Oct 2017 13:58:37 +0530 Subject: [PATCH 4/6] Update comment for href filtering --- .../audits/dobetterweb/external-anchors-use-rel-noopener.js | 2 +- 1 file changed, 1 insertion(+), 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 14353f386923..d4c1cff27f4f 100644 --- a/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js +++ b/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js @@ -48,7 +48,7 @@ class ExternalAnchorsUseRelNoopenerAudit extends Audit { } }) .filter(anchor => { - // Ignore href's that do not redirect to a new url + // Ignore href's that are not real links return !anchor.href.startsWith('javascript:'); }) .map(anchor => { From 34fbcaaa818b17b37251216362b80bf2557b85d7 Mon Sep 17 00:00:00 2001 From: Karan Thakkar Date: Thu, 26 Oct 2017 13:16:14 +0530 Subject: [PATCH 5/6] Add case sensitive check for href --- .../audits/dobetterweb/external-anchors-use-rel-noopener.js | 6 +++++- .../dobetterweb/external-anchors-use-rel-noopener-test.js | 1 + 2 files changed, 6 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 d4c1cff27f4f..63d85f977caa 100644 --- a/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js +++ b/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js @@ -49,7 +49,11 @@ class ExternalAnchorsUseRelNoopenerAudit extends Audit { }) .filter(anchor => { // Ignore href's that are not real links - return !anchor.href.startsWith('javascript:'); + return ( + anchor.href + ? !anchor.href.toLowerCase().startsWith('javascript:') + : true + ); }) .map(anchor => { return { 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 53fb7805582a..c89aab41e7c9 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 @@ -59,6 +59,7 @@ describe('External anchors use rel="noopener"', () => { const auditResult = ExternalAnchorsAudit.audit({ AnchorsWithNoRelNoopener: [ {href: 'javascript:void(0)'}, + {href: 'JAVASCRIPT:void(0)'}, ], URL: {finalUrl: URL}, }); From ebed3808ab4cfd398b59c7b41bb26e04d5695073 Mon Sep 17 00:00:00 2001 From: Karan Thakkar Date: Mon, 30 Oct 2017 11:09:47 +0530 Subject: [PATCH 6/6] Refactor filter condition --- .../audits/dobetterweb/external-anchors-use-rel-noopener.js | 6 +----- 1 file changed, 1 insertion(+), 5 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 63d85f977caa..c2ede2f2e069 100644 --- a/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js +++ b/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js @@ -49,11 +49,7 @@ class ExternalAnchorsUseRelNoopenerAudit extends Audit { }) .filter(anchor => { // Ignore href's that are not real links - return ( - anchor.href - ? !anchor.href.toLowerCase().startsWith('javascript:') - : true - ); + return !anchor.href || !anchor.href.toLowerCase().startsWith('javascript:'); }) .map(anchor => { return {