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 30, 2015
1 parent a092b47 commit 90ba511
Show file tree
Hide file tree
Showing 5 changed files with 154 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
56 changes: 56 additions & 0 deletions src/browser/ui/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ describe('ReactDOMComponent', function() {
var ReactTestUtils;

beforeEach(function() {
require('mock-modules').dumpCache();
React = require('React');
ReactTestUtils = require('ReactTestUtils');
});
Expand Down Expand Up @@ -205,6 +206,61 @@ describe('ReactDOMComponent', function() {
expect(stubStyle.color).toEqual('green');
});

it('should reject haxors on initial markup', function() {
spyOn(console, 'error');
for (var i = 0; i < 3; i++)
{
var container = document.createElement('div');
var element = React.createElement(
'x-foo-component',
{'blah" onclick="beevil" noise="hi': 'selected'},
null
);
React.render(element, container);
}
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toEqual(
'Warning: Invalid attribute name: `blah" onclick="beevil" noise="hi`'
);
});

it('should reject haxors on update', function() {
spyOn(console, 'error');
for (var i = 0; i < 3; i++)
{
var container = document.createElement('div');
var beforeUpdate = React.createElement('x-foo-component', {}, null);
React.render(beforeUpdate, container);

var afterUpdate = React.createElement(
'x-foo-component',
{'blah" onclick="beevil" noise="hi': 'selected'},
null
);
React.render(afterUpdate, container);
}
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toEqual(
'Warning: 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
58 changes: 53 additions & 5 deletions src/browser/ui/dom/DOMPropertyOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,30 @@ var DOMProperty = require('DOMProperty');
var quoteAttributeValueForBrowser = require('quoteAttributeValueForBrowser');
var warning = require('warning');

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

function isAttributeNameSafe(attributeName) {
if (validatedAttributeNameCache.hasOwnProperty(attributeName)) {
return true;
}
if (illegalAttributeNameCache.hasOwnProperty(attributeName)) {
return false;
}
if (VALID_ATTRIBUTE_NAME_REGEX.test(attributeName)) {
validatedAttributeNameCache[attributeName] = true;
return true;
}
illegalAttributeNameCache[attributeName] = true;
warning(
false,
'Invalid attribute name: `%s`',
attributeName
);
return false;
}

function shouldIgnoreValue(name, value) {
return value == null ||
(DOMProperty.hasBooleanValue[name] && !value) ||
Expand Down Expand Up @@ -110,6 +134,23 @@ 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) {
if (!isAttributeNameSafe(name)) {
return '';
}
if (value == null) {
return '';
}
return name + '=' + quoteAttributeValueForBrowser(value);
},

/**
* Sets the value for a property on a node.
*
Expand Down Expand Up @@ -141,16 +182,23 @@ 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) {
if (!isAttributeNameSafe(name)) {
return;
}
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 90ba511

Please sign in to comment.