Skip to content

Commit 9561a01

Browse files
committed
Support arbitrary attributes on elements with dashes in the tag name.
1 parent 49de80e commit 9561a01

File tree

5 files changed

+154
-7
lines changed

5 files changed

+154
-7
lines changed

src/browser/ui/ReactDOMComponent.js

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,8 +332,12 @@ ReactDOMComponent.Mixin = {
332332
}
333333
propValue = CSSPropertyOperations.createMarkupForStyles(propValue);
334334
}
335-
var markup =
336-
DOMPropertyOperations.createMarkupForProperty(propKey, propValue);
335+
var markup = null;
336+
if (this._tag != null && this._tag.indexOf('-') >= 0) {
337+
markup = DOMPropertyOperations.createMarkupForCustomAttribute(propKey, propValue);
338+
} else {
339+
markup = DOMPropertyOperations.createMarkupForProperty(propKey, propValue);
340+
}
337341
if (markup) {
338342
ret += ' ' + markup;
339343
}
@@ -534,6 +538,12 @@ ReactDOMComponent.Mixin = {
534538
} else if (lastProp) {
535539
deleteListener(this._rootNodeID, propKey);
536540
}
541+
} else if (this._tag.indexOf('-') >= 0) {
542+
BackendIDOperations.updateAttributeByID(
543+
this._rootNodeID,
544+
propKey,
545+
nextProp
546+
);
537547
} else if (
538548
DOMProperty.isStandardName[propKey] ||
539549
DOMProperty.isCustomAttribute(propKey)) {

src/browser/ui/ReactDOMIDOperations.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,24 @@ var ReactDOMIDOperations = {
6868
}
6969
},
7070

71+
/**
72+
* Updates a DOM node with new property values.
73+
*
74+
* @param {string} id ID of the node to update.
75+
* @param {string} name A valid property name.
76+
* @param {*} value New value of the property.
77+
* @internal
78+
*/
79+
updateAttributeByID: function(id, name, value) {
80+
var node = ReactMount.getNode(id);
81+
invariant(
82+
!INVALID_PROPERTY_ERRORS.hasOwnProperty(name),
83+
'updatePropertyByID(...): %s',
84+
INVALID_PROPERTY_ERRORS[name]
85+
);
86+
DOMPropertyOperations.setValueForAttribute(node, name, value);
87+
},
88+
7189
/**
7290
* Updates a DOM node to remove a property. This should only be used to remove
7391
* DOM properties in `DOMProperty`.

src/browser/ui/__tests__/ReactDOMComponent-test.js

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ describe('ReactDOMComponent', function() {
2323
var ReactTestUtils;
2424

2525
beforeEach(function() {
26+
require('mock-modules').dumpCache();
2627
React = require('React');
2728
ReactTestUtils = require('ReactTestUtils');
2829
});
@@ -205,6 +206,61 @@ describe('ReactDOMComponent', function() {
205206
expect(stubStyle.color).toEqual('green');
206207
});
207208

209+
it('should reject haxors on initial markup', function() {
210+
spyOn(console, 'error');
211+
for (var i = 0; i < 3; i++)
212+
{
213+
var container = document.createElement('div');
214+
var element = React.createElement(
215+
'x-foo-component',
216+
{'blah" onclick="beevil" noise="hi': 'selected'},
217+
null
218+
);
219+
React.render(element, container);
220+
}
221+
expect(console.error.argsForCall.length).toBe(1);
222+
expect(console.error.argsForCall[0][0]).toEqual(
223+
'Warning: Invalid attribute name: `blah" onclick="beevil" noise="hi`'
224+
);
225+
});
226+
227+
it('should reject haxors on update', function() {
228+
spyOn(console, 'error');
229+
for (var i = 0; i < 3; i++)
230+
{
231+
var container = document.createElement('div');
232+
var beforeUpdate = React.createElement('x-foo-component', {}, null);
233+
React.render(beforeUpdate, container);
234+
235+
var afterUpdate = React.createElement(
236+
'x-foo-component',
237+
{'blah" onclick="beevil" noise="hi': 'selected'},
238+
null
239+
);
240+
React.render(afterUpdate, container);
241+
}
242+
expect(console.error.argsForCall.length).toBe(1);
243+
expect(console.error.argsForCall[0][0]).toEqual(
244+
'Warning: Invalid attribute name: `blah" onclick="beevil" noise="hi`'
245+
);
246+
});
247+
248+
it('should update arbitrary attributes for tags containnig dashes', function() {
249+
var container = document.createElement('div');
250+
251+
var beforeUpdate = React.createElement('x-foo-component', {}, null);
252+
React.render(beforeUpdate, container);
253+
254+
var afterUpdate = React.createElement(
255+
'x-foo-component',
256+
{'myattr': 'myval'},
257+
null
258+
);
259+
React.render(afterUpdate, container);
260+
261+
expect(container.childNodes[0].getAttribute('myattr')).toBe('myval');
262+
});
263+
208264
it("should clear all the styles when removing 'style'", function() {
209265
var styles = {display: 'none', color: 'red'};
210266
var container = document.createElement('div');

src/browser/ui/dom/DOMPropertyOperations.js

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,30 @@ var DOMProperty = require('DOMProperty');
1717
var quoteAttributeValueForBrowser = require('quoteAttributeValueForBrowser');
1818
var warning = require('warning');
1919

20+
var VALID_ATTRIBUTE_NAME_REGEX = /^[a-zA-Z_][a-zA-Z_\.\-\d]*$/; // Simplified subset
21+
var illegalAttributeNameCache = {};
22+
var validatedAttributeNameCache = {};
23+
24+
function isAttributeNameSafe(attributeName) {
25+
if (validatedAttributeNameCache.hasOwnProperty(attributeName)) {
26+
return true;
27+
}
28+
if (illegalAttributeNameCache.hasOwnProperty(attributeName)) {
29+
return false;
30+
}
31+
if (VALID_ATTRIBUTE_NAME_REGEX.test(attributeName)) {
32+
validatedAttributeNameCache[attributeName] = true;
33+
return true;
34+
}
35+
illegalAttributeNameCache[attributeName] = true;
36+
warning(
37+
false,
38+
'Invalid attribute name: `%s`',
39+
attributeName
40+
);
41+
return false;
42+
}
43+
2044
function shouldIgnoreValue(name, value) {
2145
return value == null ||
2246
(DOMProperty.hasBooleanValue[name] && !value) ||
@@ -110,6 +134,23 @@ var DOMPropertyOperations = {
110134
return null;
111135
},
112136

137+
/**
138+
* Creates markup for a custom property.
139+
*
140+
* @param {string} name
141+
* @param {*} value
142+
* @return {?string} Markup string, or null if the property was invalid.
143+
*/
144+
createMarkupForCustomAttribute: function(name, value) {
145+
if (!isAttributeNameSafe(name)) {
146+
return '';
147+
}
148+
if (value == null) {
149+
return '';
150+
}
151+
return name + '=' + quoteAttributeValueForBrowser(value);
152+
},
153+
113154
/**
114155
* Sets the value for a property on a node.
115156
*
@@ -141,16 +182,23 @@ var DOMPropertyOperations = {
141182
}
142183
}
143184
} else if (DOMProperty.isCustomAttribute(name)) {
144-
if (value == null) {
145-
node.removeAttribute(name);
146-
} else {
147-
node.setAttribute(name, '' + value);
148-
}
185+
DOMPropertyOperations.setValueForAttribute(node, name, value);
149186
} else if (__DEV__) {
150187
warnUnknownProperty(name);
151188
}
152189
},
153190

191+
setValueForAttribute: function(node, name, value) {
192+
if (!isAttributeNameSafe(name)) {
193+
return;
194+
}
195+
if (value == null) {
196+
node.removeAttribute(name);
197+
} else {
198+
node.setAttribute(name, '' + value);
199+
}
200+
},
201+
154202
/**
155203
* Deletes the value for a property on a node.
156204
*

src/browser/ui/dom/__tests__/DOMPropertyOperations-test.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,21 @@ describe('DOMPropertyOperations', function() {
179179

180180
});
181181

182+
describe('createMarkupForProperty', function() {
183+
184+
it('should allow custom properties on web components', function() {
185+
expect(DOMPropertyOperations.createMarkupForCustomAttribute(
186+
'awesomeness',
187+
5
188+
)).toBe('awesomeness="5"');
189+
190+
expect(DOMPropertyOperations.createMarkupForCustomAttribute(
191+
'dev',
192+
'jim'
193+
)).toBe('dev="jim"');
194+
});
195+
});
196+
182197
describe('setValueForProperty', function() {
183198
var stubNode;
184199

0 commit comments

Comments
 (0)