From 101af015b00433faffe4c3ef4f4d373326e6f893 Mon Sep 17 00:00:00 2001 From: Matthew Beale Date: Wed, 7 Jan 2015 22:23:21 -0500 Subject: [PATCH] Change setProperty and morph to remove an undefined attr value --- packages/morph/lib/attr-morph.js | 6 +++--- packages/morph/lib/dom-helper.js | 5 +++-- packages/morph/lib/dom-helper/prop.js | 4 ++++ packages/morph/tests/attr-morph-test.js | 27 ++++++++++++++++++++++++- packages/morph/tests/dom-helper-test.js | 25 +++++++++++++++++++---- 5 files changed, 57 insertions(+), 10 deletions(-) diff --git a/packages/morph/lib/attr-morph.js b/packages/morph/lib/attr-morph.js index f9f4c212..523e0031 100644 --- a/packages/morph/lib/attr-morph.js +++ b/packages/morph/lib/attr-morph.js @@ -1,5 +1,5 @@ import { sanitizeAttributeValue } from "./attr-morph/sanitize-attribute-value"; -import { normalizeProperty } from "./dom-helper/prop"; +import { isAttrRemovalValue, normalizeProperty } from "./dom-helper/prop"; import { svgNamespace } from "./dom-helper/build-html-dom"; function updateProperty(value) { @@ -7,7 +7,7 @@ function updateProperty(value) { } function updateAttribute(value) { - if (value === null) { + if (isAttrRemovalValue(value)) { this.domHelper.removeAttribute(this.element, this.attrName); } else { this.domHelper.setAttribute(this.element, this.attrName, value); @@ -15,7 +15,7 @@ function updateAttribute(value) { } function updateAttributeNS(value) { - if (value === null) { + if (isAttrRemovalValue(value)) { this.domHelper.removeAttribute(this.element, this.attrName); } else { this.domHelper.setAttributeNS(this.element, this.namespace, this.attrName, value); diff --git a/packages/morph/lib/dom-helper.js b/packages/morph/lib/dom-helper.js index 65b898e8..982f2b79 100644 --- a/packages/morph/lib/dom-helper.js +++ b/packages/morph/lib/dom-helper.js @@ -12,6 +12,7 @@ import { import { normalizeProperty } from "./dom-helper/prop"; +import { isAttrRemovalValue } from "./dom-helper/prop"; var doc = typeof document === 'undefined' ? false : document; @@ -167,7 +168,7 @@ prototype.setPropertyStrict = function(element, name, value) { prototype.setProperty = function(element, name, value, namespace) { var lowercaseName = name.toLowerCase(); if (element.namespaceURI === svgNamespace || lowercaseName === 'style') { - if (value === null) { + if (isAttrRemovalValue(value)) { element.removeAttribute(name); } else { if (namespace) { @@ -181,7 +182,7 @@ prototype.setProperty = function(element, name, value, namespace) { if (normalized) { element[normalized] = value; } else { - if (value === null) { + if (isAttrRemovalValue(value)) { element.removeAttribute(name); } else { if (namespace) { diff --git a/packages/morph/lib/dom-helper/prop.js b/packages/morph/lib/dom-helper/prop.js index dcf9270d..8268cfad 100644 --- a/packages/morph/lib/dom-helper/prop.js +++ b/packages/morph/lib/dom-helper/prop.js @@ -1,3 +1,7 @@ +export function isAttrRemovalValue(value) { + return value === null || value === undefined; +} + // TODO should this be an o_create kind of thing? export var propertyCaches = {}; diff --git a/packages/morph/tests/attr-morph-test.js b/packages/morph/tests/attr-morph-test.js index 2b4e2d24..a869923c 100644 --- a/packages/morph/tests/attr-morph-test.js +++ b/packages/morph/tests/attr-morph-test.js @@ -3,7 +3,8 @@ import DOMHelper from "../morph/dom-helper"; import SafeString from "htmlbars-util/safe-string"; -var svgNamespace = "http://www.w3.org/2000/svg"; +var svgNamespace = "http://www.w3.org/2000/svg", + xlinkNamespace = "http://www.w3.org/1999/xlink"; var domHelper = new DOMHelper(); QUnit.module('morph: AttrMorph'); @@ -34,6 +35,30 @@ test("can update attribute", function(){ equal(element.getAttribute('data-bop'), undefined, 'data-bop attribute is removed'); }); +test("can remove ns attribute with null", function(){ + var element = domHelper.createElement('svg'); + domHelper.setAttribute(element, 'xlink:title', 'Great Title', xlinkNamespace); + var morph = domHelper.createAttrMorph(element, 'xlink:title', xlinkNamespace); + morph.setContent(null); + equal(element.getAttribute('xlink:title'), undefined, 'ns attribute is removed'); +}); + +test("can remove attribute with undefined", function(){ + var element = domHelper.createElement('div'); + element.setAttribute('data-bop', 'kpow'); + var morph = domHelper.createAttrMorph(element, 'data-bop'); + morph.setContent(undefined); + equal(element.getAttribute('data-bop'), undefined, 'data-bop attribute is removed'); +}); + +test("can remove ns attribute with undefined", function(){ + var element = domHelper.createElement('svg'); + domHelper.setAttribute(element, 'xlink:title', 'Great Title', xlinkNamespace); + var morph = domHelper.createAttrMorph(element, 'xlink:title', xlinkNamespace); + morph.setContent(undefined); + equal(element.getAttribute('xlink:title'), undefined, 'ns attribute is removed'); +}); + test("can update svg attribute", function(){ domHelper.setNamespace(svgNamespace); var element = domHelper.createElement('svg'); diff --git a/packages/morph/tests/dom-helper-test.js b/packages/morph/tests/dom-helper-test.js index 204b88f7..dbbdd572 100644 --- a/packages/morph/tests/dom-helper-test.js +++ b/packages/morph/tests/dom-helper-test.js @@ -5,6 +5,7 @@ import { } from "../htmlbars-test-helpers"; var xhtmlNamespace = "http://www.w3.org/1999/xhtml", + xlinkNamespace = "http://www.w3.org/1999/xlink", svgNamespace = "http://www.w3.org/2000/svg"; var foreignNamespaces = ['foreignObject', 'desc', 'title']; @@ -62,14 +63,14 @@ test('#setAttribute', function(){ test('#setAttributeNS', function(){ var node = dom.createElement('svg'); - dom.setAttributeNS(node, 'http://www.w3.org/1999/xlink', 'xlink:href', 'super-fun'); + dom.setAttributeNS(node, xlinkNamespace, 'xlink:href', 'super-fun'); // chrome adds (xmlns:xlink="http://www.w3.org/1999/xlink") property while others don't // thus equalHTML is not useful var el = document.createElement('div'); el.appendChild(node); // phantomjs omits the prefix, thus we can't find xlink: ok(el.innerHTML.indexOf('href="super-fun"') > 0); - dom.setAttributeNS(node, 'http://www.w3.org/1999/xlink', 'href', null); + dom.setAttributeNS(node, xlinkNamespace, 'href', null); ok(el.innerHTML.indexOf('href="null"') > 0); @@ -149,14 +150,30 @@ test('#setProperty', function(){ node = dom.createElement('svg'); dom.setProperty(node, 'viewBox', '0 0 0 0'); equalHTML(node, ''); - - dom.setProperty(node, 'xlink:title', 'super-blast','http://www.w3.org/1999/xlink'); + + dom.setProperty(node, 'xlink:title', 'super-blast', xlinkNamespace); // chrome adds (xmlns:xlink="http://www.w3.org/1999/xlink") property while others don't // thus equalHTML is not useful var el = document.createElement('div'); el.appendChild(node); // phantom js omits the prefix so we can't look for xlink: ok(el.innerHTML.indexOf('title="super-blast"') > 0 ); + + dom.setProperty(node, 'xlink:title', null, xlinkNamespace); + equal(node.getAttribute('xlink:title'), null, 'ns attr is removed'); +}); + +test('#setProperty removes attr with undefined', function(){ + var node = dom.createElement('div'); + dom.setProperty(node, 'data-fun', 'whoopie'); + equalHTML(node, '
'); + dom.setProperty(node, 'data-fun', undefined); + equalHTML(node, '
', 'undefined attribute removes the attribute'); + + node = dom.createElement('svg'); + dom.setProperty(node, 'xlink:title', 'Great Title', xlinkNamespace); + dom.setProperty(node, 'xlink:title', undefined, xlinkNamespace); + equal(node.getAttribute('xlink:title'), undefined, 'ns attr is removed'); }); test('#addClasses', function(){