Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: allow placeholder anchor elements #15894

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
core: allow placeholder anchor elements
  • Loading branch information
romainmenke committed Mar 26, 2024
commit f0bf8f480b8a80340a624e118532b53d69c5b707
19 changes: 19 additions & 0 deletions core/audits/seo/crawlable-anchors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -45,6 +55,7 @@ class CrawlableAnchors extends Audit {
role = '',
id,
href,
attributeNames = [],
}) => {
rawHref = rawHref.replace( /\s/g, '');
name = name.trim();
Expand All @@ -62,6 +73,14 @@ 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;

if (href === '') return true;
if (javaScriptVoidRegExp.test(rawHref)) return true;

Expand Down
2 changes: 2 additions & 0 deletions core/gather/gatherers/anchor-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -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),
};
Expand All @@ -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),
};
Expand Down
26 changes: 22 additions & 4 deletions core/test/audits/seo/crawlable-anchors-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ function runAudit({
onclick = '',
name = '',
id = '',
attributeNames = [
(rawHref || href) ? 'href' : null, role ? 'role' : null, name ? 'name' : null,
].filter(Boolean),
listeners = onclick.trim().length ? [{type: 'click'}] : [],
node = {
snippet: '',
Expand All @@ -34,6 +37,7 @@ function runAudit({
role,
node,
id,
attributeNames,
}],
URL: {
finalDisplayedUrl: 'http://example.com',
Expand All @@ -56,12 +60,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', () => {
Expand All @@ -79,7 +84,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', () => {
Expand All @@ -103,9 +112,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);
Expand Down
1 change: 1 addition & 0 deletions types/artifacts.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,7 @@ declare module Artifacts {
node: NodeDetails
onclick: string
id: string
attributeNames: Array<string>
listeners?: Array<{
type: Crdp.DOMDebugger.EventListener['type']
}>
Expand Down