Skip to content

Commit

Permalink
fix(focus-order-semantics): allow role=tooltip to pass (#2871)
Browse files Browse the repository at this point in the history
  • Loading branch information
@straker
straker authored Apr 14, 2021
1 parent 2d33ca1 commit dc526d8
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 7 deletions.
16 changes: 11 additions & 5 deletions lib/checks/aria/valid-scrollable-semantics-evaluate.js
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { getExplicitRole } from '../../commons/aria';

/**
* A map from HTML tag names to a boolean which reflects whether it is
* appropriate for scrollable elements found in the focus order.
Expand Down Expand Up @@ -42,12 +44,16 @@ function validScrollableTagName(node) {
* @return {Boolean} Whether the node has a role appropriate for a scrollable
* region.
*/
function validScrollableRole(node) {
var role = node.getAttribute('role');
function validScrollableRole(node, options) {
var role = getExplicitRole(node);
if (!role) {
return false;
}
return VALID_ROLES_FOR_SCROLLABLE_REGIONS[role.toLowerCase()] || false;
return (
VALID_ROLES_FOR_SCROLLABLE_REGIONS[role] ||
options.roles.includes(role) ||
false
);
}

/**
Expand All @@ -57,8 +63,8 @@ function validScrollableRole(node) {
* @param {HTMLElement} node
* @return {Boolean} True if the elements role or tag name is a valid scrollable region. False otherwise.
*/
function validScrollableSemanticsEvaluate(node) {
return validScrollableRole(node) || validScrollableTagName(node);
function validScrollableSemanticsEvaluate(node, options) {
return validScrollableRole(node, options) || validScrollableTagName(node);
}

export default validScrollableSemanticsEvaluate;
4 changes: 3 additions & 1 deletion lib/checks/aria/valid-scrollable-semantics.json
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
{
"id": "valid-scrollable-semantics",
"evaluate": "valid-scrollable-semantics-evaluate",
"options": [],
"options": {
"roles": ["tooltip"]
},
"metadata": {
"impact": "minor",
"messages": {
Expand Down
39 changes: 39 additions & 0 deletions test/checks/aria/valid-scrollable-semantics.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ describe('valid-scrollable-semantics', function() {
'use strict';

var fixture = document.getElementById('fixture');
var flatTreeSetup = axe.testUtils.flatTreeSetup;
var checkContext = axe.testUtils.MockCheckContext();

afterEach(function() {
Expand All @@ -13,6 +14,7 @@ describe('valid-scrollable-semantics', function() {
var node = document.createElement('div');
node.setAttribute('role', '"banner');
fixture.appendChild(node);
flatTreeSetup(fixture);
assert.isFalse(
axe.testUtils
.getCheckEvaluate('valid-scrollable-semantics')
Expand All @@ -24,6 +26,7 @@ describe('valid-scrollable-semantics', function() {
var node = document.createElement('div');
node.setAttribute('role', 'search');
fixture.appendChild(node);
flatTreeSetup(fixture);
assert.isFalse(
axe.testUtils
.getCheckEvaluate('valid-scrollable-semantics')
Expand All @@ -35,6 +38,7 @@ describe('valid-scrollable-semantics', function() {
var node = document.createElement('div');
node.setAttribute('role', 'form');
fixture.appendChild(node);
flatTreeSetup(fixture);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('valid-scrollable-semantics')
Expand All @@ -46,6 +50,7 @@ describe('valid-scrollable-semantics', function() {
var node = document.createElement('div');
node.setAttribute('role', 'navigation');
fixture.appendChild(node);
flatTreeSetup(fixture);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('valid-scrollable-semantics')
Expand All @@ -57,6 +62,7 @@ describe('valid-scrollable-semantics', function() {
var node = document.createElement('div');
node.setAttribute('role', 'complementary');
fixture.appendChild(node);
flatTreeSetup(fixture);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('valid-scrollable-semantics')
Expand All @@ -68,6 +74,7 @@ describe('valid-scrollable-semantics', function() {
var node = document.createElement('div');
node.setAttribute('role', 'contentinfo');
fixture.appendChild(node);
flatTreeSetup(fixture);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('valid-scrollable-semantics')
Expand All @@ -79,6 +86,7 @@ describe('valid-scrollable-semantics', function() {
var node = document.createElement('div');
node.setAttribute('role', 'main');
fixture.appendChild(node);
flatTreeSetup(fixture);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('valid-scrollable-semantics')
Expand All @@ -90,6 +98,7 @@ describe('valid-scrollable-semantics', function() {
var node = document.createElement('div');
node.setAttribute('role', 'region');
fixture.appendChild(node);
flatTreeSetup(fixture);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('valid-scrollable-semantics')
Expand All @@ -100,6 +109,7 @@ describe('valid-scrollable-semantics', function() {
it('should return true for nav elements', function() {
var node = document.createElement('nav');
fixture.appendChild(node);
flatTreeSetup(fixture);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('valid-scrollable-semantics')
Expand All @@ -110,6 +120,7 @@ describe('valid-scrollable-semantics', function() {
it('should return true for section elements', function() {
var node = document.createElement('section');
fixture.appendChild(node);
flatTreeSetup(fixture);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('valid-scrollable-semantics')
Expand All @@ -120,6 +131,7 @@ describe('valid-scrollable-semantics', function() {
it('should return true for article elements', function() {
var node = document.createElement('article');
fixture.appendChild(node);
flatTreeSetup(fixture);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('valid-scrollable-semantics')
Expand All @@ -130,10 +142,37 @@ describe('valid-scrollable-semantics', function() {
it('should return true for aside elements', function() {
var node = document.createElement('aside');
fixture.appendChild(node);
flatTreeSetup(fixture);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('valid-scrollable-semantics')
.call(checkContext, node)
);
});

it('should return true for role=tooltip', function() {
var node = document.createElement('div');
node.setAttribute('role', 'tooltip');
fixture.appendChild(node);
flatTreeSetup(fixture);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('valid-scrollable-semantics')
.call(checkContext, node)
);
});

describe('options', function() {
it('should allow options.roles to return true for role', function() {
var node = document.createElement('div');
node.setAttribute('role', 'banner');
fixture.appendChild(node);
flatTreeSetup(fixture);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('valid-scrollable-semantics')
.call(checkContext, node, { roles: ['banner'] })
);
});
});
});
1 change: 1 addition & 0 deletions test/integration/rules/focus-order-semantics/focus-order-semantics.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ <h4>Valid landmark roles for scrollable containers</h4>
<div id="pass6" role="main" tabindex="0"></div>
<div id="pass7" role="region" tabindex="0"></div>
<div id="pass8" role="application" tabindex="0"></div>
<div id="pass9" role="tooltip" tabindex="0"></div>
</div>
<h4>
Valid scrollable HTML tags for scrollable regions, not selected by this
Expand Down
3 changes: 2 additions & 1 deletion test/integration/rules/focus-order-semantics/focus-order-semantics.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
["#pass5"],
["#pass6"],
["#pass7"],
["#pass8"]
["#pass8"],
["#pass9"]
],
"violations": [
["#violation1"],
Expand Down

1 comment on commit dc526d8

@HeartbliT
Copy link

Choose a reason for hiding this comment

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

Suppressing notification from package by user request.
03-17 15:17:23.533 1438 1438 E NotificationService: Suppressing notification from package by user request.

03-17 15:17:27.555 1438 1438 E NotificationService: Suppressing notification from package by user request.
03-17 15:17:27.807 1438 28979 V AlarmManager: APP set(PendingIntent{8ba4bf3: PendingIntentRecord{42a69f3 com.android.chrome broadcastIntent}}) : type=1 triggerAtTime=1615990648289 win=-1 tElapsed=6655798 maxElapsed=6655798 interval=0 flags=0x0
03-17 15:17:27.808 1438 28979 D AlarmManager: remove(operation) changed bounds; rebatching operation = PendingIntent{8ba4bf3: PendingIntentRecord{42a69f3 com.android.chrome broadcastIntent}}
03-17 15:17:28.047 1438 1438 E NotificationService: Suppressing notification from me

Please sign in to comment.