fix: report all indeterminable anchors #1358
Conversation
Changes rel noopener audit to warn about all potentially violating anchor tags even when URL is malformed. Addresses #1345
| return new URL(anchor.href).host !== pageHost; | ||
| } catch (err) { | ||
| debugString = 'Lighthouse was unable to determine the destination ' + | ||
| 'of some anchor tags. Remove the href attribute if they are not ' + |
There was a problem hiding this comment.
One of the things Brendan discovered in the last PR for this audit was that many SPAs don't include an href attr and use click handlers instead. You catch will include those cases, but maybe we should modify the debug string to say so. E.g. the missing href is intentional in their app, but we're still saying remove it.
There was a problem hiding this comment.
IOW might want to to still one off the href === "" case
There was a problem hiding this comment.
ah I didn't realize anchor.href === '' would be true even if it didn't have an href at all. This seems like an ambiguous case then. There'd be a false positive most of the time if we warn for all missing hrefs but it's feasible they still send externally.
There was a problem hiding this comment.
True, SG then I'll add it back in.
|
Yea. The underlying js property always gets set. Crazy HTML.
We'll still only warn if the a also has target="_blank" so it may not be
all that noisy.
…On Thu, Dec 29, 2016, 3:25 PM Patrick Hulce ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js
<#1358>:
> const failingAnchors = artifacts.AnchorsWithNoRelNoopener.usages
- .filter(anchor => anchor.href === '' || new URL(anchor.href).host !== pageHost)
+ .filter(anchor => {
+ try {
+ return new URL(anchor.href).host !== pageHost;
+ } catch (err) {
+ debugString = 'Lighthouse was unable to determine the destination ' +
+ 'of some anchor tags. Remove the href attribute if they are not ' +
ah I didn't realize anchor.href === '' would be true even if it didn't
have an href at all. This seems like an ambiguous case then. There'd be a
false positive most of the time if we warn for all missing hrefs but it's
feasible they still send externally.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1358>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAOigAEyzjaT1z5SWafEN_8gJwPPrijMks5rNCVXgaJpZM4LXwFl>
.
|
* fix: report all indeterminable anchors Changes rel noopener audit to warn about all potentially violating anchor tags even when URL is malformed. Addresses GoogleChrome#1345 * feedback
R: @ebidel
Changes rel noopener audit to warn about all potentially violating anchor tags even when URL is malformed.
Addresses #1345