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.

Sign up for GitHub

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(label-content-name-mismatch): ignore non widget aria role(s) & do not use deprecated lookupTable.rolesOfType #2022

Merged
merged 6 commits into from
Feb 20, 2020
Merged

fix(label-content-name-mismatch): ignore non widget aria role(s) & do not use deprecated lookupTable.rolesOfType #2022

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
de35d77
fix: ignore non-widget roles for rule label-content-name-mismatch
jeeyyy Feb 5, 2020
34ea8b9
update helper fn for getting role type
jeeyyy Feb 5, 2020
decca9d
update aria-query
jeeyyy Feb 6, 2020
2aaafcc
rollback using aria-query
jeeyyy Feb 13, 2020
f17ff16
updates
jeeyyy Feb 17, 2020
4f612ef
update
jeeyyy Feb 18, 2020
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
44 changes: 1 addition & 43 deletions lib/commons/aria/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1995,7 +1995,7 @@ lookupTable.role = {
allowedElements: ['ol', 'ul']
},
tooltip: {
type: 'widget',
type: 'structure',
attributes: {
allowed: ['aria-expanded', 'aria-errormessage']
},
Expand Down Expand Up @@ -2385,45 +2385,3 @@ lookupTable.evaluateRoleForElement = {
return out;
}
};

/**
* Reference -> https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques#Widget_roles
* The current lookupTable.role['widget'] widget, yeilds
* ->
* [
* "alert", "alertdialog", "button", "checkbox", "dialog", "gridcell", "link", "log", "marquee", "menuitem", "menuitemcheckbox",
* "menuitemradio", "option", "progressbar", "radio", "scrollbar", "searchbox", "slider", "spinbutton", "status", "switch", "tab", "tabpanel",
* "textbox", "timer", "tooltip", "treeitem"
* ]
* There are some differences against specs, hence the below listing was made
*/
lookupTable.rolesOfType = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taking this out could break custom rules. Please leave it in, label it as deprecated, and make sure to include the word "deprecated" in the title of the PR so it appears in the changelog.

widget: [
'button',
'checkbox',
'dialog',
'gridcell',
'link',
'log',
'marquee',
'menuitem',
'menuitemcheckbox',
'menuitemradio',
'option',
'progressbar',
'radio',
'scrollbar',
'searchbox',
'slider',
'spinbutton',
'status',
'switch',
'tab',
'tabpanel',
'textbox',
'timer',
'tooltip',
'tree',
'treeitem'
]
};
5 changes: 4 additions & 1 deletion lib/rules/label-content-name-mismatch-matches.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ if (!role) {
return false;
}

const isWidgetType = aria.lookupTable.rolesOfType.widget.includes(role);
const widgetRoles = Object.keys(aria.lookupTable.role).filter(
key => aria.lookupTable.role[key].type === `widget`
);
const isWidgetType = widgetRoles.includes(role);
if (!isWidgetType) {
return false;
}
Expand Down
5 changes: 4 additions & 1 deletion test/integration/rules/label-content-name-mismatch/label-content-name-mismatch.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
<button id="pass7" name="link" aria-label="Next Page in the list">
Next Page
</button>

<!-- Fail -->
<div id="fail1" role="link" aria-label="OK">Next</div>
<button id="fail2" name="link" aria-label="the full">The full label</button>
Expand All @@ -38,4 +39,6 @@
<!-- inapplicable -->
<a id="inapplicable1" aria-label="OK">Next</a>
<input id="inapplicable2" type="email" aria-label="E-mail" value="Contact" />
<div id="inapplicable3" role="tooltip" aria-label="OK"></div>
<div id="inapplicable3" role="tooltip" aria-label="OK">Ok</div>
<div id="inapplicable4" role="feed" aria-label="RSS Feed">RSS Feed</div>
<div id="inapplicable5" role="marquee" aria-label="Foo">Foo Restaurant</div>
4 changes: 2 additions & 2 deletions test/rule-matches/label-content-name-mismatch-matches.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,12 @@ describe('label-content-name-mismatch-matches tests', function() {
assert.isFalse(actual);
});

it('returns true for widget role that does support name from content', function() {
it('returns false for non-widget role that does support name from content', function() {
var vNode = queryFixture(
'<div id="target" role="tooltip" aria-label="OK">Next</div>'
);
var actual = rule.matches(vNode.actualNode, vNode);
assert.isTrue(actual);
assert.isFalse(actual);
});

it('returns false for empty text content', function() {
Expand Down