diff --git a/core/audits/seo/crawlable-anchors.js b/core/audits/seo/crawlable-anchors.js index 156e55f1303f..5746bbfbfb38 100644 --- a/core/audits/seo/crawlable-anchors.js +++ b/core/audits/seo/crawlable-anchors.js @@ -18,6 +18,16 @@ const UIStrings = { columnFailingLink: 'Uncrawlable Link', }; +const hrefAssociatedAttributes = [ + 'target', + 'download', + 'ping', + 'rel', + 'hreflang', + 'type', + 'referrerpolicy', +]; + const str_ = i18n.createIcuMessageFn(import.meta.url, UIStrings); class CrawlableAnchors extends Audit { @@ -45,10 +55,14 @@ class CrawlableAnchors extends Audit { role = '', id, href, + attributeNames = [], + listeners = [], + ancestorListeners = [], }) => { rawHref = rawHref.replace( /\s/g, ''); name = name.trim(); role = role.trim(); + const hasListener = Boolean(listeners.length || ancestorListeners.length); if (role.length > 0) return; // Ignore mailto links even if they use one of the failing patterns. See https://github.com/GoogleChrome/lighthouse/issues/11443#issuecomment-694898412 @@ -62,6 +76,16 @@ class CrawlableAnchors extends Audit { if (rawHref.startsWith('file:')) return true; if (name.length > 0) return; + // https://html.spec.whatwg.org/multipage/text-level-semantics.html#the-a-element + // If the a element has no href attribute, then the element represents a placeholder for where a link might otherwise have been placed, if it had been relevant, consisting of just the element's contents. + // The target, download, ping, rel, hreflang, type, and referrerpolicy attributes must be omitted if the href attribute is not present. + if ( + !attributeNames.includes('href') && + !hrefAssociatedAttributes.some(attribute => attributeNames.includes(attribute)) + ) { + return hasListener; + } + if (href === '') return true; if (javaScriptVoidRegExp.test(rawHref)) return true; diff --git a/core/gather/gatherers/anchor-elements.js b/core/gather/gatherers/anchor-elements.js index 7ebb43c7de6b..93566eb5055b 100644 --- a/core/gather/gatherers/anchor-elements.js +++ b/core/gather/gatherers/anchor-elements.js @@ -54,6 +54,7 @@ function collectAnchorElements() { rel: node.rel, target: node.target, id: node.getAttribute('id') || '', + attributeNames: node.getAttributeNames(), // @ts-expect-error - getNodeDetails put into scope via stringification node: getNodeDetails(node), }; @@ -68,6 +69,7 @@ function collectAnchorElements() { rel: '', target: node.target.baseVal || '', id: node.getAttribute('id') || '', + attributeNames: node.getAttributeNames(), // @ts-expect-error - getNodeDetails put into scope via stringification node: getNodeDetails(node), }; @@ -119,9 +121,27 @@ class AnchorElements extends BaseGatherer { const anchorsWithEventListeners = anchors.map(async anchor => { const listeners = await getEventListeners(session, anchor.node.devtoolsNodePath); + /** @type {Set<{type: string}>} */ + const ancestorListeners = new Set(); + const splitPath = anchor.node.devtoolsNodePath.split(','); + const ancestorListenerPromises = []; + while (splitPath.length >= 2) { + splitPath.length -= 2; + const path = splitPath.join(','); + const promise = getEventListeners(session, path).then(listeners => { + for (const listener of listeners) { + ancestorListeners.add(listener); + } + }).catch(() => {}); + ancestorListenerPromises.push(promise); + } + + await Promise.all(ancestorListenerPromises); + return { ...anchor, listeners, + ancestorListeners: Array.from(ancestorListeners), }; }); diff --git a/core/test/audits/seo/crawlable-anchors-test.js b/core/test/audits/seo/crawlable-anchors-test.js index c79a11469ff6..9efa4c79fbb4 100644 --- a/core/test/audits/seo/crawlable-anchors-test.js +++ b/core/test/audits/seo/crawlable-anchors-test.js @@ -15,7 +15,11 @@ function runAudit({ onclick = '', name = '', id = '', + attributeNames = [ + (rawHref || href) ? 'href' : null, role ? 'role' : null, name ? 'name' : null, + ].filter(Boolean), listeners = onclick.trim().length ? [{type: 'click'}] : [], + ancestorListeners = [], node = { snippet: '', devtoolsNodePath: '', @@ -30,10 +34,12 @@ function runAudit({ href, name, listeners, + ancestorListeners, onclick, role, node, id, + attributeNames, }], URL: { finalDisplayedUrl: 'http://example.com', @@ -56,12 +62,13 @@ describe('SEO: Crawlable anchors audit', () => { assert.equal(runAudit({rawHref: '//example.com'}), 1, 'protocol relative link'); assert.equal(runAudit({rawHref: 'tel:5555555'}), 1, 'email link with a tel URI'); assert.equal(runAudit({rawHref: '#'}), 1, 'link with only a hash symbol'); + assert.equal(runAudit({}), 1, 'placeholder anchor element'); assert.equal(runAudit({ rawHref: '?query=string', }), 1, 'relative link which specifies a query string'); assert.equal(runAudit({rawHref: 'ftp://'}), 0, 'invalid FTP links fails'); - assert.equal(runAudit({rawHref: '', href: 'https://example.com'}), 1, 'empty attribute that links to current page'); + assert.equal(runAudit({rawHref: '', href: 'https://example.com', attributeNames: ['href']}), 1, 'empty attribute that links to current page'); }); it('allows anchors acting as an ID anchor', () => { @@ -79,7 +86,11 @@ describe('SEO: Crawlable anchors audit', () => { }); assert.equal(auditResult, 1, 'Href value has no effect when a role is present'); assert.equal(runAudit({role: 'a'}), 1, 'Using a role attribute value is an immediate pass'); - assert.equal(runAudit({role: ' '}), 0, 'A role value of a space character fails the audit'); + assert.equal( + runAudit({role: ' ', attributeNames: ['rel']}), + 0, + 'A role value of a space character fails the audit' + ); }); it('handles anchor elements which use event listeners', () => { @@ -96,6 +107,17 @@ describe('SEO: Crawlable anchors audit', () => { }); assert.equal(auditResultWithInvalidHref, 0, 'invalid href with any event listener is a faile'); + const auditResultWithNoHref = runAudit({ + listeners: [{type: 'nope'}, {type: 'another'}, {type: 'click'}], + }); + assert.equal(auditResultWithNoHref, 0, 'no href with any event listener is a fail'); + + const auditResultWithParentListenerNoHref = runAudit({ + ancestorListeners: [{type: 'nope'}, {type: 'another'}, {type: 'click'}], + }); + assert.equal(auditResultWithParentListenerNoHref, 0, + 'no href with any event listener on a parent is a fail'); + const auditResultNoListener = runAudit({ rawHref: '/validPath', }); @@ -103,9 +125,18 @@ describe('SEO: Crawlable anchors audit', () => { }); it('disallows uncrawlable anchors', () => { - assert.equal(runAudit({}), 0, 'link with no meaningful attributes and no event handlers'); + assert.equal( + runAudit({attributeNames: ['href']}), + 0, + 'link with an empty href and no other meaningful attributes and no event handlers' + ); + assert.equal(runAudit({attributeNames: ['target']}), 0, 'link with only a target attribute'); assert.equal(runAudit({rawHref: 'file:///image.png'}), 0, 'hyperlink with a `file:` URI'); - assert.equal(runAudit({name: ' '}), 0, 'name attribute with only space characters'); + assert.equal( + runAudit({name: ' ', attributeNames: ['rel']}), + 0, + 'name attribute with only space characters' + ); assert.equal(runAudit({rawHref: ' '}), 1, 'href attribute with only space characters'); const assertionMessage = 'onclick attribute with only space characters'; assert.equal(runAudit({rawHref: ' ', onclick: ' '}), 1, assertionMessage); diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts index 19ae925055fa..d8cce226b607 100644 --- a/types/artifacts.d.ts +++ b/types/artifacts.d.ts @@ -365,9 +365,13 @@ declare module Artifacts { node: NodeDetails onclick: string id: string + attributeNames: Array listeners?: Array<{ type: Crdp.DOMDebugger.EventListener['type'] }> + ancestorListeners?: Array<{ + type: Crdp.DOMDebugger.EventListener['type'] + }> } type BFCacheReasonMap = {