Skip to content

Commit

Permalink
Support arbitrary attributes on elements with dashes in the tag name.
Browse files Browse the repository at this point in the history
  • Loading branch information
jimfb committed Apr 28, 2015
1 parent 49de80e commit ad30a5a
Show file tree
Hide file tree
Showing 5 changed files with 136 additions and 7 deletions.
14 changes: 12 additions & 2 deletions src/browser/ui/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -332,8 +332,12 @@ ReactDOMComponent.Mixin = {
}
propValue = CSSPropertyOperations.createMarkupForStyles(propValue);
}
var markup =
DOMPropertyOperations.createMarkupForProperty(propKey, propValue);
var markup = null;
if (this._tag != null && this._tag.indexOf('-') >= 0) {
markup = DOMPropertyOperations.createMarkupForCustomAttribute(propKey, propValue);
} else {
markup = DOMPropertyOperations.createMarkupForProperty(propKey, propValue);
}
if (markup) {
ret += ' ' + markup;
}
Expand Down Expand Up @@ -534,6 +538,12 @@ ReactDOMComponent.Mixin = {
} else if (lastProp) {
deleteListener(this._rootNodeID, propKey);
}
} else if (this._tag.indexOf('-') >= 0) {
BackendIDOperations.updateAttributeByID(
this._rootNodeID,
propKey,
nextProp
);
} else if (
DOMProperty.isStandardName[propKey] ||
DOMProperty.isCustomAttribute(propKey)) {
Expand Down
18 changes: 18 additions & 0 deletions src/browser/ui/ReactDOMIDOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,24 @@ var ReactDOMIDOperations = {
}
},

/**
* Updates a DOM node with new property values.
*
* @param {string} id ID of the node to update.
* @param {string} name A valid property name.
* @param {*} value New value of the property.
* @internal
*/
updateAttributeByID: function(id, name, value) {
var node = ReactMount.getNode(id);
invariant(
!INVALID_PROPERTY_ERRORS.hasOwnProperty(name),
'updatePropertyByID(...): %s',
INVALID_PROPERTY_ERRORS[name]
);
DOMPropertyOperations.setValueForAttribute(node, name, value);
},

/**
* Updates a DOM node to remove a property. This should only be used to remove
* DOM properties in `DOMProperty`.
Expand Down
51 changes: 51 additions & 0 deletions src/browser/ui/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,57 @@ describe('ReactDOMComponent', function() {
expect(stubStyle.color).toEqual('green');
});

it('should reject haxors on initial markup', function() {
var container = document.createElement('div');

expect(function() {
var element = React.createElement(
'x-foo-component',
{'blah" onclick="beevil" noise="hi': 'selected'},
null
);
React.render(element, container);
}).toThrow(
'Invariant Violation: Invalid attribute name: ' +
'`blah" onclick="beevil" noise="hi`'
);
});

it('should reject haxors on update', function() {
var container = document.createElement('div');

var beforeUpdate = React.createElement('x-foo-component', {}, null);
React.render(beforeUpdate, container);

expect(function() {
var afterUpdate = React.createElement(
'x-foo-component',
{'blah" onclick="beevil" noise="hi': 'selected'},
null
);
React.render(afterUpdate, container);
}).toThrow(
'Invariant Violation: Invalid attribute name: ' +
'`blah" onclick="beevil" noise="hi`'
);
});

it('should update arbitrary attributes for tags containnig dashes', function() {
var container = document.createElement('div');

var beforeUpdate = React.createElement('x-foo-component', {}, null);
React.render(beforeUpdate, container);

var afterUpdate = React.createElement(
'x-foo-component',
{'myattr': 'myval'},
null
);
React.render(afterUpdate, container);

expect(container.childNodes[0].getAttribute('myattr')).toBe('myval');
});

it("should clear all the styles when removing 'style'", function() {
var styles = {display: 'none', color: 'red'};
var container = document.createElement('div');
Expand Down
45 changes: 40 additions & 5 deletions src/browser/ui/dom/DOMPropertyOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,23 @@
var DOMProperty = require('DOMProperty');

var quoteAttributeValueForBrowser = require('quoteAttributeValueForBrowser');
var invariant = require('invariant');
var warning = require('warning');

var VALID_ATTRIBUTE_NAME_REGEX = /^[a-zA-Z_][a-zA-Z_\.\-\d]*$/; // Simplified subset
var validatedAttributeNameCache = {};

function validateDangerousAttributeName(attributeName) {
if (!hasOwnProperty.call(validatedAttributeNameCache, attributeName)) {
invariant(
VALID_ATTRIBUTE_NAME_REGEX.test(attributeName),
'Invalid attribute name: `%s`',
attributeName
);
validatedAttributeNameCache[attributeName] = true;
}
}

function shouldIgnoreValue(name, value) {
return value == null ||
(DOMProperty.hasBooleanValue[name] && !value) ||
Expand Down Expand Up @@ -110,6 +125,21 @@ var DOMPropertyOperations = {
return null;
},

/**
* Creates markup for a custom property.
*
* @param {string} name
* @param {*} value
* @return {?string} Markup string, or null if the property was invalid.
*/
createMarkupForCustomAttribute: function(name, value) {
validateDangerousAttributeName(name);
if (value == null) {
return '';
}
return name + '=' + quoteAttributeValueForBrowser(value);
},

/**
* Sets the value for a property on a node.
*
Expand Down Expand Up @@ -141,16 +171,21 @@ var DOMPropertyOperations = {
}
}
} else if (DOMProperty.isCustomAttribute(name)) {
if (value == null) {
node.removeAttribute(name);
} else {
node.setAttribute(name, '' + value);
}
DOMPropertyOperations.setValueForAttribute(node, name, value);
} else if (__DEV__) {
warnUnknownProperty(name);
}
},

setValueForAttribute: function(node, name, value) {
validateDangerousAttributeName(name);
if (value == null) {
node.removeAttribute(name);
} else {
node.setAttribute(name, '' + value);
}
},

/**
* Deletes the value for a property on a node.
*
Expand Down
15 changes: 15 additions & 0 deletions src/browser/ui/dom/__tests__/DOMPropertyOperations-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,21 @@ describe('DOMPropertyOperations', function() {

});

describe('createMarkupForProperty', function() {

it('should allow custom properties on web components', function() {
expect(DOMPropertyOperations.createMarkupForCustomAttribute(
'awesomeness',
5
)).toBe('awesomeness="5"');

expect(DOMPropertyOperations.createMarkupForCustomAttribute(
'dev',
'jim'
)).toBe('dev="jim"');
});
});

describe('setValueForProperty', function() {
var stubNode;

Expand Down

0 comments on commit ad30a5a

Please sign in to comment.