From 5a13dd090d63c0c779c0e3b75a3a7bc70459c456 Mon Sep 17 00:00:00 2001 From: SanderSpies Date: Sun, 13 Oct 2013 08:45:43 +0200 Subject: [PATCH 1/3] Standardize prop -> DOM attribute process Allow more than strings and numbers to be used as attributes for DOM nodes. This removes the special casing for `0` and `false` that was being used in ReactDOMInput and ReactDOMTextarea. Now we will just `toString` any object we try to insert into a DOM. Closes #422, #372, #302 --- src/dom/components/ReactDOMInput.js | 14 +-- src/dom/components/ReactDOMTextarea.js | 19 +--- .../__tests__/ReactDOMInput-test.js | 59 +++++++++++- .../__tests__/ReactDOMSelect-test.js | 29 ++++++ .../__tests__/ReactDOMTextarea-test.js | 91 ++++++++++++++++--- .../__tests__/escapeTextForBrowser-test.js | 56 ++++++++++++ src/utils/escapeTextForBrowser.js | 21 +---- 7 files changed, 227 insertions(+), 62 deletions(-) create mode 100644 src/utils/__tests__/escapeTextForBrowser-test.js diff --git a/src/dom/components/ReactDOMInput.js b/src/dom/components/ReactDOMInput.js index a569a5ec44..b38b4d1172 100644 --- a/src/dom/components/ReactDOMInput.js +++ b/src/dom/components/ReactDOMInput.js @@ -55,7 +55,7 @@ var ReactDOMInput = ReactCompositeComponent.createClass({ var defaultValue = this.props.defaultValue; return { checked: this.props.defaultChecked || false, - value: defaultValue != null && defaultValue !== false ? defaultValue : '' + value: defaultValue != null ? defaultValue : '' }; }, @@ -74,9 +74,7 @@ var ReactDOMInput = ReactCompositeComponent.createClass({ this.props.checked != null ? this.props.checked : this.state.checked; var value = this.getValue(); - props.value = value != null && value !== false ? - '' + value : - this.state.value; + props.value = value != null ? value : this.state.value; props.onChange = this._handleChange; @@ -105,13 +103,7 @@ var ReactDOMInput = ReactCompositeComponent.createClass({ var value = this.getValue(); if (value != null) { - // Cast `this.props.value` to a string so falsey values that cast to - // truthy strings are not ignored. - DOMPropertyOperations.setValueForProperty( - rootNode, - 'value', - value !== false ? '' + value : '' - ); + DOMPropertyOperations.setValueForProperty(rootNode, 'value', value); } }, diff --git a/src/dom/components/ReactDOMTextarea.js b/src/dom/components/ReactDOMTextarea.js index 0aed044df3..b53df29302 100644 --- a/src/dom/components/ReactDOMTextarea.js +++ b/src/dom/components/ReactDOMTextarea.js @@ -29,9 +29,6 @@ var merge = require('merge'); // Store a reference to the ); + expect(console.warn.argsForCall.length).toBe(1); + expect(node.value).toBe('false'); + }); + + it('should allow objects as children', function() { + spyOn(console, 'warn'); + var obj = { + toString: function() { + return "sharkswithlasers"; + } + }; + var node = renderTextarea(); + expect(console.warn.argsForCall.length).toBe(1); + expect(node.value).toBe('sharkswithlasers'); + }); + it('should throw with multiple or invalid children', function() { spyOn(console, 'warn'); @@ -134,11 +194,12 @@ describe('ReactDOMTextarea', function() { expect(console.warn.argsForCall.length).toBe(1); + var stub; expect(function() { - ReactTestUtils.renderIntoDocument( - - ); - }).toThrow(); + stub = renderTextarea(); + }).not.toThrow(); + + expect(stub.value).toBe('[object Object]'); expect(console.warn.argsForCall.length).toBe(2); }); diff --git a/src/utils/__tests__/escapeTextForBrowser-test.js b/src/utils/__tests__/escapeTextForBrowser-test.js new file mode 100644 index 0000000000..ab5686afe1 --- /dev/null +++ b/src/utils/__tests__/escapeTextForBrowser-test.js @@ -0,0 +1,56 @@ +/** + * Copyright 2013 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * @emails react-core + */ + +"use strict"; + +describe('escapeTextForBrowser', function() { + + var escapeTextForBrowser = require('escapeTextForBrowser'); + + it('should escape boolean to string', function() { + expect(escapeTextForBrowser(true)).toBe('true'); + expect(escapeTextForBrowser(false)).toBe('false'); + }); + + it('should escape object to string', function() { + var escaped = escapeTextForBrowser({ + toString: function() { + return 'ponys'; + } + }); + + expect(escaped).toBe('ponys'); + }); + + it('should escape number to string', function() { + expect(escapeTextForBrowser(42)).toBe('42'); + }); + + it('should escape string', function() { + var escaped = escapeTextForBrowser(''); + expect(escaped).not.toContain('<'); + expect(escaped).not.toContain('>'); + expect(escaped).not.toContain('\''); + expect(escaped).not.toContain('/'); + expect(escaped).not.toContain('\"'); + + escaped = escapeTextForBrowser('&'); + expect(escaped).toBe('&'); + }); + +}); diff --git a/src/utils/escapeTextForBrowser.js b/src/utils/escapeTextForBrowser.js index 8e49d31e9e..26aed86338 100644 --- a/src/utils/escapeTextForBrowser.js +++ b/src/utils/escapeTextForBrowser.js @@ -19,8 +19,6 @@ "use strict"; -var invariant = require('invariant'); - var ESCAPE_LOOKUP = { "&": "&", ">": ">", @@ -30,6 +28,8 @@ var ESCAPE_LOOKUP = { "/": "/" }; +var ESCAPE_REGEX = /[&><"'\/]/g; + function escaper(match) { return ESCAPE_LOOKUP[match]; } @@ -37,24 +37,11 @@ function escaper(match) { /** * Escapes text to prevent scripting attacks. * - * @param {number|string} text Text value to escape. + * @param {*} text Text value to escape. * @return {string} An escaped string. */ function escapeTextForBrowser(text) { - var type = typeof text; - invariant( - type !== 'object', - 'escapeTextForBrowser(...): Attempted to escape an object.' - ); - if (text === '') { - return ''; - } else { - if (type === 'string') { - return text.replace(/[&><"'\/]/g, escaper); - } else { - return (''+text).replace(/[&><"'\/]/g, escaper); - } - } + return ('' + text).replace(ESCAPE_REGEX, escaper); } module.exports = escapeTextForBrowser; From b0645bd5d3bbea40e93194a6c403ce3ba633d45a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20O=E2=80=99Shannessy?= Date: Tue, 15 Oct 2013 10:42:06 -0700 Subject: [PATCH 2/3] Be consistent with object naming in tests This also fixes line length issues our linter was complaining about. --- src/dom/components/__tests__/ReactDOMInput-test.js | 12 ++++++------ .../components/__tests__/ReactDOMTextarea-test.js | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/dom/components/__tests__/ReactDOMInput-test.js b/src/dom/components/__tests__/ReactDOMInput-test.js index f5a2e17740..ea42b4e5a0 100644 --- a/src/dom/components/__tests__/ReactDOMInput-test.js +++ b/src/dom/components/__tests__/ReactDOMInput-test.js @@ -64,14 +64,14 @@ describe('ReactDOMInput', function() { expect(node.value).toBe('false'); }); - it('should display "foobar" for `defaultValue` of `objectToString`', function() { - var objectToString = { + it('should display "foobar" for `defaultValue` of `objToString`', function() { + var objToString = { toString: function() { return "foobar"; } }; - var stub = ; + var stub = ; var node = renderTextInput(stub); expect(node.value).toBe('foobar'); @@ -104,19 +104,19 @@ describe('ReactDOMInput', function() { expect(node.value).toEqual('false'); }); - it('should allow setting `value` to `objectToString`', function() { + it('should allow setting `value` to `objToString`', function() { var stub = ; var node = renderTextInput(stub); expect(node.value).toBe('foo'); - var objectToString = { + var objToString = { toString: function() { return "foobar"; } }; - stub.replaceProps({value: objectToString}); + stub.replaceProps({value: objToString}); expect(node.value).toEqual('foobar'); }); diff --git a/src/dom/components/__tests__/ReactDOMTextarea-test.js b/src/dom/components/__tests__/ReactDOMTextarea-test.js index 271cd4563f..aaca8ea596 100644 --- a/src/dom/components/__tests__/ReactDOMTextarea-test.js +++ b/src/dom/components/__tests__/ReactDOMTextarea-test.js @@ -69,14 +69,14 @@ describe('ReactDOMTextarea', function() { expect(node.value).toBe('false'); }); - it('should display "foobar" for `defaultValue` of `objectToString`', function() { - var objectToString = { + it('should display "foobar" for `defaultValue` of `objToString`', function() { + var objToString = { toString: function() { return "foobar"; } }; - var stub =