Skip to content

fix(anchors-with-no-rel): ignore same origin links#2749

Merged
brendankenny merged 2 commits intomasterfrom
ignore_same_origin_links
Jul 26, 2017
Merged

fix(anchors-with-no-rel): ignore same origin links#2749
brendankenny merged 2 commits intomasterfrom
ignore_same_origin_links

Conversation

@patrickhulce
Copy link
Collaborator

@patrickhulce patrickhulce commented Jul 26, 2017

fixes #2743

we actually already had the smoketest to catch this but it was updated without catching it in review last time, also fixes a rather humorous smokehouse bug where we said difference at undefined, found undefined but expected undefined

.filter(anchor => {
try {
return anchor.href === '' || new URL(anchor.href).host !== pageHost;
return new URL(anchor.href).host !== pageHost;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this check was redundant given the try catch handling

Copy link
Contributor

@brendankenny brendankenny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🔗 👽 ⚖️


if (!(key in actual)) {
return {keyPath, undefined, expectedValue};
return {path: keyPath, actual: undefined, expected: expectedValue};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoops, well that won't work.

({undefined} alone seems like a ridiculous thing that should be caught instead of giving {undefined: undefined} :)

@brendankenny brendankenny merged commit 78ec647 into master Jul 26, 2017
@brendankenny brendankenny deleted the ignore_same_origin_links branch July 26, 2017 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NoRelOpenerAudit: exclude same origin links (regression)

2 participants