-
Notifications
You must be signed in to change notification settings - Fork 793
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
feat(new-rule): aria-braille-equivalent finds incorrect uses of aria-braille attributes #4107
Changes from 1 commit
d44c82c
8ae0e6f
d5a68b4
c839ea6
f4c7937
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
…braille attributes
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
import { accessibleTextVirtual } from '../../commons/text'; | ||
|
||
/** | ||
* Check that if aria-braillelabel is not empty, the element has an accessible text | ||
* @memberof checks | ||
* @return {Boolean} | ||
*/ | ||
export default function brailleLabelEquivalentEvaluate( | ||
node, | ||
options, | ||
virtualNode | ||
) { | ||
const brailleLabel = virtualNode.attr('aria-braillelabel') ?? ''; | ||
if (!brailleLabel.trim()) { | ||
return true; | ||
} | ||
return accessibleTextVirtual(virtualNode) !== ''; | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,11 @@ | ||||||
{ | ||||||
"id": "braille-label-equivalent", | ||||||
"evaluate": "braille-label-equivalent-evaluate", | ||||||
"metadata": { | ||||||
"impact": "serious", | ||||||
"messages": { | ||||||
"pass": "aria-braillelabel is not used on an element with no accessible text", | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just simplifying the double negative.
Suggested change
|
||||||
"fail": "aria-braillelabel is used on an element with no accessible text" | ||||||
} | ||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
/** | ||
* Check that if aria-brailleroledescription is not empty, | ||
* the element has a non-empty aria-roledescription | ||
* @memberof checks | ||
* @return {Boolean} | ||
*/ | ||
export default function brailleRoleDescriptionEquivalentEvaluate( | ||
node, | ||
options, | ||
virtualNode | ||
) { | ||
const brailleRoleDesc = virtualNode.attr('aria-brailleroledescription') ?? ''; | ||
if (!brailleRoleDesc.trim()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto for |
||
return true; | ||
} | ||
const roleDesc = virtualNode.attr('aria-roledescription'); | ||
if (typeof roleDesc !== 'string') { | ||
this.data({ messageKey: 'noRoleDescription' }); | ||
return false; | ||
} | ||
|
||
if (roleDesc.trim() === '') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should keep consistent use of if you're using |
||
this.data({ messageKey: 'emptyRoleDescription' }); | ||
return false; | ||
} | ||
return true; | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,14 @@ | ||||||
{ | ||||||
"id": "braille-roledescription-equivalent", | ||||||
"evaluate": "braille-roledescription-equivalent-evaluate", | ||||||
"metadata": { | ||||||
"impact": "serious", | ||||||
"messages": { | ||||||
"pass": "aria-brailleroledescription is not used on an element with no accessible text", | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
"fail": { | ||||||
"noRoleDescription": "aria-brailleroledescription is used on an element with no aria-roledescription", | ||||||
"emptyRoleDescription": "aria-brailleroledescription is used on an element with an empty aria-roledescription" | ||||||
} | ||||||
} | ||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
{ | ||
"id": "aria-braille-equivalent", | ||
"selector": "[aria-brailleroledescription], [aria-braillelabel]", | ||
"tags": ["cat.aria", "wcag2a", "wcag412", "EN-301-549", "EN-9.4.1.2"], | ||
"metadata": { | ||
"description": "Ensure aria-braillelabel and aria-brailleroledescription have a non-braille equivalent", | ||
"help": "aria-braille attributes must have a non-braille equivalent" | ||
}, | ||
"all": ["braille-roledescription-equivalent", "braille-label-equivalent"], | ||
"any": [], | ||
"none": [] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
describe('braille-label-equivalent tests', () => { | ||
const { checkSetup, getCheckEvaluate } = axe.testUtils; | ||
const checkContext = axe.testUtils.MockCheckContext(); | ||
const checkEvaluate = getCheckEvaluate('braille-label-equivalent'); | ||
|
||
afterEach(() => { | ||
checkContext.reset(); | ||
}); | ||
|
||
it('returns true without aria-braillelabel', () => { | ||
const params = checkSetup('<img id="target" alt="" />'); | ||
assert.isTrue(checkEvaluate.apply(checkContext, params)); | ||
}); | ||
|
||
it('returns true when aria-braillelabel is empty', () => { | ||
const params = checkSetup( | ||
'<img id="target" alt="" aria-braillelabel="" />' | ||
); | ||
assert.isTrue(checkEvaluate.apply(checkContext, params)); | ||
}); | ||
|
||
it('returns true when aria-braillelabel is whitespace-only', () => { | ||
const params = checkSetup( | ||
'<img id="target" alt="" aria-braillelabel=" \r\t\n " />' | ||
); | ||
assert.isTrue(checkEvaluate.apply(checkContext, params)); | ||
}); | ||
|
||
describe('when aria-braillelabel has text', () => { | ||
it('returns false when the accessible name is empty', () => { | ||
const params = checkSetup(` | ||
<img id="target" alt="" aria-braillelabel="foo" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This brings up an interesting question. This element is essentially There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should fail. There's no reason someone would put |
||
`); | ||
assert.isFalse(checkEvaluate.apply(checkContext, params)); | ||
}); | ||
|
||
it('returns false when the accessible name has only whitespace', () => { | ||
const params = checkSetup(` | ||
<img id="target" alt=" \r\t\n " aria-braillelabel="foo" /> | ||
`); | ||
assert.isFalse(checkEvaluate.apply(checkContext, params)); | ||
}); | ||
|
||
it('returns true when the accessible name is not empty', () => { | ||
const params = checkSetup(` | ||
<img id="target" alt="foo" aria-braillelabel="foo" /> | ||
`); | ||
assert.isTrue(checkEvaluate.apply(checkContext, params)); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
describe('braille-roledescription-equivalent tests', () => { | ||
const { checkSetup, getCheckEvaluate } = axe.testUtils; | ||
const checkContext = axe.testUtils.MockCheckContext(); | ||
const checkEvaluate = getCheckEvaluate('braille-roledescription-equivalent'); | ||
|
||
afterEach(() => { | ||
checkContext.reset(); | ||
}); | ||
|
||
it('returns true without aria-brailleroledescription', () => { | ||
const params = checkSetup('<div id="target"></div>'); | ||
assert.isTrue(checkEvaluate.apply(checkContext, params)); | ||
}); | ||
|
||
it('returns true when aria-brailleroledecription is empty', () => { | ||
const params = checkSetup( | ||
'<div id="target" aria-brailleroledescription=""></div>' | ||
); | ||
assert.isTrue(checkEvaluate.apply(checkContext, params)); | ||
}); | ||
|
||
it('returns true when aria-brailleroledecription is whitespace-only', () => { | ||
const params = checkSetup( | ||
'<div id="target" aria-brailleroledescription=" \r\t\n "></div>' | ||
); | ||
assert.isTrue(checkEvaluate.apply(checkContext, params)); | ||
}); | ||
|
||
describe('when aria-brailleroledescription has text', () => { | ||
it('returns false without aria-roledescription', () => { | ||
const params = checkSetup(` | ||
<div | ||
id="target" | ||
aria-brailleroledescription="foo" | ||
></div> | ||
`); | ||
assert.isFalse(checkEvaluate.apply(checkContext, params)); | ||
assert.deepEqual(checkContext._data, { messageKey: 'noRoleDescription' }); | ||
}); | ||
|
||
it('returns false when aria-roledescription is empty', () => { | ||
const params = checkSetup(` | ||
<div | ||
id="target" | ||
aria-roledescription="" | ||
aria-brailleroledescription="foo" | ||
></div> | ||
`); | ||
assert.isFalse(checkEvaluate.apply(checkContext, params)); | ||
assert.deepEqual(checkContext._data, { | ||
messageKey: 'emptyRoleDescription' | ||
}); | ||
}); | ||
|
||
it('returns false when aria-roledescription has only whitespace', () => { | ||
const params = checkSetup(` | ||
<div | ||
id="target" | ||
aria-roledescription=" \r\t\n " | ||
aria-brailleroledescription="foo" | ||
></div> | ||
`); | ||
assert.isFalse(checkEvaluate.apply(checkContext, params)); | ||
assert.deepEqual(checkContext._data, { | ||
messageKey: 'emptyRoleDescription' | ||
}); | ||
}); | ||
|
||
it('returns true when aria-roledescription is not empty', () => { | ||
const params = checkSetup(` | ||
<div | ||
id="target" | ||
aria-roledescription="foo" | ||
aria-brailleroledescription="foo" | ||
></div> | ||
`); | ||
assert.isTrue(checkEvaluate.apply(checkContext, params)); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
<button id="pass1" aria-braillelabel="hello">Hello</button> | ||
<button id="pass2" aria-braillelabel=""></button> | ||
<button id="fail1" aria-braillelabel="hello"></button> | ||
|
||
<aside | ||
id="pass3" | ||
aria-roledescription="table of contents" | ||
aria-brailleroledescription="" | ||
></aside> | ||
|
||
<aside | ||
id="pass4" | ||
aria-roledescription="table of contents" | ||
aria-brailleroledescription="table of contents" | ||
></aside> | ||
|
||
<aside | ||
id="pass5" | ||
aria-roledescription="" | ||
aria-brailleroledescription="" | ||
></aside> | ||
|
||
<aside | ||
id="fail2" | ||
aria-roledescription="" | ||
aria-brailleroledescription="table of contents" | ||
></aside> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
{ | ||
"description": "aria-braille-equivalent tests", | ||
"rule": "aria-braille-equivalent", | ||
"passes": [["#pass1"], ["#pass2"], ["#pass3"], ["#pass4"], ["#pass5"]], | ||
"violations": [["#fail1"], ["#fail2"]] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use
sanitize
from our text functions here instead (similar to aria-label and aria-labelledby checks)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably shouldn't use it for aria-labelledby either. What sanitize does that trim doesn't do is replace duplicate whitespace characters in between words with single space characters. We don't need that here.
Suppose it doesn't matter much though. And if we do ever need to do this in a way that's different from
trim
it helps to have them in place.