From f2337b5a14c963a554712bc5e99f3b12e72a39eb Mon Sep 17 00:00:00 2001 From: Jesse Renee Beach Date: Tue, 13 Sep 2016 12:14:36 -0700 Subject: [PATCH 1/5] Add a hook that throws a runtime warning for invalid WAI ARIA attributes and values. --- docs/warnings/invalid-aria-prop.md | 11 ++ src/renderers/dom/ReactDOM.js | 2 + .../dom/shared/ARIADOMPropertyConfig.js | 74 +++++++++++++ .../dom/shared/ReactDefaultInjection.js | 2 + .../__tests__/ReactDOMInvalidARIAHook-test.js | 47 ++++++++ .../shared/hooks/ReactDOMInvalidARIAHook.js | 101 ++++++++++++++++++ 6 files changed, 237 insertions(+) create mode 100644 docs/warnings/invalid-aria-prop.md create mode 100644 src/renderers/dom/shared/ARIADOMPropertyConfig.js create mode 100644 src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js create mode 100644 src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js diff --git a/docs/warnings/invalid-aria-prop.md b/docs/warnings/invalid-aria-prop.md new file mode 100644 index 000000000000..53ebdd9bc406 --- /dev/null +++ b/docs/warnings/invalid-aria-prop.md @@ -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 \ No newline at end of file diff --git a/src/renderers/dom/ReactDOM.js b/src/renderers/dom/ReactDOM.js index c68819b43ea7..ff39dc5aa471 100644 --- a/src/renderers/dom/ReactDOM.js +++ b/src/renderers/dom/ReactDOM.js @@ -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; diff --git a/src/renderers/dom/shared/ARIADOMPropertyConfig.js b/src/renderers/dom/shared/ARIADOMPropertyConfig.js new file mode 100644 index 000000000000..108278940cd1 --- /dev/null +++ b/src/renderers/dom/shared/ARIADOMPropertyConfig.js @@ -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; diff --git a/src/renderers/dom/shared/ReactDefaultInjection.js b/src/renderers/dom/shared/ReactDefaultInjection.js index 6d42abd0ac02..20b89c8c9a20 100644 --- a/src/renderers/dom/shared/ReactDefaultInjection.js +++ b/src/renderers/dom/shared/ReactDefaultInjection.js @@ -11,6 +11,7 @@ 'use strict'; +var ARIADOMPropertyConfig = require('ARIADOMPropertyConfig'); var BeforeInputEventPlugin = require('BeforeInputEventPlugin'); var ChangeEventPlugin = require('ChangeEventPlugin'); var DefaultEventPluginOrder = require('DefaultEventPluginOrder'); @@ -73,6 +74,7 @@ function inject() { ReactDOMTextComponent ); + ReactInjection.DOMProperty.injectDOMPropertyConfig(ARIADOMPropertyConfig); ReactInjection.DOMProperty.injectDOMPropertyConfig(HTMLDOMPropertyConfig); ReactInjection.DOMProperty.injectDOMPropertyConfig(SVGDOMPropertyConfig); diff --git a/src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js b/src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js new file mode 100644 index 000000000000..f364a61ce5ec --- /dev/null +++ b/src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js @@ -0,0 +1,47 @@ +/** + * 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 ReactDOM; + var ReactTestUtils; + var mountComponent; + + beforeEach(() => { + jest.resetModuleRegistry(); + React = require('React'); + ReactDOM = require('ReactDOM'); + ReactTestUtils = require('ReactTestUtils'); + + mountComponent = function(props) { + ReactTestUtils.renderIntoDocument(
); + }; + }); + + 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 invalid aria-* props', () => { + 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
tag. ' + + 'For details, see https://fb.me/invalid-aria-prop' + ); + }); + }); +}); \ No newline at end of file diff --git a/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js b/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js new file mode 100644 index 000000000000..5116d342f642 --- /dev/null +++ b/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js @@ -0,0 +1,101 @@ +/** + * 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 EventPluginRegistry = require('EventPluginRegistry'); +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; \ No newline at end of file From 62daceb09a4d07bde6f820d83dbdd8790c9564e2 Mon Sep 17 00:00:00 2001 From: Jesse Renee Beach Date: Thu, 15 Sep 2016 11:01:57 -0700 Subject: [PATCH 2/5] Resolved linting errors. --- .../dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js | 4 +--- src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js | 5 ++--- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js b/src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js index f364a61ce5ec..39fddcad9fed 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js @@ -13,14 +13,12 @@ describe('ReactDOMInvalidARIAHook', () => { var React; - var ReactDOM; var ReactTestUtils; var mountComponent; beforeEach(() => { jest.resetModuleRegistry(); React = require('React'); - ReactDOM = require('ReactDOM'); ReactTestUtils = require('ReactTestUtils'); mountComponent = function(props) { @@ -44,4 +42,4 @@ describe('ReactDOMInvalidARIAHook', () => { ); }); }); -}); \ No newline at end of file +}); diff --git a/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js b/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js index 5116d342f642..d773622d5349 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js @@ -12,7 +12,6 @@ 'use strict'; var DOMProperty = require('DOMProperty'); -var EventPluginRegistry = require('EventPluginRegistry'); var ReactComponentTreeHook = require('ReactComponentTreeHook'); var warning = require('warning'); @@ -72,7 +71,7 @@ function warnInvalidARIAProps(debugID, element) { ReactComponentTreeHook.getStackAddendumByID(debugID) ); } -}; +} function handleElement(debugID, element) { if (element == null || typeof element.type !== 'string') { @@ -98,4 +97,4 @@ var ReactDOMInvalidARIAHook = { }, }; -module.exports = ReactDOMInvalidARIAHook; \ No newline at end of file +module.exports = ReactDOMInvalidARIAHook; From 53f888382b7fd1e2603d6b716b69983441fbf673 Mon Sep 17 00:00:00 2001 From: Jesse Renee Beach Date: Thu, 15 Sep 2016 11:08:39 -0700 Subject: [PATCH 3/5] Added a test case for many props. --- .../__tests__/ReactDOMInvalidARIAHook-test.js | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js b/src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js index 39fddcad9fed..060c94fb0ab5 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js @@ -32,7 +32,7 @@ describe('ReactDOMInvalidARIAHook', () => { mountComponent({'aria-label': 'Bumble bees'}); expect(console.error.calls.count()).toBe(0); }); - it('should warn for invalid aria-* props', () => { + it('should warn for one invalid aria-* prop', () => { spyOn(console, 'error'); mountComponent({'aria-badprop': 'maybe'}); expect(console.error.calls.count()).toBe(1); @@ -41,5 +41,19 @@ describe('ReactDOMInvalidARIAHook', () => { '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
' + + 'tag. For details, see https://fb.me/invalid-aria-prop' + ); + }); }); }); From 23ed173f2705bb39c119c1476747f146753d3669 Mon Sep 17 00:00:00 2001 From: Jesse Renee Beach Date: Thu, 15 Sep 2016 13:11:19 -0700 Subject: [PATCH 4/5] Added a test case for ARIA attribute proper casing. --- .../shared/__tests__/ReactDOMInvalidARIAHook-test.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js b/src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js index 060c94fb0ab5..20a320a6ba2b 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js @@ -55,5 +55,15 @@ describe('ReactDOMInvalidARIAHook', () => { '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
tag. ' + + 'For details, see https://fb.me/invalid-aria-prop' + ); + }); }); }); From cb0a46ba95b53dd671ba386cd3c51a3ae8da06b2 Mon Sep 17 00:00:00 2001 From: Jesse Renee Beach Date: Fri, 16 Sep 2016 14:54:39 -0700 Subject: [PATCH 5/5] Added a warning for uppercased attributes to ReactDOMInvalidARIAHook --- .../__tests__/ReactDOMInvalidARIAHook-test.js | 4 +-- .../shared/hooks/ReactDOMInvalidARIAHook.js | 35 ++++++++++++++----- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js b/src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js index 20a320a6ba2b..3be7d6127dd6 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js @@ -61,8 +61,8 @@ describe('ReactDOMInvalidARIAHook', () => { 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
tag. ' + - 'For details, see https://fb.me/invalid-aria-prop' + 'Warning: Unknown ARIA attribute aria-hasPopup. ' + + 'Did you mean aria-haspopup?' ); }); }); diff --git a/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js b/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js index d773622d5349..6e2fdcfa363e 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js @@ -26,15 +26,34 @@ function validateProperty(tagName, name, debugID) { ) { 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; + + if (rARIA.test(name)) { + var lowerCasedName = name.toLowerCase(); + var standardName = + DOMProperty.getPossibleStandardName.hasOwnProperty(lowerCasedName) ? + DOMProperty.getPossibleStandardName[lowerCasedName] : + null; + + // If this is an aria-* attribute, but is not listed in the known DOM + // DOM properties, then it is an invalid aria-* attribute. + if (standardName == null) { + warnedProperties[name] = true; + return false; + } + // aria-* attributes should be lowercase; suggest the lowercase version. + if (name !== standardName) { + warning( + false, + 'Unknown ARIA attribute %s. Did you mean %s?%s', + name, + standardName, + ReactComponentTreeHook.getStackAddendumByID(debugID) + ); + warnedProperties[name] = true; + return true; + } } + return true; }