Skip to content

Commit

Permalink
chore: avoid benign call-signature-mismatch in axe.commons.text.acces…
Browse files Browse the repository at this point in the history
…sibleText usage (#4332)

#### Details

Our `AxeUtils.getAccessibleText` accepted a `isLabelledByContext` boolean parameter which we always pass as `false` in our usage. We pass this as-is to `axe-core`'s `axe.commons.text.accessibleText`.

Unfortunately, `axe-core` changed the signature of `accessibleText` [two years ago](dequelabs/axe-core#1163) to use an options object rather than a boolean second parameter, and our tests never caught it. Thankfully, by complete luck, passing `false` happens to be interpreted the same as an object which includes the property as false, and that's the only way we use it (because you'd get the `false` behavior if we were to ever pass `true`!)

This PR removes our support for the options entirely, since we don't use it, and adds an integration test similar to other `AxeUtils` cases that would have caught the issue earlier.

##### Motivation

Fix broken usage of axe-core

##### Context

n/a

#### Pull request checklist
<!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox -->
- [x] Addresses an existing issue: #0000
- [x] Ran `yarn fastpass`
- [x] Added/updated relevant unit test(s) (and ran `yarn test`)
- [x] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage`
- [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`.
- [n/a] (UI changes only) Added screenshots/GIFs to description above
- [n/a] (UI changes only) Verified usability with NVDA/JAWS
  • Loading branch information
dbjorge committed Jun 5, 2021
1 parent 8380b72 commit bad797e
Show file tree
Hide file tree
Showing 15 changed files with 35 additions and 39 deletions.
6 changes: 3 additions & 3 deletions src/scanner/axe-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ export function getOptionsFromCheck(checkId: string): any {
return axe._audit.checks[checkId].options;
}

export function getAccessibleText(node: HTMLElement, isLabelledByContext: boolean): string {
return axe.commons.text.accessibleText(node, isLabelledByContext);
export function getAccessibleText(node: HTMLElement): string {
return axe.commons.text.accessibleText(node);
}

export function getAccessibleDescription(node: HTMLElement): string {
Expand Down Expand Up @@ -87,7 +87,7 @@ export function getImageCodedAs(node: HTMLElement): ImageCodedAs | null {
return 'Decorative';
}

if (getAccessibleText(node, false) !== '' || isWhiteSpace(alt)) {
if (getAccessibleText(node) !== '' || isWhiteSpace(alt)) {
return 'Meaningful';
}

Expand Down
2 changes: 1 addition & 1 deletion src/scanner/custom-rules/cues-rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export function evaluateCues(node: HTMLElement): boolean {
// tslint:disable-next-line:no-invalid-this
this.data({
element: getNativeWidgetElementType(node),
accessibleName: AxeUtils.getAccessibleText(node, false),
accessibleName: AxeUtils.getAccessibleText(node),
htmlCues: generateHTMLCuesDictionary(node),
ariaCues: generateARIACuesDictionary(node),
});
Expand Down
2 changes: 1 addition & 1 deletion src/scanner/custom-rules/custom-widget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ function createSelector(): string {
}

function evaluateCustomWidget(node: HTMLElement): boolean {
const accessibleName = AxeUtils.getAccessibleText(node, false);
const accessibleName = AxeUtils.getAccessibleText(node);
const role = node.getAttribute('role');
const describedBy = AxeUtils.getAccessibleDescription(node);
const htmlCues = generateHTMLCuesDictionary(node);
Expand Down
2 changes: 1 addition & 1 deletion src/scanner/custom-rules/image-rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export function isImage(node: HTMLElement): boolean {
}

function evaluateImageFunction(node: HTMLElement): boolean {
const accessibleName: string = AxeUtils.getAccessibleText(node, false);
const accessibleName: string = AxeUtils.getAccessibleText(node);
const codedAs: string | null = AxeUtils.getImageCodedAs(node);
const imageType: string | null = AxeUtils.getImageType(node);
const role: string | null = node.getAttribute('role');
Expand Down
2 changes: 1 addition & 1 deletion src/scanner/custom-rules/link-function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function evaluateLinkFunction(
virtualNode: any,
context: any,
): boolean {
const accessibleName = AxeUtils.getAccessibleText(node, false);
const accessibleName = AxeUtils.getAccessibleText(node);
const ariaValues = AxeUtils.getPropertyValuesMatching(node, /^aria-/);
const role = node.getAttribute('role');
const tabIndex = node.getAttribute('tabindex');
Expand Down
2 changes: 1 addition & 1 deletion src/scanner/custom-rules/link-purpose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ function evaluateLinkPurpose(
virtualNode: any,
context: any,
): boolean {
const accessibleName: string = AxeUtils.getAccessibleText(node, false);
const accessibleName: string = AxeUtils.getAccessibleText(node);
const accessibleDescription: string = AxeUtils.getAccessibleDescription(node);
const url = node.getAttribute('href');

Expand Down
2 changes: 1 addition & 1 deletion src/scanner/custom-rules/native-widgets-default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export function evaluateNativeWidget(node: HTMLElement): boolean {
// tslint:disable-next-line:no-invalid-this
this.data({
element: getNativeWidgetElementType(node),
accessibleName: AxeUtils.getAccessibleText(node, false),
accessibleName: AxeUtils.getAccessibleText(node),
accessibleDescription: AxeUtils.getAccessibleDescription(node),
});
return true;
Expand Down
2 changes: 1 addition & 1 deletion src/scanner/custom-rules/text-alternative.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ function matches(node: HTMLElement): boolean {
}

function evaluateTextAlternative(node: HTMLElement): boolean {
const accessibleName: string = AxeUtils.getAccessibleText(node, false);
const accessibleName: string = AxeUtils.getAccessibleText(node);
const accessibleDescription: string = AxeUtils.getAccessibleDescription(node);
const imageType: string | null = AxeUtils.getImageType(node);
const role: string | null = node.getAttribute('role');
Expand Down
2 changes: 1 addition & 1 deletion src/scanner/custom-rules/widget-function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export function evaluateWidgetFunction(node: HTMLElement): boolean {
// tslint:disable-next-line:no-invalid-this
this.data({
element: getNativeWidgetElementType(node),
accessibleName: AxeUtils.getAccessibleText(node, false),
accessibleName: AxeUtils.getAccessibleText(node),
role: node.getAttribute('role'),
ariaAttributes: AxeUtils.getAttributes(node, ariaAttributes),
tabIndex: node.getAttribute('tabindex'),
Expand Down
38 changes: 19 additions & 19 deletions src/tests/unit/tests/scanner/axe-utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import * as AxeUtils from 'scanner/axe-utils';
import { withAxeCommonsMocked } from 'tests/unit/tests/scanner/mock-axe-utils';
import { GlobalMock, GlobalScope, IGlobalMock, It, Mock, MockBehavior, Times } from 'typemoq';
import { GlobalMock, GlobalScope, IGlobalMock, It, MockBehavior } from 'typemoq';

describe('AxeUtils', () => {
describe('getMatchesFromRule', () => {
Expand Down Expand Up @@ -44,23 +43,24 @@ describe('AxeUtils', () => {
});

describe('getAccessibleText', () => {
it.each([true, false])(
'calls axe.commons.text.accessibleText with given node & isLabelledByContext %p',
isLabelledByContext => {
const accessibleTextMock = Mock.ofInstance(
(node, context) => '',
MockBehavior.Strict,
);
const elementStub = {} as HTMLElement;
accessibleTextMock
.setup(m => m(It.isValue(elementStub), isLabelledByContext))
.verifiable(Times.once());
withAxeCommonsMocked('text', { accessibleText: accessibleTextMock.object }, () => {
AxeUtils.getAccessibleText(elementStub, isLabelledByContext);
});
accessibleTextMock.verifyAll();
},
);
let fixture: HTMLElement;

beforeEach(() => {
fixture = createTestFixture('test-fixture', '');
});

afterEach(() => {
document.body.querySelector('#test-fixture').remove();
});

it("successfully identifies an img element's alt text as accessible text", () => {
fixture.innerHTML = `<img id="test-subject" alt="alt value"/>`;
const testSubject = fixture.querySelector(`#test-subject`) as HTMLElement;

const result = AxeUtils.withAxeSetup(() => AxeUtils.getAccessibleText(testSubject));

expect(result).toEqual('alt value');
});
});

describe('getAccessibleDescription', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ function testCuesEvaluateWithData(expectedData, nodeData): void {
const dataSetterMock = Mock.ofInstance(data => {});

dataSetterMock.setup(m => m(It.isValue(expectedData))).verifiable(Times.once());
getAccessibleTextMock.setup(m => m(nodeStub, false)).returns(n => expectedData.accessibleName);
getAccessibleTextMock.setup(m => m(nodeStub)).returns(n => expectedData.accessibleName);

let result;
GlobalScope.using(getAccessibleTextMock).with(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ function testEvaluate(
getPropertyValuesMock
.setup(m => m(It.isValue(nodeStub), It.isAny()))
.returns(v => expectedData.ariaAttributes);
getAccessibleTextMock.setup(m => m(nodeStub, false)).returns(n => expectedData.accessibleName);
getAccessibleTextMock.setup(m => m(nodeStub)).returns(n => expectedData.accessibleName);

let result;
GlobalScope.using(getPropertyValuesMock, getAccessibleTextMock).with(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ describe('link purpose', () => {

beforeEach(() => {
dataSetterMock = Mock.ofInstance(data => {});
getAccessibleTextMock.setup(m => m(It.isAny(), false)).returns(_ => 'accessible-text');
getAccessibleTextMock.setup(m => m(It.isAny())).returns(_ => 'accessible-text');
getAccessibleDescriptionMock
.setup(m => m(It.isAny()))
.returns(_ => 'accessible-description');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,7 @@ describe('native widgets default', () => {
getAccessibleDescriptionMock
.setup(m => m(nodeStub))
.returns(v => expectedData.accessibleDescription);
getAccessibleTextMock
.setup(m => m(nodeStub, false))
.returns(n => expectedData.accessibleName);
getAccessibleTextMock.setup(m => m(nodeStub)).returns(n => expectedData.accessibleName);

let result;
GlobalScope.using(getAccessibleDescriptionMock, getAccessibleTextMock).with(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,7 @@ describe('widget function', () => {
getAttributesMock
.setup(m => m(It.isValue(nodeStub), It.isAny()))
.returns(v => expectedData.ariaAttributes);
getAccessibleTextMock
.setup(m => m(nodeStub, false))
.returns(n => expectedData.accessibleName);
getAccessibleTextMock.setup(m => m(nodeStub)).returns(n => expectedData.accessibleName);

let result;
GlobalScope.using(getAttributesMock, getAccessibleTextMock).with(() => {
Expand Down

0 comments on commit bad797e

Please sign in to comment.