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

fix(p-as-heading): p-as-heading rule to account for textContent length #3145

Merged
merged 8 commits into from
Oct 4, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/rule-descriptions.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ Rules we are still testing and developing. They are not enabled by default in ax
| [label-content-name-mismatch](https://dequeuniversity.com/rules/axe/4.3/label-content-name-mismatch?application=RuleDescription) | Ensures that elements labelled through their content must have their visible text as part of their accessible name | Serious | cat.semantics, wcag21a, wcag253, experimental | failure | [2ee8b8](https://act-rules.github.io/rules/2ee8b8) |
| [link-in-text-block](https://dequeuniversity.com/rules/axe/4.3/link-in-text-block?application=RuleDescription) | Links can be distinguished without relying on color | Serious | cat.color, experimental, wcag2a, wcag141 | failure, needs review | |
| [no-autoplay-audio](https://dequeuniversity.com/rules/axe/4.3/no-autoplay-audio?application=RuleDescription) | Ensures <video> or <audio> elements do not autoplay audio for more than 3 seconds without a control mechanism to stop or mute the audio | Moderate | cat.time-and-media, wcag2a, wcag142, experimental | failure, needs review | [80f0bf](https://act-rules.github.io/rules/80f0bf) |
| [p-as-heading](https://dequeuniversity.com/rules/axe/4.3/p-as-heading?application=RuleDescription) | Ensure p elements are not used to style headings | Serious | cat.semantics, wcag2a, wcag131, experimental | failure | |
| [p-as-heading](https://dequeuniversity.com/rules/axe/4.3/p-as-heading?application=RuleDescription) | Ensure p elements are not used to style headings | Serious | cat.semantics, wcag2a, wcag131, experimental | failure, needs review | |
| [table-fake-caption](https://dequeuniversity.com/rules/axe/4.3/table-fake-caption?application=RuleDescription) | Ensure that tables with a caption use the <caption> element. | Serious | cat.tables, experimental, wcag2a, wcag131, section508, section508.22.g | failure | |
| [td-has-header](https://dequeuniversity.com/rules/axe/4.3/td-has-header?application=RuleDescription) | Ensure that each non-empty data cell in a large table has one or more table headers | Critical | cat.tables, experimental, wcag2a, wcag131, section508, section508.22.g | failure | |

Expand Down
18 changes: 18 additions & 0 deletions lib/checks/navigation/p-as-heading-evaluate.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,24 @@ function pAsHeadingEvaluate(node, options, virtualNode) {
const nextStyle = nextSibling ? getStyleValues(nextSibling) : null;
const prevStyle = prevSibling ? getStyleValues(prevSibling) : null;

if (node && nextSibling) {
const optionsHeadingLength = options.lengthThreshold.headingLength || [];
const optionsParagraphLength =
options.lengthThreshold.paragraphLength || [];

const headingLength = node.textContent.trim().length * optionsHeadingLength;
const paragraphLength =
nextSibling.textContent.trim().length * optionsParagraphLength;

if (headingLength > paragraphLength) {
return true;
} else if (headingLength < paragraphLength / 2) {
return false;
} else if (headingLength < paragraphLength) {
return undefined;
}
Zidious marked this conversation as resolved.
Show resolved Hide resolved
}

if (!nextStyle || !isHeaderStyle(currStyle, nextStyle, margins)) {
return true;
}
Expand Down
9 changes: 7 additions & 2 deletions lib/checks/navigation/p-as-heading.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,18 @@
{
"size": 1.4
}
]
],
"lengthThreshold": {
"headingLength": ["1"],
"paragraphLength": ["1"]
}
},
"metadata": {
"impact": "serious",
"messages": {
"pass": "<p> elements are not styled as headings",
"fail": "Heading elements should be used instead of styled p elements"
"fail": "Heading elements should be used instead of styled <p> elements",
"incomplete": "Unable to determine if <p> elements are styled as headings"
}
}
}
82 changes: 82 additions & 0 deletions test/checks/navigation/p-as-heading.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,88 @@ describe('p-as-heading', function() {
);
});

it('returns true if the heading elm length is greater than the paragraph elm', function() {
Zidious marked this conversation as resolved.
Show resolved Hide resolved
var params = checkSetup(
'<p id="target">elm1elm1</p>' + '<p>elm2</p>',
Zidious marked this conversation as resolved.
Show resolved Hide resolved
testOptions
);
assert.isTrue(
axe.testUtils.getCheckEvaluate('p-as-heading').apply(checkContext, params)
);
});

it('returns false if the heading is 200% shorter than the paragraph ', function() {
var params = checkSetup(
'<p id="target">el1</p>' + '<p>elm2elm2</p>',
testOptions
);
assert.isFalse(
axe.testUtils.getCheckEvaluate('p-as-heading').apply(checkContext, params)
);
});

it('returns undefined if the heading is twice as long but not greater than the length of the pararaph ', function() {
var params = checkSetup(
'<p id="target">elel</p>' + '<p>elm2elm2</p>',
testOptions
);
assert.isUndefined(
axe.testUtils.getCheckEvaluate('p-as-heading').apply(checkContext, params)
);
});
describe('options.lengthThreshold', function() {
it('returns true when options.lengthThreshold.headingLength is greater than the paragraph', function() {
var options = {
lengthThreshold: {
headingLength: ['3']
}
};
var params = checkSetup(
'<p id="target">elm2</p>' + '<p>elm2elm2/p>',
options
);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('p-as-heading')
.apply(checkContext, params)
);
});

it('returns false when options.lengthThreshold.headingLength 200% shorter than the paragraph', function() {
var options = {
lengthThreshold: {
headingLength: ['2']
}
};
var params = checkSetup(
'<p id="target">el</p>' + '<p>elm2elm2elm2elm2</p>',
options
);
assert.isFalse(
axe.testUtils
.getCheckEvaluate('p-as-heading')
.apply(checkContext, params)
);
});

it('returns undefined when options.lengthThreshold.headingLength is twice as long but not greater than the paragraph', function() {
var options = {
lengthThreshold: {
headingLength: ['2']
}
};
var params = checkSetup(
'<p id="target">el</p>' + '<p>elm2elm2</p>',
options
);
assert.isUndefined(
axe.testUtils
.getCheckEvaluate('p-as-heading')
.apply(checkContext, params)
);
});
});

describe('option.margin', function() {
it('passes if no margins are set', function() {
var options = {};
Expand Down