From 5f32953ac70517085a0b367feb07c1c92b248c08 Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Tue, 5 May 2015 16:46:49 -0700 Subject: [PATCH] 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() {