@samitouri / QOS-React / commits / c1dbc8e470

Changes to attribute whitelist logic (#10564)

* Remove HTMLPropertyConfig entries for non-boolean values When we originally removed attributes from the whitelist, we assumed a few attributes were string booleans, but they are not: Autocomplete ("on", "off") https://developer.apple.com/library/content/documentation/AppleApplications/Reference/SafariHTMLRef/Articles/Attributes.html#autocomplete Autocapitalize ("none", "sentence", "words", ...) https://developer.apple.com/library/content/documentation/AppleApplications/Reference/SafariHTMLRef/Articles/Attributes.html#autocapitalize Autocorrect ("on", "off") https://developer.apple.com/library/content/documentation/AppleApplications/Reference/SafariHTMLRef/Articles/Attributes.html#autocorrect Autosave (string) https://developer.apple.com/library/content/documentation/AppleApplications/Reference/SafariHTMLRef/Articles/Attributes.html#autosave * Only HAS_BOOLEAN_VALUE attribute flag can assign booleans * Use a non-boolean attribute in object assignment tests * Add HAS_STRING_BOOLEAN_VALUE attribute flag * Fix boolean tests, add boolean warning. * Reserved props should allow booleans * Remove outdated comments * Style tweaks * Don't treat dashed SVG tags as custom elements * SVG elements like font-face are not custom attributes - Adds exceptions to isCustomAttribute for dashed SVG elements - Use consistent custom element check across all modules * Move namespace check to isCustomAttribute. Add caveat for stack. * Remove unused namespace variable assignment * Fix the DEV-only whitelist * Don't read property twice * Ignore and warn about non-string `is` attribute * Blacklist "aria" and "data" attributes * Don't pass unknown on* attributes through * Remove dead code * Avoid accessing namespace when possible * Drop .only in ReactDOMComponent-test * Make isCustomComponent logic more solid * Do attribute name check earlier * Fix fbjs import * Revert unintentional edit * Re-allow "data" attribute We intentionally allowed it. * Use stricter check when attaching events * Pass SVG boolean attributes with correct casing * Fix the test * Undo the SVG dashed-name fix Per conversation with @sebmarkbage we decided that the fix is too complicated, and it's unfortunate it depends on the DOM element. It's only relevant for super rare tags that aren't even working consistently across browsers so we'll leave it unfixed for now. * Prettier * Fix lint * Fix flow * Pass "aria" through but still warn * Remove special cases for onfocusin, onfocusout They're covered by event handler code now. * Add a more specific warning for unknown events * Pass badly cased React attributes through with warning

Nathan Hunzaker committed Aug 30, 2017 at 21:28 UTC c1dbc8e4708d1de49d8e6a22e9682987d08a15c7
13 files changed +353 -112
src/renderers/dom/fiber/ReactDOMFiberComponent.js
+9 -11
@@ -40,10 +40,10 @@ if (__DEV__) {
40 var ReactDOMUnknownPropertyHook = require('ReactDOMUnknownPropertyHook');
41 var {validateProperties: validateARIAProperties} = ReactDOMInvalidARIAHook;
42 var {
43 - validateProperties: validateInputPropertes,
43 + validateProperties: validateInputProperties,
44 } = ReactDOMNullInputValuePropHook;
45 var {
46 - validateProperties: validateUnknownPropertes,
46 + validateProperties: validateUnknownProperties,
47 } = ReactDOMUnknownPropertyHook;
48 }
49
@@ -72,8 +72,8 @@ if (__DEV__) {
72
73 var validatePropertiesInDevelopment = function(type, props) {
74 validateARIAProperties(type, props);
75 - validateInputPropertes(type, props);
76 - validateUnknownPropertes(type, props);
75 + validateInputProperties(type, props);
76 + validateUnknownProperties(type, props);
77 };
78
79 var warnForTextDifference = function(serverText: string, clientText: string) {
@@ -233,7 +233,7 @@ function setInitialDOMProperties(
233 } else if (propKey === SUPPRESS_CONTENT_EDITABLE_WARNING) {
234 // Noop
235 } else if (registrationNameModules.hasOwnProperty(propKey)) {
236 - if (nextProp) {
236 + if (nextProp != null) {
237 if (__DEV__ && typeof nextProp !== 'function') {
238 warnForInvalidEventListener(propKey, nextProp);
239 }
@@ -305,11 +305,9 @@ var ReactDOMFiberComponent = {
305 if (namespaceURI === HTML_NAMESPACE) {
306 namespaceURI = getIntrinsicNamespace(type);
307 }
308 - if (__DEV__) {
309 - var isCustomComponentTag = isCustomComponent(type, props);
310 - }
308 if (namespaceURI === HTML_NAMESPACE) {
309 if (__DEV__) {
310 + var isCustomComponentTag = isCustomComponent(type, props);
311 // Should this check be gated by parent namespace? Not sure we want to
312 // allow <SVG> or <mATH>.
313 warning(
@@ -328,7 +326,7 @@ var ReactDOMFiberComponent = {
326 // This is guaranteed to yield a script element.
327 var firstChild = ((div.firstChild: any): HTMLScriptElement);
328 domElement = div.removeChild(firstChild);
331 - } else if (props.is) {
329 + } else if (typeof props.is === 'string') {
330 // $FlowIssue `createElement` should be updated for Web Components
331 domElement = ownerDocument.createElement(type, {is: props.is});
332 } else {
@@ -707,7 +705,7 @@ var ReactDOMFiberComponent = {
705 } else if (propKey === SUPPRESS_CONTENT_EDITABLE_WARNING) {
706 // Noop
707 } else if (registrationNameModules.hasOwnProperty(propKey)) {
710 - if (nextProp) {
708 + if (nextProp != null) {
709 // We eagerly listen to this even though we haven't committed yet.
710 if (__DEV__ && typeof nextProp !== 'function') {
711 warnForInvalidEventListener(propKey, nextProp);
@@ -951,7 +949,7 @@ var ReactDOMFiberComponent = {
949 }
950 }
951 } else if (registrationNameModules.hasOwnProperty(propKey)) {
954 - if (nextProp) {
952 + if (nextProp != null) {
953 if (__DEV__ && typeof nextProp !== 'function') {
954 warnForInvalidEventListener(propKey, nextProp);
955 }
src/renderers/dom/shared/DOMMarkupOperations.js
+5 -1
@@ -99,8 +99,12 @@ var DOMMarkupOperations = {
99 (propertyInfo.hasOverloadedBooleanValue && value === true)
100 ) {
101 return attributeName + '=""';
102 + } else if (
103 + typeof value !== 'boolean' ||
104 + DOMProperty.shouldAttributeAcceptBooleanValue(name)
105 + ) {
106 + return attributeName + '=' + quoteAttributeValueForBrowser(value);
107 }
103 - return attributeName + '=' + quoteAttributeValueForBrowser(value);
108 } else if (DOMProperty.shouldSetAttribute(name, value)) {
109 if (value == null) {
110 return '';
src/renderers/dom/shared/DOMProperty.js
+35 -21
@@ -17,14 +17,12 @@ var invariant = require('fbjs/lib/invariant');
17 // case insensitive checks
18 var RESERVED_PROPS = {
19 children: true,
20 - dangerouslysetinnerhtml: true,
21 - autofocus: true,
22 - defaultvalue: true,
23 - defaultchecked: true,
24 - innerhtml: true,
25 - suppresscontenteditablewarning: true,
26 - onfocusin: true,
27 - onfocusout: true,
20 + dangerouslySetInnerHTML: true,
21 + autoFocus: true,
22 + defaultValue: true,
23 + defaultChecked: true,
24 + innerHTML: true,
25 + suppressContentEditableWarning: true,
26 style: true,
27 };
28
@@ -42,6 +40,7 @@ var DOMPropertyInjection = {
40 HAS_NUMERIC_VALUE: 0x8,
41 HAS_POSITIVE_NUMERIC_VALUE: 0x10 | 0x8,
42 HAS_OVERLOADED_BOOLEAN_VALUE: 0x20,
43 + HAS_STRING_BOOLEAN_VALUE: 0x40,
44
45 /**
46 * Inject some specialized knowledge about the DOM. This takes a config object
@@ -103,6 +102,10 @@ var DOMPropertyInjection = {
102 propConfig,
103 Injection.HAS_OVERLOADED_BOOLEAN_VALUE,
104 ),
105 + hasStringBooleanValue: checkMask(
106 + propConfig,
107 + Injection.HAS_STRING_BOOLEAN_VALUE,
108 + ),
109 };
110 invariant(
111 propertyInfo.hasBooleanValue +
@@ -201,26 +204,21 @@ var DOMProperty = {
204 if (DOMProperty.isReservedProp(name)) {
205 return false;
206 }
204 -
207 + if (
208 + (name[0] === 'o' || name[0] === 'O') &&
209 + (name[1] === 'n' || name[1] === 'N')
210 + ) {
211 + return false;
212 + }
213 if (value === null) {
214 return true;
215 }
208 -
209 - var lowerCased = name.toLowerCase();
210 -
211 - var propertyInfo = DOMProperty.properties[name];
212 -
216 switch (typeof value) {
217 case 'boolean':
215 - if (propertyInfo) {
216 - return true;
217 - }
218 - var prefix = lowerCased.slice(0, 5);
219 - return prefix === 'data-' || prefix === 'aria-';
218 + return DOMProperty.shouldAttributeAcceptBooleanValue(name);
219 case 'undefined':
220 case 'number':
221 case 'string':
223 - return true;
222 case 'object':
223 return true;
224 default:
@@ -235,6 +233,22 @@ var DOMProperty = {
233 : null;
234 },
235
236 + shouldAttributeAcceptBooleanValue(name) {
237 + if (DOMProperty.isReservedProp(name)) {
238 + return true;
239 + }
240 + let propertyInfo = DOMProperty.getPropertyInfo(name);
241 + if (propertyInfo) {
242 + return (
243 + propertyInfo.hasBooleanValue ||
244 + propertyInfo.hasStringBooleanValue ||
245 + propertyInfo.hasOverloadedBooleanValue
246 + );
247 + }
248 + var prefix = name.toLowerCase().slice(0, 5);
249 + return prefix === 'data-' || prefix === 'aria-';
250 + },
251 +
252 /**
253 * Checks to see if a property name is within the list of properties
254 * reserved for internal React operations. These properties should
@@ -245,7 +259,7 @@ var DOMProperty = {
259 * @return {boolean} If the name is within reserved props
260 */
261 isReservedProp(name) {
248 - return RESERVED_PROPS.hasOwnProperty(name.toLowerCase());
262 + return RESERVED_PROPS.hasOwnProperty(name);
263 },
264
265 injection: DOMPropertyInjection,
src/renderers/dom/shared/HTMLDOMPropertyConfig.js
+9 -16
@@ -20,6 +20,7 @@ var HAS_POSITIVE_NUMERIC_VALUE =
20 DOMProperty.injection.HAS_POSITIVE_NUMERIC_VALUE;
21 var HAS_OVERLOADED_BOOLEAN_VALUE =
22 DOMProperty.injection.HAS_OVERLOADED_BOOLEAN_VALUE;
23 +var HAS_STRING_BOOLEAN_VALUE = DOMProperty.injection.HAS_STRING_BOOLEAN_VALUE;
24
25 var HTMLDOMPropertyConfig = {
26 // When adding attributes to this list, be sure to also add them to
@@ -27,6 +28,9 @@ var HTMLDOMPropertyConfig = {
28 // name warnings.
29 Properties: {
30 allowFullScreen: HAS_BOOLEAN_VALUE,
31 + // IE only true/false iFrame attribute
32 + // https://msdn.microsoft.com/en-us/library/ms533072(v=vs.85).aspx
33 + allowTransparency: HAS_STRING_BOOLEAN_VALUE,
34 // specifies target context for links with `preload` type
35 async: HAS_BOOLEAN_VALUE,
36 // autoFocus is polyfilled/normalized by AutoFocusUtils
@@ -35,11 +39,13 @@ var HTMLDOMPropertyConfig = {
39 capture: HAS_BOOLEAN_VALUE,
40 checked: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE,
41 cols: HAS_POSITIVE_NUMERIC_VALUE,
42 + contentEditable: HAS_STRING_BOOLEAN_VALUE,
43 controls: HAS_BOOLEAN_VALUE,
44 default: HAS_BOOLEAN_VALUE,
45 defer: HAS_BOOLEAN_VALUE,
46 disabled: HAS_BOOLEAN_VALUE,
47 download: HAS_OVERLOADED_BOOLEAN_VALUE,
48 + draggable: HAS_STRING_BOOLEAN_VALUE,
49 formNoValidate: HAS_BOOLEAN_VALUE,
50 hidden: HAS_BOOLEAN_VALUE,
51 loop: HAS_BOOLEAN_VALUE,
@@ -62,6 +68,7 @@ var HTMLDOMPropertyConfig = {
68 start: HAS_NUMERIC_VALUE,
69 // support for projecting regular DOM Elements via V1 named slots ( shadow dom )
70 span: HAS_POSITIVE_NUMERIC_VALUE,
71 + spellCheck: HAS_STRING_BOOLEAN_VALUE,
72 // Style must be explicitly set in the attribute list. React components
73 // expect a style object
74 style: 0,
@@ -75,22 +82,8 @@ var HTMLDOMPropertyConfig = {
82 htmlFor: 0,
83 httpEquiv: 0,
84 // Attributes with mutation methods must be specified in the whitelist
78 - value: 0,
79 - // The following attributes expect boolean values. They must be in
80 - // the whitelist to allow boolean attribute assignment:
81 - autoComplete: 0,
82 - // IE only true/false iFrame attribute
83 - // https://msdn.microsoft.com/en-us/library/ms533072(v=vs.85).aspx
84 - allowTransparency: 0,
85 - contentEditable: 0,
86 - draggable: 0,
87 - spellCheck: 0,
88 - // autoCapitalize and autoCorrect are supported in Mobile Safari for
89 - // keyboard hints.
90 - autoCapitalize: 0,
91 - autoCorrect: 0,
92 - // autoSave allows WebKit/Blink to persist values of input fields on page reloads
93 - autoSave: 0,
85 + // Set the string boolean flag to allow the behavior
86 + value: HAS_STRING_BOOLEAN_VALUE,
87 },
88 DOMAttributeNames: {
89 acceptCharset: 'accept-charset',
src/renderers/dom/shared/SVGDOMPropertyConfig.js
+14 -7
@@ -11,6 +11,10 @@
11
12 'use strict';
13
14 +var DOMProperty = require('DOMProperty');
15 +
16 +var {HAS_STRING_BOOLEAN_VALUE} = DOMProperty.injection;
17 +
18 var NS = {
19 xlink: 'http://www.w3.org/1999/xlink',
20 xml: 'http://www.w3.org/XML/1998/namespace',
@@ -113,15 +117,19 @@ var ATTRS = [
117 'xmlns:xlink',
118 'xml:lang',
119 'xml:space',
116 - // The following attributes expect boolean values. They must be in
117 - // the whitelist to allow boolean attribute assignment:
118 - 'autoReverse',
119 - 'externalResourcesRequired',
120 - 'preserveAlpha',
120 ];
121
122 var SVGDOMPropertyConfig = {
124 - Properties: {},
123 + Properties: {
124 + autoReverse: HAS_STRING_BOOLEAN_VALUE,
125 + externalResourcesRequired: HAS_STRING_BOOLEAN_VALUE,
126 + preserveAlpha: HAS_STRING_BOOLEAN_VALUE,
127 + },
128 + DOMAttributeNames: {
129 + autoReverse: 'autoReverse',
130 + externalResourcesRequired: 'externalResourcesRequired',
131 + preserveAlpha: 'preserveAlpha',
132 + },
133 DOMAttributeNamespaces: {
134 xlinkActuate: NS.xlink,
135 xlinkArcrole: NS.xlink,
@@ -134,7 +142,6 @@ var SVGDOMPropertyConfig = {
142 xmlLang: NS.xml,
143 xmlSpace: NS.xml,
144 },
137 - DOMAttributeNames: {},
145 };
146
147 var CAMELIZE = /[\-\:]([a-z])/g;
src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
+158 -7
@@ -178,6 +178,65 @@ describe('ReactDOMComponent', () => {
178 );
179 });
180
181 + it('should warn for unknown string event handlers', () => {
182 + spyOn(console, 'error');
183 + var container = document.createElement('div');
184 + ReactDOM.render(<div onUnknown="alert(&quot;hack&quot;)" />, container);
185 + expect(container.firstChild.hasAttribute('onUnknown')).toBe(false);
186 + expect(container.firstChild.onUnknown).toBe(undefined);
187 + ReactDOM.render(<div onunknown="alert(&quot;hack&quot;)" />, container);
188 + expect(container.firstChild.hasAttribute('onunknown')).toBe(false);
189 + expect(container.firstChild.onunknown).toBe(undefined);
190 + ReactDOM.render(<div on-unknown="alert(&quot;hack&quot;)" />, container);
191 + expect(container.firstChild.hasAttribute('on-unknown')).toBe(false);
192 + expect(container.firstChild['on-unknown']).toBe(undefined);
193 + expectDev(console.error.calls.count(0)).toBe(3);
194 + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
195 + 'Warning: Unknown event handler property `onUnknown`. It will be ignored.\n in div (at **)',
196 + );
197 + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toBe(
198 + 'Warning: Unknown event handler property `onunknown`. It will be ignored.\n in div (at **)',
199 + );
200 + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(2)[0])).toBe(
201 + 'Warning: Unknown event handler property `on-unknown`. It will be ignored.\n in div (at **)',
202 + );
203 + });
204 +
205 + it('should warn for unknown function event handlers', () => {
206 + spyOn(console, 'error');
207 + var container = document.createElement('div');
208 + ReactDOM.render(<div onUnknown={function() {}} />, container);
209 + expect(container.firstChild.hasAttribute('onUnknown')).toBe(false);
210 + expect(container.firstChild.onUnknown).toBe(undefined);
211 + ReactDOM.render(<div onunknown={function() {}} />, container);
212 + expect(container.firstChild.hasAttribute('onunknown')).toBe(false);
213 + expect(container.firstChild.onunknown).toBe(undefined);
214 + ReactDOM.render(<div on-unknown={function() {}} />, container);
215 + expect(container.firstChild.hasAttribute('on-unknown')).toBe(false);
216 + expect(container.firstChild['on-unknown']).toBe(undefined);
217 + expectDev(console.error.calls.count(0)).toBe(3);
218 + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
219 + 'Warning: Unknown event handler property `onUnknown`. It will be ignored.\n in div (at **)',
220 + );
221 + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toBe(
222 + 'Warning: Unknown event handler property `onunknown`. It will be ignored.\n in div (at **)',
223 + );
224 + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(2)[0])).toBe(
225 + 'Warning: Unknown event handler property `on-unknown`. It will be ignored.\n in div (at **)',
226 + );
227 + });
228 +
229 + it('should warn for badly cased React attributes', () => {
230 + spyOn(console, 'error');
231 + var container = document.createElement('div');
232 + ReactDOM.render(<div CHILDREN="5" />, container);
233 + expect(container.firstChild.getAttribute('CHILDREN')).toBe('5');
234 + expectDev(console.error.calls.count(0)).toBe(1);
235 + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
236 + 'Warning: Invalid DOM property `CHILDREN`. Did you mean `children`?\n in div (at **)',
237 + );
238 + });
239 +
240 it('should not warn for "0" as a unitless style value', () => {
241 spyOn(console, 'error');
242
@@ -635,12 +694,24 @@ describe('ReactDOMComponent', () => {
694 expect(nodeValueSetter.mock.calls.length).toBe(3);
695 });
696
638 - it('should ignore attribute whitelist for elements with the "is: attribute', () => {
697 + it('should ignore attribute whitelist for elements with the "is" attribute', () => {
698 var container = document.createElement('div');
699 ReactDOM.render(<button is="test" cowabunga="chevynova" />, container);
700 expect(container.firstChild.hasAttribute('cowabunga')).toBe(true);
701 });
702
703 + it('should warn about non-string "is" attribute', () => {
704 + spyOn(console, 'error');
705 + var container = document.createElement('div');
706 + ReactDOM.render(<button is={function() {}} />, container);
707 +
708 + expectDev(console.error.calls.count()).toBe(1);
709 + expectDev(console.error.calls.argsFor(0)[0]).toContain(
710 + 'Received a `function` for string attribute `is`. If this is expected, cast ' +
711 + 'the value to a string.',
712 + );
713 + });
714 +
715 it('should not update when switching between null/undefined', () => {
716 var container = document.createElement('div');
717 var node = ReactDOM.render(<div />, container);
@@ -815,6 +886,16 @@ describe('ReactDOMComponent', () => {
886 );
887 });
888
889 + it('should warn on props reserved for future use', () => {
890 + spyOn(console, 'error');
891 + ReactTestUtils.renderIntoDocument(<div aria="hello" />);
892 + expectDev(console.error.calls.count()).toBe(1);
893 + expectDev(console.error.calls.argsFor(0)[0]).toContain(
894 + 'The `aria` attribute is reserved for future use in React. ' +
895 + 'Pass individual `aria-` attributes instead.',
896 + );
897 + });
898 +
899 it('should warn if the tag is unrecognized', () => {
900 spyOn(console, 'error');
901
@@ -2003,7 +2084,7 @@ describe('ReactDOMComponent', () => {
2084 expect(el.hasAttribute('whatever')).toBe(false);
2085
2086 expectDev(console.error.calls.argsFor(0)[0]).toContain(
2006 - 'Warning: Invalid prop `whatever` on <div> tag',
2087 + 'Warning: Received `true` for non-boolean attribute `whatever`',
2088 );
2089 });
2090
@@ -2016,7 +2097,7 @@ describe('ReactDOMComponent', () => {
2097 expect(el.hasAttribute('whatever')).toBe(false);
2098
2099 expectDev(console.error.calls.argsFor(0)[0]).toContain(
2019 - 'Warning: Invalid prop `whatever` on <div> tag',
2100 + 'Warning: Received `true` for non-boolean attribute `whatever`',
2101 );
2102 });
2103
@@ -2096,10 +2177,8 @@ describe('ReactDOMComponent', () => {
2177
2178 describe('Object stringification', function() {
2179 it('allows objects on known properties', function() {
2099 - var el = ReactTestUtils.renderIntoDocument(
2100 - <div allowTransparency={{}} />,
2101 - );
2102 - expect(el.getAttribute('allowtransparency')).toBe('[object Object]');
2180 + var el = ReactTestUtils.renderIntoDocument(<div acceptCharset={{}} />);
2181 + expect(el.getAttribute('accept-charset')).toBe('[object Object]');
2182 });
2183
2184 it('should pass objects as attributes if they define toString', () => {
@@ -2157,4 +2236,76 @@ describe('ReactDOMComponent', () => {
2236 expect(el.getAttribute('ajaxify')).toBe('ajaxy');
2237 });
2238 });
2239 +
2240 + describe('String boolean attributes', function() {
2241 + it('does not assign string boolean attributes for custom attributes', function() {
2242 + spyOn(console, 'error');
2243 +
2244 + var el = ReactTestUtils.renderIntoDocument(<div whatever={true} />);
2245 +
2246 + expect(el.hasAttribute('whatever')).toBe(false);
2247 +
2248 + expectDev(console.error.calls.argsFor(0)[0]).toContain(
2249 + 'Warning: Received `true` for non-boolean attribute `whatever`.',
2250 + );
2251 + });
2252 +
2253 + it('stringifies the boolean true for allowed attributes', function() {
2254 + var el = ReactTestUtils.renderIntoDocument(<div spellCheck={true} />);
2255 +
2256 + expect(el.getAttribute('spellCheck')).toBe('true');
2257 + });
2258 +
2259 + it('stringifies the boolean false for allowed attributes', function() {
2260 + var el = ReactTestUtils.renderIntoDocument(<div spellCheck={false} />);
2261 +
2262 + expect(el.getAttribute('spellCheck')).toBe('false');
2263 + });
2264 +
2265 + it('stringifies implicit booleans for allowed attributes', function() {
2266 + // eslint-disable-next-line react/jsx-boolean-value
2267 + var el = ReactTestUtils.renderIntoDocument(<div spellCheck />);
2268 +
2269 + expect(el.getAttribute('spellCheck')).toBe('true');
2270 + });
2271 + });
2272 +
2273 + if (ReactDOMFeatureFlags.useFiber) {
2274 + // This is currently broken (and has been broken for a while).
2275 + // We had a fix based on reading namespace, but it was too convoluted.
2276 + // TODO: a proper fix that would happen at the diffing stage.
2277 + describe.skip('Hyphenated SVG elements', function() {
2278 + it('the font-face element is not a custom element', function() {
2279 + spyOn(console, 'error');
2280 + var el = ReactTestUtils.renderIntoDocument(
2281 + <svg><font-face x-height={false} /></svg>,
2282 + );
2283 +
2284 + expect(el.querySelector('font-face').hasAttribute('x-height')).toBe(
2285 + false,
2286 + );
2287 +
2288 + expectDev(console.error.calls.count()).toBe(1);
2289 + expectDev(console.error.calls.argsFor(0)[0]).toContain(
2290 + 'Warning: Invalid DOM property `x-height`. Did you mean `xHeight`',
2291 + );
2292 + });
2293 +
2294 + it('the font-face element does not allow unknown boolean values', function() {
2295 + spyOn(console, 'error');
2296 + var el = ReactTestUtils.renderIntoDocument(
2297 + <svg><font-face whatever={false} /></svg>,
2298 + );
2299 +
2300 + expect(el.querySelector('font-face').hasAttribute('whatever')).toBe(
2301 + false,
2302 + );
2303 +
2304 + expectDev(console.error.calls.count()).toBe(1);
2305 + expectDev(console.error.calls.argsFor(0)[0]).toContain(
2306 + 'Warning: Received `false` for non-boolean attribute `whatever`.',
2307 + );
2308 + });
2309 + });
2310 + }
2311 });
src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js
+50 -16
@@ -596,16 +596,14 @@ describe('ReactDOMServerIntegration', () => {
596 expect(e.getAttribute('class')).toBe('');
597 });
598
599 - // this probably is just masking programmer error, but it is existing behavior.
600 - itRenders('className prop with true value', async render => {
601 - const e = await render(<div className={true} />);
602 - expect(e.getAttribute('class')).toBe('true');
599 + itRenders('no className prop with true value', async render => {
600 + const e = await render(<div className={true} />, 1);
601 + expect(e.hasAttribute('class')).toBe(false);
602 });
603
605 - // this probably is just masking programmer error, but it is existing behavior.
606 - itRenders('className prop with false value', async render => {
607 - const e = await render(<div className={false} />);
608 - expect(e.getAttribute('class')).toBe('false');
604 + itRenders('no className prop with false value', async render => {
605 + const e = await render(<div className={false} />, 1);
606 + expect(e.hasAttribute('class')).toBe(false);
607 });
608
609 itRenders('no className prop with null value', async render => {
@@ -663,16 +661,14 @@ describe('ReactDOMServerIntegration', () => {
661 expect(e.getAttribute('for')).toBe('');
662 });
663
666 - // this probably is just masking programmer error, but it is existing behavior.
667 - itRenders('htmlFor prop with true value', async render => {
668 - const e = await render(<div htmlFor={true} />);
669 - expect(e.getAttribute('for')).toBe('true');
664 + itRenders('no htmlFor prop with true value', async render => {
665 + const e = await render(<div htmlFor={true} />, 1);
666 + expect(e.hasAttribute('for')).toBe(false);
667 });
668
672 - // this probably is just masking programmer error, but it is existing behavior.
673 - itRenders('htmlFor prop with false value', async render => {
674 - const e = await render(<div htmlFor={false} />);
675 - expect(e.getAttribute('for')).toBe('false');
669 + itRenders('no htmlFor prop with false value', async render => {
670 + const e = await render(<div htmlFor={false} />, 1);
671 + expect(e.hasAttribute('for')).toBe(false);
672 });
673
674 itRenders('no htmlFor prop with null value', async render => {
@@ -849,6 +845,12 @@ describe('ReactDOMServerIntegration', () => {
845 const e = await render(<div aria-label={null} />);
846 expect(e.hasAttribute('aria-label')).toBe(false);
847 });
848 +
849 + itRenders('"aria" attribute with a warning', async render => {
850 + // Reserved for future use.
851 + const e = await render(<div aria="hello" />, 1);
852 + expect(e.getAttribute('aria')).toBe('hello');
853 + });
854 });
855
856 describe('cased attributes', function() {
@@ -892,6 +894,17 @@ describe('ReactDOMServerIntegration', () => {
894 expect(e.getAttribute('data-foo')).toBe('bar');
895 });
896
897 + itRenders('badly cased reserved attributes', async render => {
898 + const e = await render(<div CHILDREN="5" />, 1);
899 + expect(e.getAttribute('CHILDREN')).toBe('5');
900 + });
901 +
902 + itRenders('"data" attribute', async render => {
903 + // For `<object />` acts as `src`.
904 + const e = await render(<object data="hello" />);
905 + expect(e.getAttribute('data')).toBe('hello');
906 + });
907 +
908 itRenders('no unknown data- attributes with null value', async render => {
909 const e = await render(<div data-foo={null} />);
910 expect(e.hasAttribute('data-foo')).toBe(false);
@@ -954,6 +967,19 @@ describe('ReactDOMServerIntegration', () => {
967 },
968 );
969
970 + // This is currently broken (and has been broken for a while).
971 + // We had a fix based on reading namespace, but it was too convoluted.
972 + // TODO: a proper fix that would happen at the diffing stage.
973 + //
974 + // itRenders(
975 + // 'known SVG attributes for elements with dashes in tag',
976 + // async render => {
977 + // const e = await render(<svg><font-face accentHeight={10} /></svg>);
978 + // expect(e.firstChild.hasAttribute('accentHeight')).toBe(false);
979 + // expect(e.firstChild.getAttribute('accent-height')).toBe('10');
980 + // },
981 + // );
982 +
983 itRenders('cased custom attributes', async render => {
984 const e = await render(<div fooBar="test" />);
985 expect(e.getAttribute('fooBar')).toBe('test');
@@ -966,6 +992,14 @@ describe('ReactDOMServerIntegration', () => {
992 expect(e.getAttribute('onClick')).toBe(null);
993 expect(e.getAttribute('click')).toBe(null);
994 });
995 +
996 + itRenders('no unknown events', async render => {
997 + const e = await render(
998 + <div onunknownevent="alert(&quot;hack&quot;)" />,
999 + 1,
1000 + );
1001 + expect(e.getAttribute('onunknownevent')).toBe(null);
1002 + });
1003 });
1004
1005 describe('elements and children', function() {
src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js
+2 -1
@@ -12,6 +12,7 @@
12 'use strict';
13
14 var DOMProperty = require('DOMProperty');
15 +var isCustomComponent = require('isCustomComponent');
16
17 var warnedProperties = {};
18 var rARIA = new RegExp('^(aria)-[' + DOMProperty.ATTRIBUTE_NAME_CHAR + ']*$');
@@ -145,7 +146,7 @@ function warnInvalidARIAProps(type, props, debugID) {
146 }
147
148 function validateProperties(type, props, debugID /* Stack only */) {
148 - if (type.indexOf('-') >= 0 || props.is) {
149 + if (isCustomComponent(type, props)) {
150 return;
151 }
152 warnInvalidARIAProps(type, props, debugID);
src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js
+53 -14
@@ -13,6 +13,7 @@
13
14 var DOMProperty = require('DOMProperty');
15 var EventPluginRegistry = require('EventPluginRegistry');
16 +var isCustomComponent = require('isCustomComponent');
17
18 if (__DEV__) {
19 var warning = require('fbjs/lib/warning');
@@ -78,6 +79,17 @@ if (__DEV__) {
79 return true;
80 }
81
82 + if (lowerCasedName.indexOf('on') === 0) {
83 + warning(
84 + false,
85 + 'Unknown event handler property `%s`. It will be ignored.%s',
86 + name,
87 + getStackAddendum(debugID),
88 + );
89 + warnedProperties[name] = true;
90 + return true;
91 + }
92 +
93 // Let the ARIA attribute hook validate ARIA attributes
94 if (ARIA_NAME_REGEX.test(name)) {
95 return true;
@@ -104,6 +116,33 @@ if (__DEV__) {
116 return true;
117 }
118
119 + if (lowerCasedName === 'aria') {
120 + warning(
121 + false,
122 + 'The `aria` attribute is reserved for future use in React. ' +
123 + 'Pass individual `aria-` attributes instead.',
124 + );
125 + warnedProperties[name] = true;
126 + return true;
127 + }
128 +
129 + if (
130 + lowerCasedName === 'is' &&
131 + value !== null &&
132 + value !== undefined &&
133 + typeof value !== 'string'
134 + ) {
135 + warning(
136 + false,
137 + 'Received a `%s` for string attribute `is`. If this is expected, cast ' +
138 + 'the value to a string.%s',
139 + typeof value,
140 + getStackAddendum(debugID),
141 + );
142 + warnedProperties[name] = true;
143 + return true;
144 + }
145 +
146 if (typeof value === 'number' && isNaN(value)) {
147 warning(
148 false,
@@ -132,6 +171,19 @@ if (__DEV__) {
171 }
172 }
173
174 + if (typeof value === 'boolean') {
175 + warning(
176 + DOMProperty.shouldAttributeAcceptBooleanValue(name),
177 + 'Received `%s` for non-boolean attribute `%s`. If this is expected, cast ' +
178 + 'the value to a string.%s',
179 + value,
180 + name,
181 + getStackAddendum(debugID),
182 + );
183 + warnedProperties[name] = true;
184 + return true;
185 + }
186 +
187 // Now that we've validated casing, do not validate
188 // data types for reserved props
189 if (DOMProperty.isReservedProp(name)) {
@@ -154,23 +206,10 @@ var warnUnknownProperties = function(type, props, debugID) {
206 var isValid = validateProperty(type, key, props[key], debugID);
207 if (!isValid) {
208 unknownProps.push(key);
157 - var value = props[key];
158 - if (typeof value === 'object' && value !== null) {
159 - warning(
160 - false,
161 - 'The %s prop on <%s> is not a known property, and was given an object.' +
162 - 'Remove it, or it will appear in the ' +
163 - 'DOM after a future React update.%s',
164 - key,
165 - type,
166 - getStackAddendum(debugID),
167 - );
168 - }
209 }
210 }
211
212 var unknownPropString = unknownProps.map(prop => '`' + prop + '`').join(', ');
173 -
213 if (unknownProps.length === 1) {
214 warning(
215 false,
@@ -195,7 +234,7 @@ var warnUnknownProperties = function(type, props, debugID) {
234 };
235
236 function validateProperties(type, props, debugID /* Stack only */) {
198 - if (type.indexOf('-') >= 0 || props.is) {
237 + if (isCustomComponent(type, props)) {
238 return;
239 }
240 warnUnknownProperties(type, props, debugID);
src/renderers/dom/shared/hooks/possibleStandardNames.js
+4 -4
@@ -64,7 +64,7 @@ var possibleStandardNames = {
64 enctype: 'encType',
65 for: 'htmlFor',
66 form: 'form',
67 - formMethod: 'formMethod',
67 + formmethod: 'formMethod',
68 formaction: 'formAction',
69 formenctype: 'formEncType',
70 formnovalidate: 'formNoValidate',
@@ -99,7 +99,7 @@ var possibleStandardNames = {
99 loop: 'loop',
100 low: 'low',
101 manifest: 'manifest',
102 - marginWidth: 'marginWidth',
102 + marginwidth: 'marginWidth',
103 marginheight: 'marginHeight',
104 max: 'max',
105 maxlength: 'maxLength',
@@ -107,7 +107,7 @@ var possibleStandardNames = {
107 mediagroup: 'mediaGroup',
108 method: 'method',
109 min: 'min',
110 - minLength: 'minlength',
110 + minlength: 'minLength',
111 multiple: 'multiple',
112 muted: 'muted',
113 name: 'name',
@@ -236,7 +236,7 @@ var possibleStandardNames = {
236 filter: 'filter',
237 filterres: 'filterRes',
238 filterunits: 'filterUnits',
239 - floodOpacity: 'floodOpacity',
239 + floodopacity: 'floodOpacity',
240 'flood-opacity': 'floodOpacity',
241 floodcolor: 'floodColor',
242 'flood-color': 'floodColor',
src/renderers/dom/shared/utils/isCustomComponent.js
+3 -2
@@ -7,12 +7,13 @@
7 * of patent rights can be found in the PATENTS file in the same directory.
8 *
9 * @providesModule isCustomComponent
10 + * @flow
11 */
12
13 'use strict';
14
14 -function isCustomComponent(tagName, props) {
15 - return tagName.indexOf('-') >= 0 || props.is != null;
15 +function isCustomComponent(tagName: string, props: Object) {
16 + return tagName.indexOf('-') >= 0 || typeof props.is === 'string';
17 }
18
19 module.exports = isCustomComponent;
src/renderers/dom/stack/client/ReactDOMComponent.js
+4 -4
@@ -480,9 +480,6 @@ ReactDOMComponent.Mixin = {
480
481 assertValidProps(this, props);
482
483 - if (__DEV__) {
484 - var isCustomComponentTag = isCustomComponent(this._tag, props);
485 - }
483 // We create tags in the namespace of their parent container, except HTML
484 // tags get no namespace.
485 var namespaceURI;
@@ -500,6 +497,9 @@ ReactDOMComponent.Mixin = {
497 ) {
498 namespaceURI = Namespaces.html;
499 }
500 + if (__DEV__) {
501 + var isCustomComponentTag = isCustomComponent(this._tag, props);
502 + }
503 if (namespaceURI === Namespaces.html) {
504 if (__DEV__) {
505 warning(
@@ -548,7 +548,7 @@ ReactDOMComponent.Mixin = {
548 var div = ownerDocument.createElement('div');
549 div.innerHTML = `<${type}></${type}>`;
550 el = div.removeChild(div.firstChild);
551 - } else if (props.is) {
551 + } else if (typeof props.is === 'string') {
552 el = ownerDocument.createElement(type, {is: props.is});
553 } else {
554 // Separate else branch instead of using `props.is || undefined` above because of a Firefox bug.
src/renderers/shared/server/ReactPartialRenderer.js
+7 -8
@@ -28,6 +28,7 @@ var hyphenateStyleName = require('fbjs/lib/hyphenateStyleName');
28 var invariant = require('fbjs/lib/invariant');
29 var memoizeStringOnly = require('fbjs/lib/memoizeStringOnly');
30 var omittedCloseTags = require('omittedCloseTags');
31 +var isCustomComponent = require('isCustomComponent');
32
33 var toArray = React.Children.toArray;
34
@@ -39,15 +40,15 @@ if (__DEV__) {
40 validateProperties: validateARIAProperties,
41 } = require('ReactDOMInvalidARIAHook');
42 var {
42 - validateProperties: validateInputPropertes,
43 + validateProperties: validateInputProperties,
44 } = require('ReactDOMNullInputValuePropHook');
45 var {
45 - validateProperties: validateUnknownPropertes,
46 + validateProperties: validateUnknownProperties,
47 } = require('ReactDOMUnknownPropertyHook');
48 var validatePropertiesInDevelopment = function(type, props) {
49 validateARIAProperties(type, props);
49 - validateInputPropertes(type, props);
50 - validateUnknownPropertes(type, props);
50 + validateInputProperties(type, props);
51 + validateUnknownProperties(type, props);
52 };
53
54 var describeComponentFrame = require('describeComponentFrame');
@@ -254,14 +255,11 @@ var RESERVED_PROPS = {
255 suppressContentEditableWarning: null,
256 };
257
257 -function isCustomComponent(tagName, props) {
258 - return tagName.indexOf('-') >= 0 || props.is != null;
259 -}
260 -
258 function createOpenTagMarkup(
259 tagVerbatim,
260 tagLowercase,
261 props,
262 + namespace,
263 makeStaticMarkup,
264 isRootElement,
265 instForDebug,
@@ -786,6 +784,7 @@ class ReactDOMServerRenderer {
784 element.type,
785 tag,
786 props,
787 + namespace,
788 this.makeStaticMarkup,
789 this.stack.length === 1,
790 null,