From a88b655ccd6f6519fb25fb8e81161f80274b9b02 Mon Sep 17 00:00:00 2001 From: Federico Rampazzo Date: Tue, 5 May 2015 16:08:22 -0700 Subject: [PATCH 1/2] Added support for namespace attributes --- src/browser/__tests__/ReactDOMSVG-test.js | 90 +++++++++++++++++++ src/browser/ui/dom/DOMProperty.js | 12 +++ src/browser/ui/dom/DOMPropertyOperations.js | 8 +- src/browser/ui/dom/HTMLDOMPropertyConfig.js | 5 +- src/browser/ui/dom/SVGDOMPropertyConfig.js | 43 ++++++++- .../__tests__/DOMPropertyOperations-test.js | 7 ++ 6 files changed, 162 insertions(+), 3 deletions(-) create mode 100644 src/browser/__tests__/ReactDOMSVG-test.js diff --git a/src/browser/__tests__/ReactDOMSVG-test.js b/src/browser/__tests__/ReactDOMSVG-test.js new file mode 100644 index 0000000000..b5f31e89fc --- /dev/null +++ b/src/browser/__tests__/ReactDOMSVG-test.js @@ -0,0 +1,90 @@ +/** + * Copyright 2013-2015, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @emails react-core + */ + +/*jslint evil: true */ + +'use strict'; + +var React = require('React'); +var ReactTestUtils = require('ReactTestUtils'); + +var SVGDOMNamespaces = { + xlink: 'http://www.w3.org/1999/xlink', + xml: 'http://www.w3.org/XML/1998/namespace', + xmlns: 'http://www.w3.org/2000/xmlns/' +}; + +describe('ReactDOMSVG', function() { + it("allows a SVG element", function() { + var element = React.createElement('svg', {xmlnsXlink: 'http://www.w3.org/1999/xlink'}); + var instance = ReactTestUtils.renderIntoDocument(element); + var svg = React.findDOMNode(instance); + expect(svg.tagName).toBe('svg'); + var xlink = svg.getAttributeNS(SVGDOMNamespaces.xmlns, 'xlink'); + expect(xlink).toBe('http://www.w3.org/1999/xlink'); + }); + + it("allows a SVG element with an image node", function() { + var instance = ReactTestUtils.renderIntoDocument( + + + + ); + var svg = React.findDOMNode(instance); + var image = svg.childNodes[0]; + expect(image.tagName).toBe('image'); + var href = image.getAttributeNS(SVGDOMNamespaces.xlink, 'href'); + expect(href).toBe('http://i.imgur.com/w7GCRPb.png'); + }); + + it("allows a SVG element with a link", function() { + var instance = ReactTestUtils.renderIntoDocument( + + + + React + + + + ); + var svg = React.findDOMNode(instance); + var link = svg.childNodes[0].childNodes[0]; + expect(link.tagName).toBe('a'); + var href = link.getAttributeNS(SVGDOMNamespaces.xlink, 'href'); + expect(href).toBe('http://facebook.github.io/react/'); + var actuate = link.getAttributeNS(SVGDOMNamespaces.xlink, 'actuate'); + expect(actuate).toBe('onRequest'); + var show = link.getAttributeNS(SVGDOMNamespaces.xlink, 'show'); + expect(show).toBe('new'); + var arcrole = link.getAttributeNS(SVGDOMNamespaces.xlink, 'arcrole'); + expect(arcrole).toBe('http://example.com/iri-arcrole-reference.svg'); + var role = link.getAttributeNS(SVGDOMNamespaces.xlink, 'role'); + expect(role).toBe('http://example.com/iri-role-reference.svg'); + var title = link.getAttributeNS(SVGDOMNamespaces.xlink, 'title'); + expect(title).toBe('React'); + var type = link.getAttributeNS(SVGDOMNamespaces.xlink, 'type'); + expect(type).toBe('simple'); + var text = link.childNodes[0]; + var lang = text.getAttributeNS(SVGDOMNamespaces.xml, 'lang'); + expect(lang).toBe('en-US'); + var space = text.getAttributeNS(SVGDOMNamespaces.xml, 'space'); + expect(space).toBe('preserve'); + }); + +}); \ No newline at end of file diff --git a/src/browser/ui/dom/DOMProperty.js b/src/browser/ui/dom/DOMProperty.js index d6cc399e8a..1939d8072d 100644 --- a/src/browser/ui/dom/DOMProperty.js +++ b/src/browser/ui/dom/DOMProperty.js @@ -60,6 +60,7 @@ var DOMPropertyInjection = { */ injectDOMPropertyConfig: function(domPropertyConfig) { var Properties = domPropertyConfig.Properties || {}; + var PropertyNamespaces = domPropertyConfig.PropertyNamespaces || {}; var DOMAttributeNames = domPropertyConfig.DOMAttributeNames || {}; var DOMPropertyNames = domPropertyConfig.DOMPropertyNames || {}; var DOMMutationMethods = domPropertyConfig.DOMMutationMethods || {}; @@ -93,6 +94,11 @@ var DOMPropertyInjection = { DOMProperty.getAttributeName[propName] = lowerCased; } + if (PropertyNamespaces.hasOwnProperty(propName)) { + DOMProperty.getAttributeNamespace[propName] = + PropertyNamespaces[propName]; + } + DOMProperty.getPropertyName[propName] = DOMPropertyNames.hasOwnProperty(propName) ? DOMPropertyNames[propName] : @@ -182,6 +188,12 @@ var DOMProperty = { */ getAttributeName: {}, + /** + * Mapping from normalized names to namespaces. + * @type {Object} + */ + getAttributeNamespace: {}, + /** * Mapping from normalized names to properties on DOM node instances. * (This includes properties that mutate due to external factors.) diff --git a/src/browser/ui/dom/DOMPropertyOperations.js b/src/browser/ui/dom/DOMPropertyOperations.js index f049403184..b25f62c01d 100644 --- a/src/browser/ui/dom/DOMPropertyOperations.js +++ b/src/browser/ui/dom/DOMPropertyOperations.js @@ -126,9 +126,15 @@ var DOMPropertyOperations = { } else if (shouldIgnoreValue(name, value)) { this.deleteValueForProperty(node, name); } else if (DOMProperty.mustUseAttribute[name]) { + var attributeName = DOMProperty.getAttributeName[name]; + var namespace = DOMProperty.getAttributeNamespace[name]; // `setAttribute` with objects becomes only `[object]` in IE8/9, // ('' + value) makes it output the correct toString()-value. - node.setAttribute(DOMProperty.getAttributeName[name], '' + value); + if (namespace) { + node.setAttributeNS(namespace, attributeName, '' + value); + } else { + node.setAttribute(attributeName, '' + value); + } } else { var propName = DOMProperty.getPropertyName[name]; // Must explicitly cast values for HAS_SIDE_EFFECTS-properties to the diff --git a/src/browser/ui/dom/HTMLDOMPropertyConfig.js b/src/browser/ui/dom/HTMLDOMPropertyConfig.js index b814f3b611..e59757f444 100644 --- a/src/browser/ui/dom/HTMLDOMPropertyConfig.js +++ b/src/browser/ui/dom/HTMLDOMPropertyConfig.js @@ -39,7 +39,6 @@ if (ExecutionEnvironment.canUseDOM) { ); } - var HTMLDOMPropertyConfig = { isCustomAttribute: RegExp.prototype.test.bind( /^(data|aria)-[a-z_][a-z\d_.\-]*$/ @@ -160,6 +159,7 @@ var HTMLDOMPropertyConfig = { value: MUST_USE_PROPERTY | HAS_SIDE_EFFECTS, width: MUST_USE_ATTRIBUTE, wmode: MUST_USE_ATTRIBUTE, + xmlns: MUST_USE_ATTRIBUTE, /** * Non-standard Properties @@ -183,6 +183,9 @@ var HTMLDOMPropertyConfig = { // IE-only attribute that controls focus behavior unselectable: MUST_USE_ATTRIBUTE }, + PropertyNamespaces: { + xmlns: 'http://www.w3.org/2000/xmlns/' + }, DOMAttributeNames: { acceptCharset: 'accept-charset', className: 'class', diff --git a/src/browser/ui/dom/SVGDOMPropertyConfig.js b/src/browser/ui/dom/SVGDOMPropertyConfig.js index ef2f9c3ffc..7332fb311c 100644 --- a/src/browser/ui/dom/SVGDOMPropertyConfig.js +++ b/src/browser/ui/dom/SVGDOMPropertyConfig.js @@ -17,6 +17,12 @@ var DOMProperty = require('DOMProperty'); var MUST_USE_ATTRIBUTE = DOMProperty.injection.MUST_USE_ATTRIBUTE; +var SVGDOMNamespaces = { + xlink: 'http://www.w3.org/1999/xlink', + xml: 'http://www.w3.org/XML/1998/namespace', + xmlns: 'http://www.w3.org/2000/xmlns/' +}; + var SVGDOMPropertyConfig = { Properties: { clipPath: MUST_USE_ATTRIBUTE, @@ -60,10 +66,34 @@ var SVGDOMPropertyConfig = { x1: MUST_USE_ATTRIBUTE, x2: MUST_USE_ATTRIBUTE, x: MUST_USE_ATTRIBUTE, + xlinkActuate: MUST_USE_ATTRIBUTE, + xlinkArcrole: MUST_USE_ATTRIBUTE, + xlinkHref: MUST_USE_ATTRIBUTE, + xlinkRole: MUST_USE_ATTRIBUTE, + xlinkShow: MUST_USE_ATTRIBUTE, + xlinkTitle: MUST_USE_ATTRIBUTE, + xlinkType: MUST_USE_ATTRIBUTE, + xmlBase: MUST_USE_ATTRIBUTE, + xmlLang: MUST_USE_ATTRIBUTE, + xmlSpace: MUST_USE_ATTRIBUTE, + xmlnsXlink: MUST_USE_ATTRIBUTE, y1: MUST_USE_ATTRIBUTE, y2: MUST_USE_ATTRIBUTE, y: MUST_USE_ATTRIBUTE }, + PropertyNamespaces: { + xlinkActuate: SVGDOMNamespaces.xlink, + xlinkArcrole: SVGDOMNamespaces.xlink, + xlinkHref: SVGDOMNamespaces.xlink, + xlinkRole: SVGDOMNamespaces.xlink, + xlinkShow: SVGDOMNamespaces.xlink, + xlinkTitle: SVGDOMNamespaces.xlink, + xlinkType: SVGDOMNamespaces.xlink, + xmlBase: SVGDOMNamespaces.xml, + xmlLang: SVGDOMNamespaces.xml, + xmlSpace: SVGDOMNamespaces.xml, + xmlnsXlink: SVGDOMNamespaces.xmlns + }, DOMAttributeNames: { clipPath: 'clip-path', fillOpacity: 'fill-opacity', @@ -85,7 +115,18 @@ var SVGDOMPropertyConfig = { strokeOpacity: 'stroke-opacity', strokeWidth: 'stroke-width', textAnchor: 'text-anchor', - viewBox: 'viewBox' + viewBox: 'viewBox', + xlinkActuate: 'xlink:actuate', + xlinkArcrole: 'xlink:arcrole', + xlinkHref: 'xlink:href', + xlinkRole: 'xlink:role', + xlinkShow: 'xlink:show', + xlinkTitle: 'xlink:title', + xlinkType: 'xlink:type', + xmlBase: 'xml:base', + xmlLang: 'xml:lang', + xmlSpace: 'xml:space', + xmlnsXlink: 'xmlns:xlink' } }; diff --git a/src/browser/ui/dom/__tests__/DOMPropertyOperations-test.js b/src/browser/ui/dom/__tests__/DOMPropertyOperations-test.js index 8cadb40d1f..474cb5970e 100644 --- a/src/browser/ui/dom/__tests__/DOMPropertyOperations-test.js +++ b/src/browser/ui/dom/__tests__/DOMPropertyOperations-test.js @@ -181,9 +181,11 @@ describe('DOMPropertyOperations', function() { describe('setValueForProperty', function() { var stubNode; + var stubSVGNode; beforeEach(function() { stubNode = document.createElement('div'); + stubSVGNode = document.createElement('svg'); }); it('should set values as properties by default', function() { @@ -196,6 +198,11 @@ describe('DOMPropertyOperations', function() { expect(stubNode.getAttribute('role')).toBe('#'); expect(stubNode.role).toBeUndefined(); }); + + it('should set values as namespace attributes if necessary', function() { + DOMPropertyOperations.setValueForProperty(stubSVGNode, 'xmlnsXlink', 'http://www.w3.org/1999/xlink'); + expect(stubSVGNode.getAttribute('xmlns:xlink')).toBe('http://www.w3.org/1999/xlink'); + }); it('should convert attribute values to string first', function() { // Browsers default to this behavior, but some test environments do not. From 5f32953ac70517085a0b367feb07c1c92b248c08 Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Tue, 5 May 2015 16:46:49 -0700 Subject: [PATCH 2/2] Follow-ups for #3718 - Rename NamespaceProperties to DOMAttributeNamespaces - Make tests pass in jest - Remove unnecessary xmlns attributes --- src/browser/__tests__/ReactDOMSVG-test.js | 71 +++---------------- src/browser/ui/dom/DOMProperty.js | 11 ++- src/browser/ui/dom/HTMLDOMPropertyConfig.js | 4 -- src/browser/ui/dom/SVGDOMPropertyConfig.js | 32 ++++----- .../__tests__/DOMPropertyOperations-test.js | 15 ++-- 5 files changed, 43 insertions(+), 90 deletions(-) diff --git a/src/browser/__tests__/ReactDOMSVG-test.js b/src/browser/__tests__/ReactDOMSVG-test.js index b5f31e89fc..1ce05d6c80 100644 --- a/src/browser/__tests__/ReactDOMSVG-test.js +++ b/src/browser/__tests__/ReactDOMSVG-test.js @@ -13,8 +13,8 @@ 'use strict'; -var React = require('React'); -var ReactTestUtils = require('ReactTestUtils'); +var React; +var ReactTestUtils; var SVGDOMNamespaces = { xlink: 'http://www.w3.org/1999/xlink', @@ -23,68 +23,19 @@ var SVGDOMNamespaces = { }; describe('ReactDOMSVG', function() { - it("allows a SVG element", function() { - var element = React.createElement('svg', {xmlnsXlink: 'http://www.w3.org/1999/xlink'}); - var instance = ReactTestUtils.renderIntoDocument(element); - var svg = React.findDOMNode(instance); - expect(svg.tagName).toBe('svg'); - var xlink = svg.getAttributeNS(SVGDOMNamespaces.xmlns, 'xlink'); - expect(xlink).toBe('http://www.w3.org/1999/xlink'); + + beforeEach(function() { + React = require('React'); + ReactTestUtils = require('ReactTestUtils'); }); - it("allows a SVG element with an image node", function() { - var instance = ReactTestUtils.renderIntoDocument( - + it('creates initial namespaced markup', function() { + var markup = React.renderToString( + ); - var svg = React.findDOMNode(instance); - var image = svg.childNodes[0]; - expect(image.tagName).toBe('image'); - var href = image.getAttributeNS(SVGDOMNamespaces.xlink, 'href'); - expect(href).toBe('http://i.imgur.com/w7GCRPb.png'); + expect(markup).toContain('xlink:href="http://i.imgur.com/w7GCRPb.png"'); }); - it("allows a SVG element with a link", function() { - var instance = ReactTestUtils.renderIntoDocument( - - - - React - - - - ); - var svg = React.findDOMNode(instance); - var link = svg.childNodes[0].childNodes[0]; - expect(link.tagName).toBe('a'); - var href = link.getAttributeNS(SVGDOMNamespaces.xlink, 'href'); - expect(href).toBe('http://facebook.github.io/react/'); - var actuate = link.getAttributeNS(SVGDOMNamespaces.xlink, 'actuate'); - expect(actuate).toBe('onRequest'); - var show = link.getAttributeNS(SVGDOMNamespaces.xlink, 'show'); - expect(show).toBe('new'); - var arcrole = link.getAttributeNS(SVGDOMNamespaces.xlink, 'arcrole'); - expect(arcrole).toBe('http://example.com/iri-arcrole-reference.svg'); - var role = link.getAttributeNS(SVGDOMNamespaces.xlink, 'role'); - expect(role).toBe('http://example.com/iri-role-reference.svg'); - var title = link.getAttributeNS(SVGDOMNamespaces.xlink, 'title'); - expect(title).toBe('React'); - var type = link.getAttributeNS(SVGDOMNamespaces.xlink, 'type'); - expect(type).toBe('simple'); - var text = link.childNodes[0]; - var lang = text.getAttributeNS(SVGDOMNamespaces.xml, 'lang'); - expect(lang).toBe('en-US'); - var space = text.getAttributeNS(SVGDOMNamespaces.xml, 'space'); - expect(space).toBe('preserve'); - }); - -}); \ No newline at end of file +}); diff --git a/src/browser/ui/dom/DOMProperty.js b/src/browser/ui/dom/DOMProperty.js index 1939d8072d..c5b7b80b80 100644 --- a/src/browser/ui/dom/DOMProperty.js +++ b/src/browser/ui/dom/DOMProperty.js @@ -50,6 +50,9 @@ var DOMPropertyInjection = { * attribute name. Attribute names not specified use the **lowercase** * normalized name. * + * DOMAttributeNamespaces: object mapping React attribute name to the DOM + * attribute namespace URL. (Attribute names not specified use no namespace.) + * * DOMPropertyNames: similar to DOMAttributeNames but for DOM properties. * Property names not specified use the normalized name. * @@ -60,7 +63,7 @@ var DOMPropertyInjection = { */ injectDOMPropertyConfig: function(domPropertyConfig) { var Properties = domPropertyConfig.Properties || {}; - var PropertyNamespaces = domPropertyConfig.PropertyNamespaces || {}; + var DOMAttributeNamespaces = domPropertyConfig.DOMAttributeNamespaces || {}; var DOMAttributeNames = domPropertyConfig.DOMAttributeNames || {}; var DOMPropertyNames = domPropertyConfig.DOMPropertyNames || {}; var DOMMutationMethods = domPropertyConfig.DOMMutationMethods || {}; @@ -94,9 +97,11 @@ var DOMPropertyInjection = { DOMProperty.getAttributeName[propName] = lowerCased; } - if (PropertyNamespaces.hasOwnProperty(propName)) { + if (DOMAttributeNamespaces.hasOwnProperty(propName)) { DOMProperty.getAttributeNamespace[propName] = - PropertyNamespaces[propName]; + DOMAttributeNamespaces[propName]; + } else { + DOMProperty.getAttributeNamespace[propName] = null; } DOMProperty.getPropertyName[propName] = diff --git a/src/browser/ui/dom/HTMLDOMPropertyConfig.js b/src/browser/ui/dom/HTMLDOMPropertyConfig.js index e59757f444..d8095e03e7 100644 --- a/src/browser/ui/dom/HTMLDOMPropertyConfig.js +++ b/src/browser/ui/dom/HTMLDOMPropertyConfig.js @@ -159,7 +159,6 @@ var HTMLDOMPropertyConfig = { value: MUST_USE_PROPERTY | HAS_SIDE_EFFECTS, width: MUST_USE_ATTRIBUTE, wmode: MUST_USE_ATTRIBUTE, - xmlns: MUST_USE_ATTRIBUTE, /** * Non-standard Properties @@ -183,9 +182,6 @@ var HTMLDOMPropertyConfig = { // IE-only attribute that controls focus behavior unselectable: MUST_USE_ATTRIBUTE }, - PropertyNamespaces: { - xmlns: 'http://www.w3.org/2000/xmlns/' - }, DOMAttributeNames: { acceptCharset: 'accept-charset', className: 'class', diff --git a/src/browser/ui/dom/SVGDOMPropertyConfig.js b/src/browser/ui/dom/SVGDOMPropertyConfig.js index 7332fb311c..528ab2bd45 100644 --- a/src/browser/ui/dom/SVGDOMPropertyConfig.js +++ b/src/browser/ui/dom/SVGDOMPropertyConfig.js @@ -17,10 +17,9 @@ var DOMProperty = require('DOMProperty'); var MUST_USE_ATTRIBUTE = DOMProperty.injection.MUST_USE_ATTRIBUTE; -var SVGDOMNamespaces = { +var NS = { xlink: 'http://www.w3.org/1999/xlink', - xml: 'http://www.w3.org/XML/1998/namespace', - xmlns: 'http://www.w3.org/2000/xmlns/' + xml: 'http://www.w3.org/XML/1998/namespace' }; var SVGDOMPropertyConfig = { @@ -76,23 +75,21 @@ var SVGDOMPropertyConfig = { xmlBase: MUST_USE_ATTRIBUTE, xmlLang: MUST_USE_ATTRIBUTE, xmlSpace: MUST_USE_ATTRIBUTE, - xmlnsXlink: MUST_USE_ATTRIBUTE, y1: MUST_USE_ATTRIBUTE, y2: MUST_USE_ATTRIBUTE, y: MUST_USE_ATTRIBUTE }, - PropertyNamespaces: { - xlinkActuate: SVGDOMNamespaces.xlink, - xlinkArcrole: SVGDOMNamespaces.xlink, - xlinkHref: SVGDOMNamespaces.xlink, - xlinkRole: SVGDOMNamespaces.xlink, - xlinkShow: SVGDOMNamespaces.xlink, - xlinkTitle: SVGDOMNamespaces.xlink, - xlinkType: SVGDOMNamespaces.xlink, - xmlBase: SVGDOMNamespaces.xml, - xmlLang: SVGDOMNamespaces.xml, - xmlSpace: SVGDOMNamespaces.xml, - xmlnsXlink: SVGDOMNamespaces.xmlns + DOMAttributeNamespaces: { + xlinkActuate: NS.xlink, + xlinkArcrole: NS.xlink, + xlinkHref: NS.xlink, + xlinkRole: NS.xlink, + xlinkShow: NS.xlink, + xlinkTitle: NS.xlink, + xlinkType: NS.xlink, + xmlBase: NS.xml, + xmlLang: NS.xml, + xmlSpace: NS.xml }, DOMAttributeNames: { clipPath: 'clip-path', @@ -125,8 +122,7 @@ var SVGDOMPropertyConfig = { xlinkType: 'xlink:type', xmlBase: 'xml:base', xmlLang: 'xml:lang', - xmlSpace: 'xml:space', - xmlnsXlink: 'xmlns:xlink' + xmlSpace: 'xml:space' } }; diff --git a/src/browser/ui/dom/__tests__/DOMPropertyOperations-test.js b/src/browser/ui/dom/__tests__/DOMPropertyOperations-test.js index 474cb5970e..27b57aed6d 100644 --- a/src/browser/ui/dom/__tests__/DOMPropertyOperations-test.js +++ b/src/browser/ui/dom/__tests__/DOMPropertyOperations-test.js @@ -181,11 +181,9 @@ describe('DOMPropertyOperations', function() { describe('setValueForProperty', function() { var stubNode; - var stubSVGNode; beforeEach(function() { stubNode = document.createElement('div'); - stubSVGNode = document.createElement('svg'); }); it('should set values as properties by default', function() { @@ -198,10 +196,17 @@ describe('DOMPropertyOperations', function() { expect(stubNode.getAttribute('role')).toBe('#'); expect(stubNode.role).toBeUndefined(); }); - + it('should set values as namespace attributes if necessary', function() { - DOMPropertyOperations.setValueForProperty(stubSVGNode, 'xmlnsXlink', 'http://www.w3.org/1999/xlink'); - expect(stubSVGNode.getAttribute('xmlns:xlink')).toBe('http://www.w3.org/1999/xlink'); + spyOn(stubNode, 'setAttributeNS'); + DOMPropertyOperations.setValueForProperty( + stubNode, + 'xlinkHref', + 'about:blank' + ); + expect(stubNode.setAttributeNS.argsForCall.length).toBe(1); + expect(stubNode.setAttributeNS.argsForCall[0]) + .toEqual(['http://www.w3.org/1999/xlink', 'xlink:href', 'about:blank']); }); it('should convert attribute values to string first', function() {