diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index 0ce5956146a3..4fe120cdaf47 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -40,10 +40,10 @@ if (__DEV__) { var ReactDOMUnknownPropertyHook = require('ReactDOMUnknownPropertyHook'); var {validateProperties: validateARIAProperties} = ReactDOMInvalidARIAHook; var { - validateProperties: validateInputPropertes, + validateProperties: validateInputProperties, } = ReactDOMNullInputValuePropHook; var { - validateProperties: validateUnknownPropertes, + validateProperties: validateUnknownProperties, } = ReactDOMUnknownPropertyHook; } @@ -72,8 +72,8 @@ if (__DEV__) { var validatePropertiesInDevelopment = function(type, props) { validateARIAProperties(type, props); - validateInputPropertes(type, props); - validateUnknownPropertes(type, props); + validateInputProperties(type, props); + validateUnknownProperties(type, props); }; var warnForTextDifference = function(serverText: string, clientText: string) { @@ -233,7 +233,7 @@ function setInitialDOMProperties( } else if (propKey === SUPPRESS_CONTENT_EDITABLE_WARNING) { // Noop } else if (registrationNameModules.hasOwnProperty(propKey)) { - if (nextProp) { + if (nextProp != null) { if (__DEV__ && typeof nextProp !== 'function') { warnForInvalidEventListener(propKey, nextProp); } @@ -305,11 +305,9 @@ var ReactDOMFiberComponent = { if (namespaceURI === HTML_NAMESPACE) { namespaceURI = getIntrinsicNamespace(type); } - if (__DEV__) { - var isCustomComponentTag = isCustomComponent(type, props); - } if (namespaceURI === HTML_NAMESPACE) { if (__DEV__) { + var isCustomComponentTag = isCustomComponent(type, props); // Should this check be gated by parent namespace? Not sure we want to // allow or . warning( @@ -328,7 +326,7 @@ var ReactDOMFiberComponent = { // This is guaranteed to yield a script element. var firstChild = ((div.firstChild: any): HTMLScriptElement); domElement = div.removeChild(firstChild); - } else if (props.is) { + } else if (typeof props.is === 'string') { // $FlowIssue `createElement` should be updated for Web Components domElement = ownerDocument.createElement(type, {is: props.is}); } else { @@ -707,7 +705,7 @@ var ReactDOMFiberComponent = { } else if (propKey === SUPPRESS_CONTENT_EDITABLE_WARNING) { // Noop } else if (registrationNameModules.hasOwnProperty(propKey)) { - if (nextProp) { + if (nextProp != null) { // We eagerly listen to this even though we haven't committed yet. if (__DEV__ && typeof nextProp !== 'function') { warnForInvalidEventListener(propKey, nextProp); @@ -951,7 +949,7 @@ var ReactDOMFiberComponent = { } } } else if (registrationNameModules.hasOwnProperty(propKey)) { - if (nextProp) { + if (nextProp != null) { if (__DEV__ && typeof nextProp !== 'function') { warnForInvalidEventListener(propKey, nextProp); } diff --git a/src/renderers/dom/shared/DOMMarkupOperations.js b/src/renderers/dom/shared/DOMMarkupOperations.js index 9323f7182776..772133625bf2 100644 --- a/src/renderers/dom/shared/DOMMarkupOperations.js +++ b/src/renderers/dom/shared/DOMMarkupOperations.js @@ -99,8 +99,12 @@ var DOMMarkupOperations = { (propertyInfo.hasOverloadedBooleanValue && value === true) ) { return attributeName + '=""'; + } else if ( + typeof value !== 'boolean' || + DOMProperty.shouldAttributeAcceptBooleanValue(name) + ) { + return attributeName + '=' + quoteAttributeValueForBrowser(value); } - return attributeName + '=' + quoteAttributeValueForBrowser(value); } else if (DOMProperty.shouldSetAttribute(name, value)) { if (value == null) { return ''; diff --git a/src/renderers/dom/shared/DOMProperty.js b/src/renderers/dom/shared/DOMProperty.js index d6ddf9c3b3ae..be3c8a1233b4 100644 --- a/src/renderers/dom/shared/DOMProperty.js +++ b/src/renderers/dom/shared/DOMProperty.js @@ -17,14 +17,12 @@ var invariant = require('fbjs/lib/invariant'); // case insensitive checks var RESERVED_PROPS = { children: true, - dangerouslysetinnerhtml: true, - autofocus: true, - defaultvalue: true, - defaultchecked: true, - innerhtml: true, - suppresscontenteditablewarning: true, - onfocusin: true, - onfocusout: true, + dangerouslySetInnerHTML: true, + autoFocus: true, + defaultValue: true, + defaultChecked: true, + innerHTML: true, + suppressContentEditableWarning: true, style: true, }; @@ -42,6 +40,7 @@ var DOMPropertyInjection = { HAS_NUMERIC_VALUE: 0x8, HAS_POSITIVE_NUMERIC_VALUE: 0x10 | 0x8, HAS_OVERLOADED_BOOLEAN_VALUE: 0x20, + HAS_STRING_BOOLEAN_VALUE: 0x40, /** * Inject some specialized knowledge about the DOM. This takes a config object @@ -103,6 +102,10 @@ var DOMPropertyInjection = { propConfig, Injection.HAS_OVERLOADED_BOOLEAN_VALUE, ), + hasStringBooleanValue: checkMask( + propConfig, + Injection.HAS_STRING_BOOLEAN_VALUE, + ), }; invariant( propertyInfo.hasBooleanValue + @@ -201,26 +204,21 @@ var DOMProperty = { if (DOMProperty.isReservedProp(name)) { return false; } - + if ( + (name[0] === 'o' || name[0] === 'O') && + (name[1] === 'n' || name[1] === 'N') + ) { + return false; + } if (value === null) { return true; } - - var lowerCased = name.toLowerCase(); - - var propertyInfo = DOMProperty.properties[name]; - switch (typeof value) { case 'boolean': - if (propertyInfo) { - return true; - } - var prefix = lowerCased.slice(0, 5); - return prefix === 'data-' || prefix === 'aria-'; + return DOMProperty.shouldAttributeAcceptBooleanValue(name); case 'undefined': case 'number': case 'string': - return true; case 'object': return true; default: @@ -235,6 +233,22 @@ var DOMProperty = { : null; }, + shouldAttributeAcceptBooleanValue(name) { + if (DOMProperty.isReservedProp(name)) { + return true; + } + let propertyInfo = DOMProperty.getPropertyInfo(name); + if (propertyInfo) { + return ( + propertyInfo.hasBooleanValue || + propertyInfo.hasStringBooleanValue || + propertyInfo.hasOverloadedBooleanValue + ); + } + var prefix = name.toLowerCase().slice(0, 5); + return prefix === 'data-' || prefix === 'aria-'; + }, + /** * Checks to see if a property name is within the list of properties * reserved for internal React operations. These properties should @@ -245,7 +259,7 @@ var DOMProperty = { * @return {boolean} If the name is within reserved props */ isReservedProp(name) { - return RESERVED_PROPS.hasOwnProperty(name.toLowerCase()); + return RESERVED_PROPS.hasOwnProperty(name); }, injection: DOMPropertyInjection, diff --git a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js index ed3808c7f7dc..19af1eadc0e8 100644 --- a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js +++ b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js @@ -20,6 +20,7 @@ var HAS_POSITIVE_NUMERIC_VALUE = DOMProperty.injection.HAS_POSITIVE_NUMERIC_VALUE; var HAS_OVERLOADED_BOOLEAN_VALUE = DOMProperty.injection.HAS_OVERLOADED_BOOLEAN_VALUE; +var HAS_STRING_BOOLEAN_VALUE = DOMProperty.injection.HAS_STRING_BOOLEAN_VALUE; var HTMLDOMPropertyConfig = { // When adding attributes to this list, be sure to also add them to @@ -27,6 +28,9 @@ var HTMLDOMPropertyConfig = { // name warnings. Properties: { allowFullScreen: HAS_BOOLEAN_VALUE, + // IE only true/false iFrame attribute + // https://msdn.microsoft.com/en-us/library/ms533072(v=vs.85).aspx + allowTransparency: HAS_STRING_BOOLEAN_VALUE, // specifies target context for links with `preload` type async: HAS_BOOLEAN_VALUE, // autoFocus is polyfilled/normalized by AutoFocusUtils @@ -35,11 +39,13 @@ var HTMLDOMPropertyConfig = { capture: HAS_BOOLEAN_VALUE, checked: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE, cols: HAS_POSITIVE_NUMERIC_VALUE, + contentEditable: HAS_STRING_BOOLEAN_VALUE, controls: HAS_BOOLEAN_VALUE, default: HAS_BOOLEAN_VALUE, defer: HAS_BOOLEAN_VALUE, disabled: HAS_BOOLEAN_VALUE, download: HAS_OVERLOADED_BOOLEAN_VALUE, + draggable: HAS_STRING_BOOLEAN_VALUE, formNoValidate: HAS_BOOLEAN_VALUE, hidden: HAS_BOOLEAN_VALUE, loop: HAS_BOOLEAN_VALUE, @@ -62,6 +68,7 @@ var HTMLDOMPropertyConfig = { start: HAS_NUMERIC_VALUE, // support for projecting regular DOM Elements via V1 named slots ( shadow dom ) span: HAS_POSITIVE_NUMERIC_VALUE, + spellCheck: HAS_STRING_BOOLEAN_VALUE, // Style must be explicitly set in the attribute list. React components // expect a style object style: 0, @@ -75,22 +82,8 @@ var HTMLDOMPropertyConfig = { htmlFor: 0, httpEquiv: 0, // Attributes with mutation methods must be specified in the whitelist - value: 0, - // The following attributes expect boolean values. They must be in - // the whitelist to allow boolean attribute assignment: - autoComplete: 0, - // IE only true/false iFrame attribute - // https://msdn.microsoft.com/en-us/library/ms533072(v=vs.85).aspx - allowTransparency: 0, - contentEditable: 0, - draggable: 0, - spellCheck: 0, - // autoCapitalize and autoCorrect are supported in Mobile Safari for - // keyboard hints. - autoCapitalize: 0, - autoCorrect: 0, - // autoSave allows WebKit/Blink to persist values of input fields on page reloads - autoSave: 0, + // Set the string boolean flag to allow the behavior + value: HAS_STRING_BOOLEAN_VALUE, }, DOMAttributeNames: { acceptCharset: 'accept-charset', diff --git a/src/renderers/dom/shared/SVGDOMPropertyConfig.js b/src/renderers/dom/shared/SVGDOMPropertyConfig.js index 46db12aa4250..0254d8ee0c20 100644 --- a/src/renderers/dom/shared/SVGDOMPropertyConfig.js +++ b/src/renderers/dom/shared/SVGDOMPropertyConfig.js @@ -11,6 +11,10 @@ 'use strict'; +var DOMProperty = require('DOMProperty'); + +var {HAS_STRING_BOOLEAN_VALUE} = DOMProperty.injection; + var NS = { xlink: 'http://www.w3.org/1999/xlink', xml: 'http://www.w3.org/XML/1998/namespace', @@ -113,15 +117,19 @@ var ATTRS = [ 'xmlns:xlink', 'xml:lang', 'xml:space', - // The following attributes expect boolean values. They must be in - // the whitelist to allow boolean attribute assignment: - 'autoReverse', - 'externalResourcesRequired', - 'preserveAlpha', ]; var SVGDOMPropertyConfig = { - Properties: {}, + Properties: { + autoReverse: HAS_STRING_BOOLEAN_VALUE, + externalResourcesRequired: HAS_STRING_BOOLEAN_VALUE, + preserveAlpha: HAS_STRING_BOOLEAN_VALUE, + }, + DOMAttributeNames: { + autoReverse: 'autoReverse', + externalResourcesRequired: 'externalResourcesRequired', + preserveAlpha: 'preserveAlpha', + }, DOMAttributeNamespaces: { xlinkActuate: NS.xlink, xlinkArcrole: NS.xlink, @@ -134,7 +142,6 @@ var SVGDOMPropertyConfig = { xmlLang: NS.xml, xmlSpace: NS.xml, }, - DOMAttributeNames: {}, }; var CAMELIZE = /[\-\:]([a-z])/g; diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js index 981c9d3ed2b4..b37e4e647971 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -178,6 +178,65 @@ describe('ReactDOMComponent', () => { ); }); + it('should warn for unknown string event handlers', () => { + spyOn(console, 'error'); + var container = document.createElement('div'); + ReactDOM.render(
, container); + expect(container.firstChild.hasAttribute('onUnknown')).toBe(false); + expect(container.firstChild.onUnknown).toBe(undefined); + ReactDOM.render(
, container); + expect(container.firstChild.hasAttribute('onunknown')).toBe(false); + expect(container.firstChild.onunknown).toBe(undefined); + ReactDOM.render(
, container); + expect(container.firstChild.hasAttribute('on-unknown')).toBe(false); + expect(container.firstChild['on-unknown']).toBe(undefined); + expectDev(console.error.calls.count(0)).toBe(3); + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( + 'Warning: Unknown event handler property `onUnknown`. It will be ignored.\n in div (at **)', + ); + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toBe( + 'Warning: Unknown event handler property `onunknown`. It will be ignored.\n in div (at **)', + ); + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(2)[0])).toBe( + 'Warning: Unknown event handler property `on-unknown`. It will be ignored.\n in div (at **)', + ); + }); + + it('should warn for unknown function event handlers', () => { + spyOn(console, 'error'); + var container = document.createElement('div'); + ReactDOM.render(
, container); + expect(container.firstChild.hasAttribute('onUnknown')).toBe(false); + expect(container.firstChild.onUnknown).toBe(undefined); + ReactDOM.render(
, container); + expect(container.firstChild.hasAttribute('onunknown')).toBe(false); + expect(container.firstChild.onunknown).toBe(undefined); + ReactDOM.render(
, container); + expect(container.firstChild.hasAttribute('on-unknown')).toBe(false); + expect(container.firstChild['on-unknown']).toBe(undefined); + expectDev(console.error.calls.count(0)).toBe(3); + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( + 'Warning: Unknown event handler property `onUnknown`. It will be ignored.\n in div (at **)', + ); + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toBe( + 'Warning: Unknown event handler property `onunknown`. It will be ignored.\n in div (at **)', + ); + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(2)[0])).toBe( + 'Warning: Unknown event handler property `on-unknown`. It will be ignored.\n in div (at **)', + ); + }); + + it('should warn for badly cased React attributes', () => { + spyOn(console, 'error'); + var container = document.createElement('div'); + ReactDOM.render(
, container); + expect(container.firstChild.getAttribute('CHILDREN')).toBe('5'); + expectDev(console.error.calls.count(0)).toBe(1); + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( + 'Warning: Invalid DOM property `CHILDREN`. Did you mean `children`?\n in div (at **)', + ); + }); + it('should not warn for "0" as a unitless style value', () => { spyOn(console, 'error'); @@ -635,12 +694,24 @@ describe('ReactDOMComponent', () => { expect(nodeValueSetter.mock.calls.length).toBe(3); }); - it('should ignore attribute whitelist for elements with the "is: attribute', () => { + it('should ignore attribute whitelist for elements with the "is" attribute', () => { var container = document.createElement('div'); ReactDOM.render(