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

React dom invalid aria hook #7744

Merged
merged 5 commits into from
Sep 28, 2016
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
11 changes: 11 additions & 0 deletions docs/warnings/invalid-aria-prop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
title: Invalid ARIA Prop Warning
layout: single
permalink: warnings/invalid-aria-prop.html
---

The invalid-aria-prop warning will fire if you attempt to render a DOM element with an aria-* prop that does not exist in the Web Accessibility Initiative (WAI) Accessible Rich Internet Application (ARIA) [specification](https://www.w3.org/TR/wai-aria-1.1/#states_and_properties).

1. If you feel that you are using a valid prop, check the spelling carefully. `aria-labelledby` and `aria-activedescendant` are often misspelled.

2. React does not yet recognize the attribute you specified. This will likely be fixed in a future version of React. However, React currently strips all unknown attributes, so specifying them in your React app will not cause them to be rendered
2 changes: 2 additions & 0 deletions src/renderers/dom/ReactDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,11 @@ if (__DEV__) {
var ReactInstrumentation = require('ReactInstrumentation');
var ReactDOMUnknownPropertyHook = require('ReactDOMUnknownPropertyHook');
var ReactDOMNullInputValuePropHook = require('ReactDOMNullInputValuePropHook');
var ReactDOMInvalidARIAHook = require('ReactDOMInvalidARIAHook');

ReactInstrumentation.debugTool.addHook(ReactDOMUnknownPropertyHook);
ReactInstrumentation.debugTool.addHook(ReactDOMNullInputValuePropHook);
ReactInstrumentation.debugTool.addHook(ReactDOMInvalidARIAHook);
}

module.exports = ReactDOM;
74 changes: 74 additions & 0 deletions src/renderers/dom/shared/ARIADOMPropertyConfig.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/**
* Copyright 2013-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule ARIADOMPropertyConfig
*/

'use strict';

var ARIADOMPropertyConfig = {
Properties: {
// Global States and Properties
'aria-current': 0, // state
'aria-details': 0,
'aria-disabled': 0, // state
'aria-hidden': 0, // state
'aria-invalid': 0, // state
'aria-keyshortcuts': 0,
'aria-label': 0,
'aria-roledescription': 0,
// Widget Attributes
'aria-autocomplete': 0,
'aria-checked': 0,
'aria-expanded': 0,
'aria-haspopup': 0,
'aria-level': 0,
'aria-modal': 0,
'aria-multiline': 0,
'aria-multiselectable': 0,
'aria-orientation': 0,
'aria-placeholder': 0,
'aria-pressed': 0,
'aria-readonly': 0,
'aria-required': 0,
'aria-selected': 0,
'aria-sort': 0,
'aria-valuemax': 0,
'aria-valuemin': 0,
'aria-valuenow': 0,
'aria-valuetext': 0,
// Live Region Attributes
'aria-atomic': 0,
'aria-busy': 0,
'aria-live': 0,
'aria-relevant': 0,
// Drag-and-Drop Attributes
'aria-dropeffect': 0,
'aria-grabbed': 0,
// Relationship Attributes
'aria-activedescendant': 0,
'aria-colcount': 0,
'aria-colindex': 0,
'aria-colspan': 0,
'aria-controls': 0,
'aria-describedby': 0,
'aria-errormessage': 0,
'aria-flowto': 0,
'aria-labelledby': 0,
'aria-owns': 0,
'aria-posinset': 0,
'aria-rowcount': 0,
'aria-rowindex': 0,
'aria-rowspan': 0,
'aria-setsize': 0,
},
DOMAttributeNames: {},
DOMPropertyNames: {},
};

module.exports = ARIADOMPropertyConfig;
2 changes: 2 additions & 0 deletions src/renderers/dom/shared/ReactDefaultInjection.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

'use strict';

var ARIADOMPropertyConfig = require('ARIADOMPropertyConfig');
var BeforeInputEventPlugin = require('BeforeInputEventPlugin');
var ChangeEventPlugin = require('ChangeEventPlugin');
var DefaultEventPluginOrder = require('DefaultEventPluginOrder');
Expand Down Expand Up @@ -73,6 +74,7 @@ function inject() {
ReactDOMTextComponent
);

ReactInjection.DOMProperty.injectDOMPropertyConfig(ARIADOMPropertyConfig);
ReactInjection.DOMProperty.injectDOMPropertyConfig(HTMLDOMPropertyConfig);
ReactInjection.DOMProperty.injectDOMPropertyConfig(SVGDOMPropertyConfig);

Expand Down
69 changes: 69 additions & 0 deletions src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/**
* Copyright 2013-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @emails react-core
*/

'use strict';

describe('ReactDOMInvalidARIAHook', () => {
var React;
var ReactTestUtils;
var mountComponent;

beforeEach(() => {
jest.resetModuleRegistry();
React = require('React');
ReactTestUtils = require('ReactTestUtils');

mountComponent = function(props) {
ReactTestUtils.renderIntoDocument(<div {...props} />);
};
});

describe('aria-* props', () => {
it('should allow valid aria-* props', () => {
spyOn(console, 'error');
mountComponent({'aria-label': 'Bumble bees'});
expect(console.error.calls.count()).toBe(0);
});
it('should warn for one invalid aria-* prop', () => {
spyOn(console, 'error');
mountComponent({'aria-badprop': 'maybe'});
expect(console.error.calls.count()).toBe(1);
expect(console.error.calls.argsFor(0)[0]).toContain(
'Warning: Invalid aria prop `aria-badprop` on <div> tag. ' +
'For details, see https://fb.me/invalid-aria-prop'
);
});
it('should warn for many invalid aria-* props', () => {
spyOn(console, 'error');
mountComponent(
{
'aria-badprop': 'Very tall trees',
'aria-malprop': 'Turbulent seas',
}
);
expect(console.error.calls.count()).toBe(1);
expect(console.error.calls.argsFor(0)[0]).toContain(
'Warning: Invalid aria props `aria-badprop`, `aria-malprop` on <div> ' +
'tag. For details, see https://fb.me/invalid-aria-prop'
);
});
it('should warn for an improperly cased aria-* prop', () => {
spyOn(console, 'error');
// The valid attribute name is aria-haspopup.
mountComponent({'aria-hasPopup': 'true'});
expect(console.error.calls.count()).toBe(1);
expect(console.error.calls.argsFor(0)[0]).toContain(
'Warning: Invalid aria prop `aria-hasPopup` on <div> tag. ' +
'For details, see https://fb.me/invalid-aria-prop'
);
Copy link
Contributor

@aweary aweary Sep 15, 2016

Choose a reason for hiding this comment

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

Since we're using injectDOMPropertyConfig these aria- properties should be populated in DOMProperty.getPossibleStandardName. ReactDOMUnknownPropertyHook uses that to make a recommendation when warning for improperly cased props.

Can you do the same in ReactDOMInvalidARIAHook? That way if someone passes down aria-hasPopup it will ask if they meant aria-haspopup.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great suggestion. I added the changes to address this change request.

});
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we also add a test verifying we get a warning when passing in a valid aria- attribute with invalid casing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Certainly. Good call.

});
100 changes: 100 additions & 0 deletions src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/**
* Copyright 2013-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule ReactDOMInvalidARIAHook
*/

'use strict';

var DOMProperty = require('DOMProperty');
var ReactComponentTreeHook = require('ReactComponentTreeHook');

var warning = require('warning');

var warnedProperties = {};
var rARIA = new RegExp('^(aria)-[' + DOMProperty.ATTRIBUTE_NAME_CHAR + ']*$');

function validateProperty(tagName, name, debugID) {
if (
warnedProperties.hasOwnProperty(name)
&& warnedProperties[name]
) {
return true;
}
// If this is an aria-* attribute, but is not listed in the known DOM
// DOM properties, then it is an invalid aria-* attribute.
if (
rARIA.test(name)
&& !DOMProperty.properties.hasOwnProperty(name)
) {
warnedProperties[name] = true;
return false;
}
return true;
}

function warnInvalidARIAProps(debugID, element) {
const invalidProps = [];

for (var key in element.props) {
var isValid = validateProperty(element.type, key, debugID);
if (!isValid) {
invalidProps.push(key);
}
}

const unknownPropString = invalidProps
.map(prop => '`' + prop + '`')
.join(', ');

if (invalidProps.length === 1) {
warning(
false,
'Invalid aria prop %s on <%s> tag. ' +
'For details, see https://fb.me/invalid-aria-prop%s',
unknownPropString,
element.type,
ReactComponentTreeHook.getStackAddendumByID(debugID)
);
} else if (invalidProps.length > 1) {
warning(
false,
'Invalid aria props %s on <%s> tag. ' +
'For details, see https://fb.me/invalid-aria-prop%s',
unknownPropString,
element.type,
ReactComponentTreeHook.getStackAddendumByID(debugID)
);
}
}

function handleElement(debugID, element) {
if (element == null || typeof element.type !== 'string') {
return;
}
if (element.type.indexOf('-') >= 0 || element.props.is) {
return;
}

warnInvalidARIAProps(debugID, element);
}

var ReactDOMInvalidARIAHook = {
onBeforeMountComponent(debugID, element) {
if (__DEV__) {
handleElement(debugID, element);
}
},
onBeforeUpdateComponent(debugID, element) {
if (__DEV__) {
handleElement(debugID, element);
}
},
};

module.exports = ReactDOMInvalidARIAHook;